Re: [PATCH v7 37/49] x86/resctrl: Expand the width of dom_id by replacing mon_data_bits

[Reinette Chatre]

Hi James,

On 3/12/25 11:04 AM, James Morse wrote:
> On 07/03/2025 05:03, Reinette Chatre wrote:
>> On 2/28/25 11:59 AM, James Morse wrote:

...

>> With all of the above I do not think this will work on an SNC enabled
>> system ... to confirm this I tried it out and it is not possible to mount
>> resctrl on an SNC enabled system and the WARN_ON_ONCE() this patch adds to
>> mon_add_all_files() is hit.
> 
> I hadn't realised the mon_sub directories for SNC weren't all directly under mon_data.
> Searching from mon_data will need the parent name too. What I've come up with is:
> -------%<-------
> 	snc_mode = r->mon_scope == RESCTRL_L3_NODE;
> 	if (!snc_mode) {
> 		sprintf(name, "mon_%s_%02d", r->name, d->hdr.id);
> 		kn_target_dir = kernfs_find_and_get(kn_mondata, name);
> 	} else {
> 		sprintf(name, "mon_%s_%02d", r->name, d->ci->id);
> 		kn_target_dir = kernfs_find_and_get(kn_mondata, name);
> 
> 		if (snc_mode && !do_sum) {

snc_mode should always be true here?

> 			sprintf(name, "mon_sub_%s_%02d", r->name, d->hdr.id);
> 			kernfs_put(kn_target_dir);

I think this needs some extra guardrails. If kn_target_dir is NULL here
it looks like that the kernfs_put() above will be fine, but from what I can tell
the kernfs_find_and_get() below will not be.

> 			kn_target_dir = kernfs_find_and_get(kn_target_dir, name);
> 		}
> 	}
> 	kernfs_put(kn_target_dir);
> 	if (!kn_target_dir)
> 		return NULL;
> -------%<-------
> 

This looks good to me. In original patch a NULL kn within mon_get_default_kn_priv()
was used as prompt to create the private data. It is thus not obvious to me from this
snippet what is being returned "to", but I do not think that was your point of sharing
this snippet. 

Reinette