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

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

Hi Reinette,

On 10/13/2026 3:25 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 10/13/26 11:52 AM, Babu Moger wrote:
>> On 10/13/26 13:22, Reinette Chatre wrote:
>>> On 10/13/26 9:51 AM, Babu Moger wrote:
>>>> On 10/13/26 10:57, Ben Horgan wrote:
>>>>> On 10/10/2026 00:44, Babu Moger wrote:
>>>>>> 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. Also revise the rdtgroup_assign_cntrs() function
>>>>>> comments to account for the new callers.
>>>>>>
>>>>>> 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
>>>>>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>>>>>> Cc: stable@vger.kernel.org
>>>>>> ---
>>>>>> v4: Changelog update (Reinette's input).
>>>>>>        Function comment update for rdtgroup_assign_cntrs().
>>>>>>
>>>>>> v3: Changelog update.
>>>>>>        Code comment cleanup.
>>>>>>        Added Cc to stable.
>>>>>>        Added Reported-by and Closes tags.
>>>>>>
>>>>>> v2: New patch.
>>>>>>        This patch addresses the Sashiko comment about documentation issue where
>>>>>>        counters are not assigned automatically when mode is switched to mbm_event.
>>>>>>        https://sashiko.dev/#/patchset/8cb66e18e32e4087a9712c1e68ee6da614efe244.1784322818.git.babu.moger%40amd.com
>>>>>>        In fact, it exposed a real issue. When switching to mbm_event mode, existing
>>>>>>        monitoring groups should be assigned counters whenever counters are available.
>>>>>>        This provides a smooth transition between modes and aligns the behavior with
>>>>>>        the existing auto-assignment mechanism.
>>>>>
>>>>>
>>>>> Given the name of the option is 'mbm_assign_on_mkdir' I expected the auto assignment to only occur
>>>>> on the creation of ctrl groups or monitor groups. That's not my concern though. As you are now
>>>>> treating this as more of a general auto assignment should the assignment also be done when a resctrl
>>>>> domain is brought back online.
>>>>>
>>>>> For illustration on an MPAM model I see:
>>>>>
>>>>> /sys/fs/resctrl# cat mbm_L3_assignments
>>>>> mbm_total_bytes:2=e;3=e
>>>>>
>>>>> # offline one domain
>>>>> /sys/fs/resctrl# for i in $(seq 0 3); do echo 0 > /sys/devices/system/cpu/cpu$i/online; done
>>>>> /sys/fs/resctrl# cat mbm_L3_assignments
>>>>> mbm_total_bytes:3=e
>>>>>
>>>>> # bring the domain back online
>>>>> /sys/fs/resctrl# cat mbm_L3_assignments
>>>>> mbm_total_bytes:2=_;3=e # Would it be better, or worse, for auto-assignment to occur here?
>>>>>
>>>> Interesting.
>>>>
>>>> We don't know the previous state. What if it looked like this before the CPU was taken offline?
>>>>
>>>> /sys/fs/resctrl# cat mbm_L3_assignments
>>>> mbm_total_bytes:2=_;3=e
>>>>
>>>> In that case, forcing auto-assignment may not be the right thing to do.
>>>> Since we don't know the previous state, we can only speculate.
>>>>
>>>> It may be best to leave this decision to the user.
>>> I think this raises an important point. To get a better idea I looked back at the original
>>> bug report referenced by the "Closes:" tag and I do not think this is a relevant fix for
>>> that bug.
>>>
>>> Let me elaborate ...
>>>
>>> The bug reported by Sashiko was in response to the patch that added a step to
>>> "Examples on working with mbm_assign_mode" to switch to mbm_event mode from default
>>> mode. The rest of the example was not changed to accommodate the assign mode change
>>> and the documentation thus reflected the state of a system that was in mbm_event mode when
>>> the resource groups were created, not in the new default mode.
>>>
>>> In this new version of the series the documentation change looks different - it does
>>> not imply that the system started out in default mode and thus does not imply any
>>> mode under which those resource groups were created.
>>>
>>> I agree with your final statement "It may be best to leave this decision to the user.".
>>> It is not possible for resctrl to know how user intends to assign counters. Why not just
>>> leave them all unassigned when switching to mbm_event mode?
>>>
>>
>> That is a good point. We can do that.
>>
>> Should we treat the default group as a special case and handle it differently?
> I am missing why the default group is special here. As I understand user space
> can allocate counters to the events belonging to the default group just the same
> as any other group. Could you please elaborate what sets the default group apart here?
> 

As you know, the default group is the "catch-all" container for the 
system, and users may expect it to report valid bandwidth measurements. 
That said, I'm not sure those considerations alone justify giving it 
guaranteed counter assignment.

Thanks,
Babu