linux-kernel - Re: [PATCH v11 20/31] fs/resctrl: Refactor Sub-NUMA Cluster (SNC) in mkdir/rmdir code flow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ed1a10d2-2f13-43aa-93f8-7dfe8864cf47@intel.com>
Date: Wed, 8 Oct 2025 10:12:36 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: "Luck, Tony" <tony.luck@...el.com>
CC: Fenghua Yu <fenghuay@...dia.com>, Maciej Wieczor-Retman
	<maciej.wieczor-retman@...el.com>, Peter Newman <peternewman@...gle.com>,
	James Morse <james.morse@....com>, Babu Moger <babu.moger@....com>, "Drew
 Fustini" <dfustini@...libre.com>, Dave Martin <Dave.Martin@....com>, Chen Yu
	<yu.c.chen@...el.com>, <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
	<patches@...ts.linux.dev>
Subject: Re: [PATCH v11 20/31] fs/resctrl: Refactor Sub-NUMA Cluster (SNC) in
 mkdir/rmdir code flow

Hi Tony,

On 10/6/25 4:10 PM, Luck, Tony wrote:
> On Fri, Oct 03, 2025 at 04:58:45PM -0700, Reinette Chatre wrote:
>> On 9/25/25 1:03 PM, Tony Luck wrote:

>>> @@ -3253,7 +3267,7 @@ static int mkdir_mondata_subdir(struct kernfs_node *parent_kn,
>>>  		ret = rdtgroup_kn_set_ugid(kn);
>>>  		if (ret)
>>>  			goto out_destroy;
>>> -		ret = mon_add_all_files(kn, hdr, r, prgrp, hdr->id, snc_mode);
>>> +		ret = mon_add_all_files(kn, hdr, r, prgrp, domid, snc_mode);
>>>  		if (ret)
>>>  			goto out_destroy;
>>>  	}
>>
>> mkdir_mondata_subdir(), similar to __mon_event_count(), is now unreasonably
>> complicated. Just like in that earlier change this inconsistently adds 
>> RDT_RESOURCE_L3 checks, not to separate L3 code but instead to benefit PERF_PKG
>> enabling to reach the handful of lines needed by it. 
>> Here too I think the best way forward is to split mkdir_mondata_subdir().
>>
>> rmdir_mondata_subdir_allrdtgrp() may also do with a split ... most of the
>> code within it is dedicated to SNC and mon_rmdir_one_subdir() only exists
>> because of SNC ... any other usage can just call kernfs_remove_by_name(), no?
>>
>> SNC is already complicated enabling and I think that PERF_PKG trying to wedge
>> itself into that is just too confusing. I expect separating this should simplify
>> this a lot.
> 
> Ok. Splitting these makes sense. I'm terrible at naming. So I
> tentatively have:
> 
> static int mkdir_mondata_subdir(struct kernfs_node *parent_kn,
> 				struct rdt_domain_hdr *hdr,
> 				struct rdt_resource *r, struct rdtgroup *prgrp)
> {
> 	lockdep_assert_held(&rdtgroup_mutex);
> 
> 	if (r->mon_scope == RESCTRL_L3_NODE)
> 		return mkdir_mondata_subdir_snc(parent_kn, hdr, r, prgrp);
> 	else
> 		return mkdir_mondata_subdir_normal(parent_kn, hdr, r, prgrp);
> }
> 
> and:
> 
> static void rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
> 					   struct rdt_domain_hdr *hdr)
> {
> 	if (r->mon_scope == RESCTRL_L3_NODE)
> 		rmdir_mondata_subdir_allrdtgrp_snc(r, hdr);
> 	else
> 		rmdir_mondata_subdir_allrdtgrp_normal(r, hdr);
> }
> 
> Better suggestions gratefully accepted.

It is not quite obvious to me how it all will turn out from here with the
addition of support for PERF_PKG. By just considering the above I think
that it helps to match the naming pattern with partners,
for example rmdir_mondata_subdir_allrdtgrp() as you have that matches
mkdir_mondata_subdir_allrdtgrp() that is not listed here. The problem is
that the new rmdir_mondata_subdir_allrdtgrp() is L3 specific while 
mkdir_mondata_subdir_allrdtgrp() remains generic. I thus think that it may make
the code easier to follow if the L3 specific functions have _l3_ in the
name as you established in patch #8. So perhaps above should be
rmdir_l3_mondata_subdir_allrdtgrp() instead and then there may be a new
rmdir_mondata_subdir_allrdtgrp() that will be the new generic function
that calls the resource specific ones?

This could be extended to the new mkdir_mondata_subdir() above where
it is named mkdir_l3_mondata_subdir() called by generic mkdir_mondata_subdir()?

Reinette