[PATCH v23 14/19] x86/resctrl: Handle removing directories in Sub-NUMA Cluster (SNC) mode

Tony Luck posted 19 patches 1 year, 5 months ago
[PATCH v23 14/19] x86/resctrl: Handle removing directories in Sub-NUMA Cluster (SNC) mode
Posted by Tony Luck 1 year, 5 months ago
In SNC mode there are multiple subdirectories in each L3 level monitor
directory (one for each SNC node). If all the CPUs in an SNC node are
taken offline, just remove the SNC directory for that node. In
non-SNC mode, or when the last SNC node directory is removed,
remove the L3 monitor directory.

Signed-off-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 40 +++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 8502385e389f..67a8188c6d8f 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -3008,20 +3008,46 @@ static int mon_addfile(struct kernfs_node *parent_kn, const char *name,
 
 /*
  * Remove all subdirectories of mon_data of ctrl_mon groups
- * and monitor groups with given domain id.
+ * and monitor groups for the given domain.
+ * Remove files and directories containing "sum" of domain data
+ * when last domain being summed is removed.
  */
 static void rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
-					   unsigned int dom_id)
+					   struct rdt_mon_domain *d)
 {
 	struct rdtgroup *prgrp, *crgrp;
+	struct kernfs_node *kn;
+	char subname[32];
+	bool snc_mode;
 	char name[32];
 
+	snc_mode = r->mon_scope == RESCTRL_L3_NODE;
+	sprintf(name, "mon_%s_%02d", r->name, snc_mode ? d->ci->id : d->hdr.id);
+	if (snc_mode)
+		sprintf(subname, "mon_sub_%s_%02d", r->name, d->hdr.id);
+
 	list_for_each_entry(prgrp, &rdt_all_groups, rdtgroup_list) {
-		sprintf(name, "mon_%s_%02d", r->name, dom_id);
-		kernfs_remove_by_name(prgrp->mon.mon_data_kn, name);
+		kn = kernfs_find_and_get(prgrp->mon.mon_data_kn, name);
+		if (!kn)
+			continue;
+		kernfs_put(kn);
+
+		if (kn->dir.subdirs <= 1)
+			kernfs_remove(kn);
+		else
+			kernfs_remove_by_name(kn, subname);
 
-		list_for_each_entry(crgrp, &prgrp->mon.crdtgrp_list, mon.crdtgrp_list)
-			kernfs_remove_by_name(crgrp->mon.mon_data_kn, name);
+		list_for_each_entry(crgrp, &prgrp->mon.crdtgrp_list, mon.crdtgrp_list) {
+			kn = kernfs_find_and_get(crgrp->mon.mon_data_kn, name);
+			if (!kn)
+				continue;
+			kernfs_put(kn);
+
+			if (kn->dir.subdirs <= 1)
+				kernfs_remove(kn);
+			else
+				kernfs_remove_by_name(kn, subname);
+		}
 	}
 }
 
@@ -3991,7 +4017,7 @@ void resctrl_offline_mon_domain(struct rdt_resource *r, struct rdt_mon_domain *d
 	 * per domain monitor data directories.
 	 */
 	if (resctrl_mounted && resctrl_arch_mon_capable())
-		rmdir_mondata_subdir_allrdtgrp(r, d->hdr.id);
+		rmdir_mondata_subdir_allrdtgrp(r, d);
 
 	if (is_mbm_enabled())
 		cancel_delayed_work(&d->mbm_over);
-- 
2.45.2
Re: [PATCH v23 14/19] x86/resctrl: Handle removing directories in Sub-NUMA Cluster (SNC) mode
Posted by Borislav Petkov 1 year, 5 months ago
On Fri, Jun 28, 2024 at 02:56:14PM -0700, Tony Luck wrote:
>  	list_for_each_entry(prgrp, &rdt_all_groups, rdtgroup_list) {
> -		sprintf(name, "mon_%s_%02d", r->name, dom_id);
> -		kernfs_remove_by_name(prgrp->mon.mon_data_kn, name);
> +		kn = kernfs_find_and_get(prgrp->mon.mon_data_kn, name);
> +		if (!kn)
> +			continue;
> +		kernfs_put(kn);
> +
> +		if (kn->dir.subdirs <= 1)
> +			kernfs_remove(kn);
> +		else
> +			kernfs_remove_by_name(kn, subname);

This...

>  
> -		list_for_each_entry(crgrp, &prgrp->mon.crdtgrp_list, mon.crdtgrp_list)
> -			kernfs_remove_by_name(crgrp->mon.mon_data_kn, name);
> +		list_for_each_entry(crgrp, &prgrp->mon.crdtgrp_list, mon.crdtgrp_list) {
> +			kn = kernfs_find_and_get(crgrp->mon.mon_data_kn, name);
> +			if (!kn)
> +				continue;
> +			kernfs_put(kn);
> +
> +			if (kn->dir.subdirs <= 1)
> +				kernfs_remove(kn);
> +			else
> +				kernfs_remove_by_name(kn, subname);
> +		}

... and this are almost identical hunks.

Why isn't there a helper which gets called here?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v23 14/19] x86/resctrl: Handle removing directories in Sub-NUMA Cluster (SNC) mode
Posted by Tony Luck 1 year, 5 months ago
On Tue, Jul 02, 2024 at 10:53:37AM +0200, Borislav Petkov wrote:
> On Fri, Jun 28, 2024 at 02:56:14PM -0700, Tony Luck wrote:
...
> ... and this are almost identical hunks.
> 
> Why isn't there a helper which gets called here?

A helper will avoid this duplication. Draft below. Is this
what you want? Is the function name OK. I wasn't sure if the
"mon" looked better at the beginning. Or as rmdir_one_mon_subdir()

I've also fixed up part 17 to use msr_clear_bit()

Are you still digging through the patches?

-Tony

static void mon_rmdir_one_subdir(struct kernfs_node *pkn, char *name, char *subname)
{
	struct kernfs_node *kn;

	kn = kernfs_find_and_get(pkn, name);
	if (!kn)
		return;
	kernfs_put(kn);

	if (kn->dir.subdirs <= 1)
		kernfs_remove(kn);
	else
		kernfs_remove_by_name(kn, subname);
}

/*
 * Remove all subdirectories of mon_data of ctrl_mon groups
 * and monitor groups for the given domain.
 * Remove files and directories containing "sum" of domain data
 * when last domain being summed is removed.
 */
static void rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
					   struct rdt_mon_domain *d)
{
	struct rdtgroup *prgrp, *crgrp;
	char subname[32];
	bool snc_mode;
	char name[32];

	snc_mode = r->mon_scope == RESCTRL_L3_NODE;
	sprintf(name, "mon_%s_%02d", r->name, snc_mode ? d->ci->id : d->hdr.id);
	if (snc_mode)
		sprintf(subname, "mon_sub_%s_%02d", r->name, d->hdr.id);

	list_for_each_entry(prgrp, &rdt_all_groups, rdtgroup_list) {
		mon_rmdir_one_subdir(prgrp->mon.mon_data_kn, name, subname);

		list_for_each_entry(crgrp, &prgrp->mon.crdtgrp_list, mon.crdtgrp_list)
			mon_rmdir_one_subdir(crgrp->mon.mon_data_kn, name, subname);
	}
}
Re: [PATCH v23 14/19] x86/resctrl: Handle removing directories in Sub-NUMA Cluster (SNC) mode
Posted by Borislav Petkov 1 year, 5 months ago
On Tue, Jul 02, 2024 at 10:16:39AM -0700, Tony Luck wrote:
> A helper will avoid this duplication. Draft below. Is this
> what you want?

Yap, looks good.

> Is the function name OK. I wasn't sure if the
> "mon" looked better at the beginning. Or as rmdir_one_mon_subdir()

It's a static function so the namespace doesn't matter that much, I'd say.

> I've also fixed up part 17 to use msr_clear_bit()

Good.

> Are you still digging through the patches?

Yap, and you can send me only those two which you've modified and I'll
integrate them with the rest and queue them.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette