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 6:23 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 10/13/26 4:05 PM, Moger, Babu wrote:
>> On 10/13/2026 3:25 PM, Reinette Chatre wrote:
>>> 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.
> 
> The current documentation states following about impact of switching to mbm_event mode:
> - "Changing the mode may cause all counters on the resource to reset."
> - "Moving to mbm_event counter assignment mode requires users to assign the counters
>     to the events. Otherwise, the MBM event counters will return 'Unassigned' when read."
> 
> The second is clear about expected behavior and there is no "except default group" caveat.
> 
> At this point I do not see a motivation to change the existing behavior.
> 
> My vote is for this patch to be dropped.

Sure. Will drop this patch.

Thank you,
Babu