Re: [PATCH v9 07/10] fs/resctrl: Introduce interface to display io_alloc CBMs

["Moger, Babu" <bmoger@amd.com>]

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@amd.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