Re: [RFC] mpam,x86,fs/resctrl: Generic schema description Proof of Concept

[Reinette Chatre]

Hi Chenyu,

On 7/8/26 5:56 AM, Chen, Yu C wrote:
> Hi Reinette,
> 
> On 6/5/2026 5:42 AM, Reinette Chatre wrote:
>> Hi Tony,
>>
>> On 6/3/26 11:46 AM, Luck, Tony wrote:
>>> Reinette,
>>>
>>> Tiny bug in "mpam,x86,fs/resctrl: Transition resource control to a list"
>>>
>>>> /*
>>>>   * Return length needed to display longest control suffix.
>>>>   * Add 1 for the "_" character when control name exists.
>>>>   */
>>>> size_t resctrl_resource_ctrl_max_len(struct rdt_resource *r)
>>>> {
>>>>     struct resctrl_ctrl *ctrl;
>>>>     size_t total = 0;
>>>>     size_t len;
>>>>
>>>>     for_each_resource_ctrl(ctrl,r) {
>>>>         len = strlen(resctrl_ctrl_name_str(ctrl->name));
>>>>         if (len)
>>>>             total += 1 + len;
>>>
>>> Should be:
>>>
>>>             total = max(total, 1 + len);
>>
>> Thank you very much. I squashed this locally.
>>
> 
> A minor alignment schemata issue is found when testing region-aware RDT:
> [resctrl]# cat schemata
>             MB:0=100
>             MB_REGION0_OPT:0=255
>             MB_REGION0_MIN:0=255
>             MB_REGION0_MAX:0=255
>             MB_REGION1_OPT:0=255
>             MB_REGION1_MIN:0=255
>             MB_REGION1_MAX:0=255
>             L2:0=ffff;2=ffff;4=ffff
>             L3:0=3ff
> 
> The schemata file right-justified only the resource name in a
> max_name_width field, then printed the control suffix afterwards
> without padding. Since max_name_width reserves space for name plus
> suffix, this left a run of spaces before every line. A small hacky
> fix:
> 
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index 9b51464b66b7..7d517f3457bf 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -521,6 +521,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
>  static void show_doms(struct seq_file *s, struct rdt_resource_final *f,
>                       bool print_ctrl, int closid, struct resctrl_ctrl *ctrl)
>  {
> +       char name[20];
>         struct rdt_resource *r = f->res;
>         struct rdt_ctrl_domain *dom;
>         bool sep = false;
> @@ -529,11 +530,13 @@ static void show_doms(struct seq_file *s, struct rdt_resource_final *f,
>         /* Walking r->domains, ensure it can't race with cpuhp */
>         lockdep_assert_cpus_held();
> 
> -       if (print_ctrl)
> -               seq_printf(s, "%*s%s%s:", max_name_width, f->name,
> -                               resctrl_ctrl_is_default(ctrl) ? "" : "_",
> -                               resctrl_ctrl_is_default(ctrl) ?
> -                                "" : resctrl_ctrl_name_str(ctrl->name));
> +       if (print_ctrl) {
> +               snprintf(name, sizeof(name), "%s%s%s", f->name,
> +                        resctrl_ctrl_is_default(ctrl) ? "" : "_",
> +                        resctrl_ctrl_is_default(ctrl) ?
> +                         "" : resctrl_ctrl_name_str(ctrl->name));
> +               seq_printf(s, "%*s:", max_name_width, name);
> +       }
>         list_for_each_entry(dom, &ctrl->domains, hdr.list) {
>                 if (sep)
>                         seq_puts(s, ";");
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index c1aac0dcd332..916bf6a8eab4 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -1654,6 +1654,7 @@ static int rdtgroup_size_show(struct kernfs_open_file *of,
>         struct rdtgroup *rdtgrp;
>         struct rdt_resource *r;
>         unsigned int size;
> +       char name[20];
>         u32 ctrl_val;
>         int ret = 0;
>         u32 closid;
> @@ -1693,11 +1694,11 @@ static int rdtgroup_size_show(struct kernfs_open_file *of,
>                 type = f->conf_type;
>                 for_each_resource_ctrl(ctrl, r) {
>                         sep = false;
> -                       seq_printf(s, "%*s", max_name_width, f->name);
> -                       if (!resctrl_ctrl_is_default(ctrl))
> -                               seq_printf(s, "_%s:", resctrl_ctrl_name_str(ctrl->name));
> -                       else
> -                               seq_putc(s, ':');
> +                       snprintf(name, sizeof(name), "%s%s%s", f->name,
> +                                resctrl_ctrl_is_default(ctrl) ? "" : "_",
> +                                resctrl_ctrl_is_default(ctrl) ?
> +                                 "" : resctrl_ctrl_name_str(ctrl->name));
> +                       seq_printf(s, "%*s:", max_name_width, name);
>                         list_for_each_entry(d, &ctrl->domains, hdr.list) {
>                                 if (sep)
>                                         seq_putc(s, ';');
> 
> 
> 
> And it becomes:
> [ resctrl]# cat schemata
>             MB:0=100
> MB_REGION0_OPT:0=255
> MB_REGION0_MIN:0=255
> MB_REGION0_MAX:0=255
> MB_REGION1_OPT:0=255
> MB_REGION1_MIN:0=255
> MB_REGION1_MAX:0=255
>             L2:0=ffff;2=ffff;4=ffff
>             L3:0=3ff
> 
Thank you for the report and the proposed fix. I'll look into making the fix generic.

Reinette