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

[Babu Moger <babu.moger@amd.com>]

Hi Reinette,

On 10/13/26 13:22, Reinette Chatre wrote:
> Hi Babu,
> 
> 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?

Thanks,
Babu