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

From: Reinette Chatre
Date: Thu Mar 13 2025 - 11:25:28 EST


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