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

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

[Babu Moger]

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().

Fixes: ef712fe97ec57 ("fs/resctrl: Auto assign counters on mkdir and clean up on group removal")
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
This issue was introduced during the rebase from v17 [1] to v18 [2].
https://lore.kernel.org/lkml/1ee0f8674f0ab48bdbc3e05c11b7df30d6fa53fe.1755224735.git.babu.moger@amd.com/ # [1]
https://lore.kernel.org/lkml/db4240e3d815c3f193402b36723995427ec358b0.1757108044.git.babu.moger@amd.com/ # [2]
---
 fs/resctrl/monitor.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index 50c24460d992..4076336fbba6 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -1200,7 +1200,8 @@ void rdtgroup_assign_cntrs(struct rdtgroup *rdtgrp)
 {
 	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
 
-	if (!r->mon_capable || !resctrl_arch_mbm_cntr_assign_enabled(r))
+	if (!r->mon_capable || !resctrl_arch_mbm_cntr_assign_enabled(r) ||
+	    !r->mon.mbm_assign_on_mkdir)
 		return;
 
 	if (resctrl_is_mon_event_enabled(QOS_L3_MBM_TOTAL_EVENT_ID))
@@ -1258,8 +1259,7 @@ void rdtgroup_unassign_cntrs(struct rdtgroup *rdtgrp)
 {
 	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
 
-	if (!r->mon_capable || !resctrl_arch_mbm_cntr_assign_enabled(r) ||
-	    !r->mon.mbm_assign_on_mkdir)
+	if (!r->mon_capable || !resctrl_arch_mbm_cntr_assign_enabled(r))
 		return;
 
 	if (resctrl_is_mon_event_enabled(QOS_L3_MBM_TOTAL_EVENT_ID))
-- 
2.34.1

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

[Reinette Chatre]

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:

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

Reinette

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

[Moger, Babu]

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

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

[Borislav Petkov]

On Tue, Sep 16, 2025 at 05:46:56PM -0500, Moger, Babu wrote:
> > 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.

No need, lemme simply do that now.

Reinette, your version is exactly the structure one would like to read: first
you set up what the behavioral expectation is, and then you explain what the
issue is - i.e., what's wrong.

Good.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Moger, Babu]

Hi Boris,

On 9/17/2025 4:26 AM, Borislav Petkov wrote:
> On Tue, Sep 16, 2025 at 05:46:56PM -0500, Moger, Babu wrote:
>>> 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.
> No need, lemme simply do that now.

Thank you

Babu

[tip: x86/cache] fs/resctrl: Fix counter auto-assignment on mkdir with mbm_event enabled

[tip-bot2 for Babu Moger]

The following commit has been merged into the x86/cache branch of tip:

Commit-ID:     dd86b69d20fb9fa7e941ed01ff05f1e662fcc3ff
Gitweb:        https://git.kernel.org/tip/dd86b69d20fb9fa7e941ed01ff05f1e662fcc3ff
Author:        Babu Moger <babu.moger@amd.com>
AuthorDate:    Tue, 16 Sep 2025 12:25:49 -05:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Wed, 17 Sep 2025 11:31:12 +02:00

fs/resctrl: Fix counter auto-assignment on mkdir with mbm_event enabled

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().

  [ bp: Replace commit message with Reinette's version. ]

Fixes: ef712fe97ec57 ("fs/resctrl: Auto assign counters on mkdir and clean up on group removal")
Signed-off-by: Babu Moger <babu.moger@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Acked-by: Reinette Chatre <reinette.chatre@intel.com>
---
 fs/resctrl/monitor.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index 50c2446..4076336 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -1200,7 +1200,8 @@ void rdtgroup_assign_cntrs(struct rdtgroup *rdtgrp)
 {
 	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
 
-	if (!r->mon_capable || !resctrl_arch_mbm_cntr_assign_enabled(r))
+	if (!r->mon_capable || !resctrl_arch_mbm_cntr_assign_enabled(r) ||
+	    !r->mon.mbm_assign_on_mkdir)
 		return;
 
 	if (resctrl_is_mon_event_enabled(QOS_L3_MBM_TOTAL_EVENT_ID))
@@ -1258,8 +1259,7 @@ void rdtgroup_unassign_cntrs(struct rdtgroup *rdtgrp)
 {
 	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
 
-	if (!r->mon_capable || !resctrl_arch_mbm_cntr_assign_enabled(r) ||
-	    !r->mon.mbm_assign_on_mkdir)
+	if (!r->mon_capable || !resctrl_arch_mbm_cntr_assign_enabled(r))
 		return;
 
 	if (resctrl_is_mon_event_enabled(QOS_L3_MBM_TOTAL_EVENT_ID))