Re: [PATCH v10 RESEND 02/15] fs/resctrl: Skip monitor context allocation when force-freeing RMIDs

[Jinjie Ruan]

在 2026/10/26 22:11, Ben Horgan 写道:
> Hi Jinjie,
> 
> On 26/10/2026 14:08, Ben Horgan wrote:
>> Hi Jinjie,
>>
>> On 20/10/2026 14:42, Jinjie Ruan wrote:
>>> __check_limbo() allocates the architecture monitor context before the
>>> scan loop, even when force_free is set. The force-free path never reads
>>> occupancy and never uses the context.
>>>
>>> If the allocation fails, the function returns early and leaves the limbo
>>> list unflushed. Callers such as the unmount path then silently fail to
>>> return the RMIDs to the free list, leaving stale CLOSIDs in limbo that
>>> are re-interpreted with the CDP state of the next mount.
>>
>> Is this a theoretical fix or is there some way that resctrl_arch_mon_ctx_alloc() can fail in practice?
> 
> This does seem a good change to me and we do have to consider that the memory alocation might fail.

Hi Ben,

I agree. force_free=true not rely on the allocation of arch_mon_ctx
succeeding, we should directly loop to free the RMID.

Perhaps a small refactor is needed to split __check_limbo() into two
versions as there isn't much duplication?

> 
> Thanks,
> 
> Ben
> 
>>
>> Thanks,
>>
>> Ben
>>
>>>
>>> Only allocate and free the monitor context when occupancy will be read.
>>>
>>> Fixes: fa7d949337cc ("x86/resctrl: Rename and move rdt files to a separate directory")
>>> Cc: James Morse <james.morse@xxxxxxx>
>>> Cc: Reinette Chatre <reinette.chatre@xxxxxxxxx>
>>> Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
>>> ---
>>>  fs/resctrl/monitor.c | 15 +++++++++------
>>>  1 file changed, 9 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
>>> index da883adc3adc..ffa9c67c9f00 100644
>>> --- a/fs/resctrl/monitor.c
>>> +++ b/fs/resctrl/monitor.c
>>> @@ -142,11 +142,13 @@ void __check_limbo(struct rdt_l3_mon_domain *d, bool force_free)
>>>  	u64 val = 0;
>>>  
>>>  	arch_priv = mon_event_all[QOS_L3_OCCUP_EVENT_ID].arch_priv;
>>> -	arch_mon_ctx = resctrl_arch_mon_ctx_alloc(r, QOS_L3_OCCUP_EVENT_ID);
>>> -	if (IS_ERR(arch_mon_ctx)) {
>>> -		pr_warn_ratelimited("Failed to allocate monitor context: %pe",
>>> -				    arch_mon_ctx);
>>> -		return;
>>> +	if (!force_free) {
>>> +		arch_mon_ctx = resctrl_arch_mon_ctx_alloc(r, QOS_L3_OCCUP_EVENT_ID);
>>> +		if (IS_ERR(arch_mon_ctx)) {
>>> +			pr_warn_ratelimited("Failed to allocate monitor context: %pe",
>>> +					    arch_mon_ctx);
>>> +			return;
>>> +		}
>>>  	}
>>>  
>>>  	/*
>>> @@ -192,7 +194,8 @@ void __check_limbo(struct rdt_l3_mon_domain *d, bool force_free)
>>>  		cur_idx = idx + 1;
>>>  	}
>>>  
>>> -	resctrl_arch_mon_ctx_free(r, QOS_L3_OCCUP_EVENT_ID, arch_mon_ctx);
>>> +	if (!force_free)
>>> +		resctrl_arch_mon_ctx_free(r, QOS_L3_OCCUP_EVENT_ID, arch_mon_ctx);
>>>  }
>>>  
>>>  bool has_busy_rmid(struct rdt_l3_mon_domain *d)
>>
> 

-- 
Best regards,
Jinjie