Re: [PATCH v3 2/3] fs/resctrl: Assign counters to existing groups when enabling mbm_event

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Babu,

On 10/2/26 2:26 PM, Babu Moger wrote:
> When the user enables counter assignment mode by writing "mbm_event"
> to /sys/fs/resctrl/info/L3_MON/mbm_assign_mode, resctrl resets all
> monitoring state and sets mbm_assign_on_mkdir for subsequent mkdir, but
> does not assign counters to groups that already exist, including the
> default group created at mount. The counters of those groups return
> "Unassigned" until the user assigns counters by hand.

Above summarizes the context and then below follows with the change,
what makes this deserve a "Fixes:" tag? What is the issue needing fixing?


Consider, for example:

      When the user switches counter assignment mode by writing
      "mbm_event" to /sys/fs/resctrl/info/L3_MON/mbm_assign_mode,
      resctrl frees all assignable counters, resets per-domain RMID
      state, and enables counter auto-assignment exposed to user space
      as mbm_assign_on_mkdir.

      Even though counter auto-assignment is enabled, groups that
      already exist at the time of the switch, including the default
      group created at mount, are not assigned a counter. All MBM
      events read "Unassigned" until the user assigns a counter by hand.

      Walk every existing CTRL_MON and its MON children after the reset
      and assign counters to their MBM events. Enabling "mbm_event" now
      leaves the same per-group state that auto-assignment would have
      produced. There may be fewer available counters than MBM events
      across the existing groups; in that case stop assignment when no
      counters remain. Events in the remaining groups read "Unassigned",
      matching the behavior of creating a group when no counters are
      available.

> 
> Enable mbm_assign_on_mkdir and assign counters, while there are some
> available, to existing CTRL_MON and MON groups so the switch matches
> mkdir auto-assignment. An event left without a counter reads
> "Unassigned".
> 
> Fixes: 8004ea01cf63 ("fs/resctrl: Introduce the interface to switch between monitor modes")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/8cb66e18e32e4087a9712c1e68ee6da614efe244.1784322818.git.babu.moger%40amd.com
> Cc: stable@vger.kernel.org

tip tag order places "Cc:" after author's SoB.

> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---


> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> index 73413cb128ea..fca0734bf346 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -1300,8 +1300,7 @@ static int rdtgroup_assign_cntr_event(struct rdt_l3_mon_domain *d, struct rdtgro
>  }
>  
>  /*
> - * rdtgroup_assign_cntrs() - Assign counters to MBM events. Called when
> - *			     a new group is created.
> + * rdtgroup_assign_cntrs() - Assign counters to MBM events.

This patch adds another caller of rdtgroup_assign_cntrs() and above thus correctly
updates the function comments to not just mention the original caller ...

>   *
>   * Each group can accommodate two counters per domain: one for the total
>   * event and one for the local event. Assignments may fail due to the limited

... but the detailed function comments trimmed here still considers original
caller as the only use case and uses original caller to justify why it does not
need to return an error. 

Reinette