linux-kernel - Re: [PATCH v9 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: <4da93c53-54ae-4599-ba40-3ba1cddd5b8e@amd.com>
Date: Fri, 19 Sep 2025 14:38:22 -0500
From: "Moger, Babu" <bmoger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>,
 Babu Moger <babu.moger@....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, kas@...nel.org,
 rick.p.edgecombe@...el.com, akpm@...ux-foundation.org, paulmck@...nel.org,
 pmladek@...e.com, pawan.kumar.gupta@...ux.intel.com, rostedt@...dmis.org,
 kees@...nel.org, arnd@...db.de, fvdl@...gle.com, seanjc@...gle.com,
 thomas.lendacky@....com, manali.shukla@....com, perry.yuan@....com,
 sohil.mehta@...el.com, xin@...or.com, peterz@...radead.org,
 mario.limonciello@....com, gautham.shenoy@....com, nikunj@....com,
 dapeng1.mi@...ux.intel.com, ak@...ux.intel.com, chang.seok.bae@...el.com,
 ebiggers@...gle.com, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-coco@...ts.linux.dev, kvm@...r.kernel.org
Subject: Re: [PATCH v9 07/10] fs/resctrl: Introduce interface to display
 io_alloc CBMs

Hi Reinette,

On 9/18/2025 12:43 AM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 9/2/25 3:41 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.
> 
> (repetitive)
> 
>>
>> 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.
>>
>> The CBM interface file io_alloc_cbm resides in the info directory (e.g.,
>> /sys/fs/resctrl/info/L3/). Since the resource name is part of the path, it
>> is not necessary to display the resource name as done in the schemata file.
>> Pass the resource name to show_doms() and print it only if the name is
>> valid. For io_alloc, pass NULL pointer to suppress printing the resource
>> name.
> 
> Related to changelog feedback received during ABMC series I think the portion
> that describes the code  (from "Pass the resource name ..." to "printing the
> resource name"), is unnecessary since it can be seen by looking at the patch.

Sure.

> 
>>
>> 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 are kept in sync. So, the
> 
> I do not think the "To ensure consistent cache allocation behavior" is accurate.
> This is just to avoid the possible user space confusion if supporting the feature
> with L3CODE used for I/O allocation and L3DATA becomes unusable, no?

Yes. that is correct.

> 
> Also, this also needs to be in imperative tone.
> 
>> Capacity Bit Masks (CBMs) accessed through either L3CODE or L3DATA will
>> reflect identical values.
> 
> Attempt to put it together, please feel free to improve:
> 
> 	Introduce the "io_alloc_cbm" resctrl file to display the Capacity Bit
> 	Masks (CBMs) that represent the portions of each cache instance allocated
> 	for I/O traffic on a cache resource that supports the "io_alloc" feature.
> 
> 	io_alloc_cbm resides in the info directory of a cache resource, for example,
> 	/sys/fs/resctrl/info/L3/. Since the resource name is part of the path, it
> 	is not necessary to display the resource name as done in the schemata file.
> 
> 	When CDP is enabled, io_alloc routes traffic using the highest CLOSID
> 	associated with the L3CODE resource and that CLOSID becomes unusable for
> 	the L3DATA resource. The highest CLOSID of L3CODE and L3DATA resources will
> 	be kept	in sync	to ensure consistent user interface. In preparation for this,
> 	access the CBMs for I/O traffic through highest CLOSID of either L3CODE or
> 	L3DATA resource.
> 

Looks good.

>>
>> Signed-off-by: Babu Moger <babu.moger@....com>
>> ---
> 
> ..
> 
>> @@ -807,3 +809,40 @@ ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
>>   
>>   	return ret ?: nbytes;
>>   }
>> +
>> +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;
> 
> The return code when io_alloc is not enabled is different between reading from (EINVAL) and
> writing to (ENODEV) io_alloc_cbm. Please be consistent.


Will change the return code in patch 9 to -EINVAL to be consistent.

> 
>> +		goto out_unlock;
>> +	}
>> +
>> +	/*
>> +	 * When CDP is enabled, resctrl_io_alloc_init_cbm() sets the same CBM for
>> +	 * both L3CODE and L3DATA of the highest CLOSID. As a result, the io_alloc
> 
> Not just during initialization, they are kept in sync during runtime also (when
> user writes to io_alloc_cbm). First sentence can perhaps just be
> 	"When CDP is enabled the CBMs of the highest CLOSID of L3CODE
> 	 and L3DATA are kept in sync. As a result, ..."
> 

Sure. Thanks
Babu