Re: [PATCH] fs/resctrl: Do not allocate a monitor context when releasing limbo RMIDs

[Jinjie Ruan]

Please ignore this patch.

在 2026/10/30 17:21, Jinjie Ruan 写道:
> Busy RMIDs of a monitor domain are normally released by the limbo
> handler once their cache occupancy drops below the threshold. When a
> domain goes offline, its busy RMIDs are released without checking
> occupancy, since the domain is going away and the RMIDs must not be
> left behind.
> 
> On the monitor domain offline path, __check_limbo() allocates
> the monitor context that an occupancy check needs before it scans
> the busy RMIDs, but without using it, and returns early when that
> allocation fails. Then the caller destroys the domain's monitoring state,
> including the bitmap that records which RMIDs are busy, so nothing can
> clear those bits afterwards and the busy counts never reach zero. Those
> RMIDs never return to the free list and, where the RMID depends on
> the CLOSID, the dirty count of their CLOSID never decreases. The allocation
> can also wait for a monitor to become free, which stalls the CPU offline
> path on a context that is not going to be used.
> 
> Only MPAM systems are affected. An architecture that can always read an
> RMID allocates nothing for the monitor context, so the allocation can
> neither fail nor wait there.
> 
> So move the release of busy RMIDs into a separate function that does
> not take a monitor context and leave the occupancy check to
> __check_limbo().
> 
> This is not a revert of the fixed commit. It preserves the original fix's
> intent of not reading a counter in the offline path.
> 
> Fixes: fc16126cc11d ("x86,fs/resctrl: Prevent out-of-bounds access while offlining CPU when SNC enabled")
> Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
> Cc: James Morse <james.morse@xxxxxxx>
> Cc: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
> ---
>  fs/resctrl/internal.h |  2 +-
>  fs/resctrl/monitor.c  | 72 ++++++++++++++++++++++++++++---------------
>  fs/resctrl/rdtgroup.c |  2 +-
>  3 files changed, 49 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
> index e62a277dee85..1c85e06f09bf 100644
> --- a/fs/resctrl/internal.h
> +++ b/fs/resctrl/internal.h
> @@ -405,7 +405,7 @@ void cqm_handle_limbo(struct work_struct *work);
>  
>  bool has_busy_rmid(struct rdt_l3_mon_domain *d);
>  
> -void __check_limbo(struct rdt_l3_mon_domain *d, bool force_free);
> +void release_limbo_rmids(struct rdt_l3_mon_domain *d);
>  
>  void resctrl_file_fflags_init(const char *config, unsigned long fflags);
>  
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> index 88674e28b2f8..3c9f65888004 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -124,18 +124,42 @@ static void limbo_release_entry(struct rmid_entry *entry)
>  		closid_num_dirty_rmid[entry->closid]--;
>  }
>  
> +/*
> + * Release the RMIDs that are marked as busy for this domain without
> + * checking their LLC occupancy. Cache lines may still be tagged with
> + * these RMIDs and will be accounted to the next user of each RMID.
> + */
> +void release_limbo_rmids(struct rdt_l3_mon_domain *d)
> +{
> +	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
> +	struct rmid_entry *entry;
> +	u32 idx, cur_idx = 1;
> +
> +	for (;;) {
> +		idx = find_next_bit(d->rmid_busy_llc, idx_limit, cur_idx);
> +		if (idx >= idx_limit)
> +			break;
> +
> +		entry = __rmid_entry(idx);
> +		clear_bit(idx, d->rmid_busy_llc);
> +		if (!--entry->busy)
> +			limbo_release_entry(entry);
> +		cur_idx = idx + 1;
> +	}
> +}
> +
>  /*
>   * Check the RMIDs that are marked as busy for this domain. If the
>   * reported LLC occupancy is below the threshold clear the busy bit and
>   * decrement the count. If the busy count gets to zero on an RMID, we
>   * free the RMID
>   */
> -void __check_limbo(struct rdt_l3_mon_domain *d, bool force_free)
> +static void __check_limbo(struct rdt_l3_mon_domain *d)
>  {
>  	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
>  	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
>  	struct rmid_entry *entry;
> -	bool rmid_dirty = true;
> +	bool rmid_dirty;
>  	u32 idx, cur_idx = 1;
>  	void *arch_mon_ctx;
>  	void *arch_priv;
> @@ -161,30 +185,28 @@ void __check_limbo(struct rdt_l3_mon_domain *d, bool force_free)
>  			break;
>  
>  		entry = __rmid_entry(idx);
> -		if (!force_free) {
> -			if (resctrl_arch_rmid_read(r, &d->hdr, entry->closid,
> -						   entry->rmid, QOS_L3_OCCUP_EVENT_ID,
> -						   arch_priv, &val, arch_mon_ctx)) {
> -				rmid_dirty = true;
> -			} else {
> -				rmid_dirty = (val >= resctrl_rmid_realloc_threshold);
> -
> -				/*
> -				 * x86's CLOSID and RMID are independent numbers,
> -				 * so the entry's CLOSID is an empty CLOSID
> -				 * (X86_RESCTRL_EMPTY_CLOSID). On Arm the RMID
> -				 * (PMG) extends the CLOSID (PARTID) space with
> -				 * bits that aren't used to select the configuration.
> -				 * It is thus necessary to track both CLOSID and
> -				 * RMID because there may be dependencies between
> -				 * them on some architectures.
> -				 */
> -				trace_mon_llc_occupancy_limbo(entry->closid, entry->rmid,
> -							      d->hdr.id, val);
> -			}
> +		if (resctrl_arch_rmid_read(r, &d->hdr, entry->closid,
> +					   entry->rmid, QOS_L3_OCCUP_EVENT_ID,
> +					   arch_priv, &val, arch_mon_ctx)) {
> +			rmid_dirty = true;
> +		} else {
> +			rmid_dirty = (val >= resctrl_rmid_realloc_threshold);
> +
> +			/*
> +			 * x86's CLOSID and RMID are independent numbers,
> +			 * so the entry's CLOSID is an empty CLOSID
> +			 * (X86_RESCTRL_EMPTY_CLOSID). On Arm the RMID
> +			 * (PMG) extends the CLOSID (PARTID) space with
> +			 * bits that aren't used to select the configuration.
> +			 * It is thus necessary to track both CLOSID and
> +			 * RMID because there may be dependencies between
> +			 * them on some architectures.
> +			 */
> +			trace_mon_llc_occupancy_limbo(entry->closid, entry->rmid,
> +						      d->hdr.id, val);
>  		}
>  
> -		if (force_free || !rmid_dirty) {
> +		if (!rmid_dirty) {
>  			clear_bit(idx, d->rmid_busy_llc);
>  			if (!--entry->busy)
>  				limbo_release_entry(entry);
> @@ -843,7 +865,7 @@ void cqm_handle_limbo(struct work_struct *work)
>  	if (cpumask_empty(&d->hdr.cpu_mask))
>  		goto out_unlock;
>  
> -	__check_limbo(d, false);
> +	__check_limbo(d);
>  
>  	if (has_busy_rmid(d)) {
>  		d->cqm_work_cpu = cpumask_any_housekeeping(&d->hdr.cpu_mask,
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 5dcbb0a964e8..20c800e678d5 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -4592,7 +4592,7 @@ void resctrl_offline_mon_domain(struct rdt_resource *r, struct rdt_domain_hdr *h
>  		 * the RMID as busy creates RMID leaks if the
>  		 * package never comes back.
>  		 */
> -		__check_limbo(d, true);
> +		release_limbo_rmids(d);
>  	}
>  
>  	domain_destroy_l3_mon_state(d);

-- 
Best regards,
Jinjie