Re: [PATCH] fs/resctrl: Fix counter auto-assignment on mkdir with mbm_event enabled

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

Hi Reinette,

On 9/16/2025 4:58 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 9/16/25 10:25 AM, Babu Moger wrote:
>> Found that the automatic counter assignment is not working as expected when
>> "mbm_event" is enabled. Counters are being assigned regardless of whether
>> mbm_assign_on_mkdir is enabled or not.
>>
>> The logic was mistakenly placed in rdtgroup_unassign_cntrs() instead of
>> rdtgroup_assign_cntrs().
>>
>> Fix it by moving the code snippet to rdtgroup_assign_cntrs().
>>
> With the goal to address Boris's concerns about changelogs I think the changelog
> can be made more specific by replacing the vague "the logic" and "the code snippet"
> terms. Below is an example changelog that addresses this but I am afraid that it may
> now be considered too much text :(. As I am still learning how to get this right I
> surely will not hold up the patch because of this, tag is below.
>
> 	rdt_resource::resctrl_mon::mbm_assign_on_mkdir determines if a counter will
> 	automatically be assigned to an RMID, MBM event pair when its associated
> 	monitor group is created via mkdir.
>
> 	Testing shows that counters are always automatically assigned to new monitor
> 	groups,	whether	mbm_assign_on_mkdir is set or not.
>
> 	To support automatic counter assignment the check for mbm_assign_on_mkdir
> 	should be in rdtgroup_assign_cntrs() that assigns counters during monitor group
> 	creation. Instead, the check for mbm_assign_on_mkdir is in rdtgroup_unassign_cntrs()
> 	that is called on monitor group deletion from where counters should always be
> 	unassigned, whether mbm_assign_on_mkdir is set or not.
>
> 	Fix automatic counter assignment by moving the mbm_assign_on_mkdir check from
> 	rdtgroup_unassign_cntrs() to rdtgroup_assign_cntrs().
>
>> Fixes: ef712fe97ec57 ("fs/resctrl: Auto assign counters on mkdir and clean up on group removal")
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
> Thank you very much for catching and fixing this issue.
>
> It is not clear to me if the changelog will be acceptable and I provided alternative
> text just in case. The fix looks good to me, for that:

Looks good to me.

Boris, I will send v2 with updated changelog. Let me know otherwise.

>
> Acked-by: Reinette Chatre <reinette.chatre@intel.com>

Thank you.

Babu