linux-kernel - Re: [PATCH v8 07/10] fs/resctrl: Introduce interface to display io

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: <dbbf40e7-8667-45fe-9de1-8e64e8ea4bdf@amd.com>
Date: Tue, 26 Aug 2025 13:33:12 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, corbet@....net,
 tony.luck@...el.com, Dave.Martin@....com, james.morse@....com,
 tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com
Cc: x86@...nel.org, hpa@...or.com, akpm@...ux-foundation.org,
 paulmck@...nel.org, rostedt@...dmis.org, Neeraj.Upadhyay@....com,
 david@...hat.com, arnd@...db.de, fvdl@...gle.com, seanjc@...gle.com,
 thomas.lendacky@....com, pawan.kumar.gupta@...ux.intel.com,
 yosry.ahmed@...ux.dev, sohil.mehta@...el.com, xin@...or.com,
 kai.huang@...el.com, xiaoyao.li@...el.com, peterz@...radead.org,
 me@...aill.net, mario.limonciello@....com, xin3.li@...el.com,
 ebiggers@...gle.com, ak@...ux.intel.com, chang.seok.bae@...el.com,
 andrew.cooper3@...rix.com, perry.yuan@....com, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, manali.shukla@....com, gautham.shenoy@....com
Subject: Re: [PATCH v8 07/10] fs/resctrl: Introduce interface to display
 io_alloc CBMs

Hi Reinette,

On 8/7/25 20:51, Reinette Chatre wrote:
> Hi Babu,
> 
> On 8/5/25 4:30 PM, Babu Moger wrote:
>> The io_alloc feature in resctrl enables system software to configure
>> the portion of the cache allocated for I/O traffic.
>>
>> Add "io_alloc_cbm" resctrl file to display CBMs (Capacity Bit Mask) of
>> io_alloc feature.
> 
> This is a bit vague. How about:
> 	Add "io_alloc_cbm" resctrl file to display the Capacity Bit Masks
> 	(CBMs) that represent the portion of each cache instance allocated
> 	for I/O traffic.

Sure.

>>
>> The CBM interface file io_alloc_cbm resides in the info directory
>> (e.g., /sys/fs/resctrl/info/L3/). Displaying the resource name is not
>> necessary. Pass the resource name to show_doms() and print it only if
> 
> "Displaying the resource name is not necessary." -> "Since the
> resource name is part of the path it is not necessary to display the
> resource name as done in the schemata file."?

Sure.

> 
> 
>> the name is valid. For io_alloc, pass NULL to suppress printing the
>> resource name.
>>
>> When CDP is enabled, io_alloc routes traffic using the highest CLOSID
>> associated with the L3CODE resource. To ensure consistent cache allocation
>> behavior, the L3CODE and L3DATA resources must remain synchronized.
> 
> "must remain synchronized" -> "are kept in sync"

Sure.

> 
>> rdtgroup_init_cat() function takes both L3CODE and L3DATA into account when
> 
> I do not understand this part. rdtgroup_init_cat() is part of current implementation
> and it takes L3CODE and L3DATE of _other_ CLOSID into account when
> determining what CBM to initialize new CLOSID with. How is that relevant
> here? I wonder if you are not perhaps trying to say:
> "resctrl_io_alloc_init_cbm() initializes L3CODE and L3DATA of highest CLOSID
>  with the same CBM." 
> I do not think this is necessary to include here though since this is what the
> previous patch does and just saying that L3CODE and L3DATA are kept in sync is
> sufficient here.

Ok. Sounds good.
> 
>> initializing CBMs for new groups.  The io_alloc feature adheres to this
>> same principle, meaning that the Cache Bit Masks (CBMs) accessed through
>> either L3CODE or L3DATA will reflect identical values.
> 
> I do not understand what you are trying to say here. What do you mean with
> "same principle"? The fact that L3CODE and L3DATA are kept in sync is
> part of io_alloc only, no?

Yes. That is correct. I will remove that text.

> 
>>
>> Signed-off-by: Babu Moger <babu.moger@....com>
>> ---
> 
> ...
> 
>> ---
> 
> ...
> 
>> +int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq, void *v)
>> +{
>> +	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
>> +	struct rdt_resource *r = s->res;
>> +	int ret = 0;
>> +
>> +	cpus_read_lock();
>> +	mutex_lock(&rdtgroup_mutex);
>> +
>> +	rdt_last_cmd_clear();
>> +
>> +	if (!r->cache.io_alloc_capable) {
>> +		rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
>> +		ret = -ENODEV;
>> +		goto out_unlock;
>> +	}
>> +
>> +	if (!resctrl_arch_get_io_alloc_enabled(r)) {
>> +		rdt_last_cmd_printf("io_alloc is not enabled on %s\n", s->name);
>> +		ret = -EINVAL;
>> +		goto out_unlock;
>> +	}
>> +
> 
> Could you please add a comment here that explains to the reader that CBMs of
> L3CODE and L3DATA are kept in sync elsewhere and the io_alloc CBMs displayed from
> either CDP resource are thus identical and accurately reflect the CBMs used
> for I/O.

Sure.

> 
>> +	show_doms(seq, s, NULL, resctrl_io_alloc_closid(r));
>> +
>> +out_unlock:
>> +	mutex_unlock(&rdtgroup_mutex);
>> +	cpus_read_unlock();
>> +	return ret;
>> +}
> 
> Reinette
> 

-- 
Thanks
Babu Moger