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@xxxxxxx>
>> ---
>
> ...
>
>> ---
>
> ...
>
>> +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
