[PATCH 0/2] fs/resctrl: Fix RMID limbo handling

[PATCH 0/2] fs/resctrl: Fix RMID limbo handling

[Jinjie Ruan]

Fix resctrl RMID allocation to report a retryable -EBUSY
only when the target CLOSID actually has RMIDs in limbo, and split
the domain-offline path out so it releases busy RMIDs without allocating
a monitor context it never uses.

These two fixes are split out of the v10 Narrow-PARTID series below.
	https://lore.kernel.org/all/20261020134239.3921377-1-ruanjinjie@huawei.com/

Jinjie Ruan (2):
  fs/resctrl: Return -EBUSY when per-CLOSID RMIDs are in limbo
  fs/resctrl: Skip monitor context allocation when force-freeing RMIDs

 fs/resctrl/internal.h |  3 +-
 fs/resctrl/monitor.c  | 84 ++++++++++++++++++++++++++++---------------
 fs/resctrl/rdtgroup.c |  2 +-
 3 files changed, 59 insertions(+), 30 deletions(-)

-- 
2.34.1

[PATCH 1/2] fs/resctrl: Return -EBUSY when per-CLOSID RMIDs are in limbo

[Jinjie Ruan]

resctrl_find_free_rmid() returns -ENOSPC when no matching RMID is
found. With CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID the target CLOSID may
have RMIDs in limbo rather than permanently exhausted.

Check closid_num_dirty_rmid[] for the target CLOSID before returning
-ENOSPC, and return -EBUSY so userspace can retry.

Cc: James Morse <james.morse@arm.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Cc: Babu Moger <babu.moger@amd.com>
Fixes: c4c0376eefe1 ("x86/resctrl: Allow RMID allocation to be scoped by CLOSID")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 fs/resctrl/monitor.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index 04e4ea612b7c..b8f4e1ff4083 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -207,9 +207,6 @@ static struct rmid_entry *resctrl_find_free_rmid(u32 closid)
 	struct rmid_entry *itr;
 	u32 itr_idx, cmp_idx;
 
-	if (list_empty(&rmid_free_lru))
-		return rmid_limbo_count ? ERR_PTR(-EBUSY) : ERR_PTR(-ENOSPC);
-
 	list_for_each_entry(itr, &rmid_free_lru, list) {
 		/*
 		 * Get the index of this free RMID, and the index it would need
@@ -225,7 +222,17 @@ static struct rmid_entry *resctrl_find_free_rmid(u32 closid)
 			return itr;
 	}
 
-	return ERR_PTR(-ENOSPC);
+	/*
+	 * No free RMID matched. If this CLOSID has RMIDs in limbo, return
+	 * -EBUSY so userspace can retry. Otherwise, the CLOSID is genuinely
+	 * out of RMIDs. When RMIDs are not per-CLOSID, any RMID in limbo
+	 * delays the allocation.
+	 */
+	if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
+		return closid_num_dirty_rmid[closid] ? ERR_PTR(-EBUSY) :
+						       ERR_PTR(-ENOSPC);
+
+	return rmid_limbo_count ? ERR_PTR(-EBUSY) : ERR_PTR(-ENOSPC);
 }
 
 /**
-- 
2.34.1

Re: [PATCH 1/2] fs/resctrl: Return -EBUSY when per-CLOSID RMIDs are in limbo

[Reinette Chatre]

Hi Jinjie,

Thank you very much for these fixes.

On 10/29/26 1:15 AM, Jinjie Ruan wrote:
> resctrl_find_free_rmid() returns -ENOSPC when no matching RMID is
> found. With CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID the target CLOSID may
> have RMIDs in limbo rather than permanently exhausted.

Just a quick note about customs around resctrl fs and x86 patches that relate to
both of these patches:
  The resctrl patches need to follow the tip requirements documented in
  Documentation/process/maintainer-tip.rst. Of particular note is the "Changelog"
  section that needs fixes to be described in a particular format consisting
  of a context, problem, and solution.
  Apart from this the changelog should avoid narrating the code and instead
  describe it at a higher level to support understanding without needing to
  refer to the code and the impact.

Even though the changelog mentions CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID
it is written for somebody very familiar with the code and it is possible to
state the impact more clearly without needing to go look where
CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID is set. I also think that it would be helpful
to describe all scenarios of the issue to help support the shape of the fix.

As an example that aims to address all comments above, consider a changelog
like below:

  fs/resctrl: Report a temporary RMID shortage per CLOSID

  An RMID that is freed is held in limbo until its cache occupancy drops
  below the threshold, after which it returns to the free list. A
  request that fails because of limbo is thus temporary and worth
  retrying, which resctrl reports to userspace as -EBUSY. A request
  that fails because every RMID is in use is permanent and reported as
  -ENOSPC.

  Where an RMID is only meaningful together with a CLOSID, a request can
  only be satisfied by an RMID belonging to the requested CLOSID, so the
  distinction has to be drawn per CLOSID. Instead it is drawn system
  wide. -ENOSPC is reported when the free list holds no RMID for the
  requested CLOSID, even when that CLOSID has RMIDs in limbo. -EBUSY is
  reported when the free list is empty and another CLOSID has an RMID in
  limbo, even when the requested CLOSID has none. Userspace is thus
  told to give up on a shortage that may clear on its own, and to retry
  one that cannot clear until it removes a group itself.

  Architectures where the RMID is independent of the CLOSID always match
  the first entry on the free list, so neither case arises there and only
  MPAM systems are affected.

  Draw the distinction from the number of RMIDs the requested CLOSID has
  in limbo where the RMID depends on the CLOSID, and from the system
  wide count otherwise. Drop the separate test for an empty free list,
  which the search handles on its own.

> 
> Check closid_num_dirty_rmid[] for the target CLOSID before returning
> -ENOSPC, and return -EBUSY so userspace can retry.
> 
> Cc: James Morse <james.morse@arm.com>
> Cc: Reinette Chatre <reinette.chatre@intel.com>
> Cc: Babu Moger <babu.moger@amd.com>
> Fixes: c4c0376eefe1 ("x86/resctrl: Allow RMID allocation to be scoped by CLOSID")

We should take care here. Even though this "Fixes:" commit added support for
RMID allocation to be scoped by CLOSID this issue only impacts MPAM systems,
and MPAM support landed after this commit.

Do you think this needs a "Cc: stable" tag? Have you heard complaints from
users or is this just something you noticed? Having a benefit to users may
be sufficient justification for adding a stable tag. Accuracy of
"Fixes:" tag is then very important, or the stable tag self can have extra
information on which kernels need it.

> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>

Please re-order the tags based on "Ordering of commit tags" found in
Documentation/process/maintainer-tip.rst

> ---
>  fs/resctrl/monitor.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
Patch looks good to me.

Thank you very much.

Reinette

[PATCH 2/2] fs/resctrl: Skip monitor context allocation when force-freeing RMIDs

[Jinjie Ruan]

__check_limbo() allocates the architecture monitor context before the
scan loop, even when the domain is going offline and the RMIDs will
be freed without reading their occupancy. So if the allocation fails,
the offline path returns early and leaves the busy RMIDs in limbo
which will be leaked instead of being released.

Split out release_limbo_rmids() for the offline path, which doesn't need
the monitor context.

Cc: James Morse <james.morse@arm.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Fixes: fc16126cc11d ("x86,fs/resctrl: Prevent out-of-bounds access while offlining CPU when SNC enabled")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 fs/resctrl/internal.h |  3 +-
 fs/resctrl/monitor.c  | 69 ++++++++++++++++++++++++++++---------------
 fs/resctrl/rdtgroup.c |  2 +-
 3 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
index e62a277dee85..a1837679b27a 100644
--- a/fs/resctrl/internal.h
+++ b/fs/resctrl/internal.h
@@ -405,7 +405,8 @@ 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 __check_limbo(struct rdt_l3_mon_domain *d);
+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 b8f4e1ff4083..020140b794a7 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -124,13 +124,36 @@ static void limbo_release_entry(struct rmid_entry *entry)
 		closid_num_dirty_rmid[entry->closid]--;
 }
 
+/*
+ * Release the busy RMIDs of a domain that is going offline. Their occupancy
+ * can't be read any more, so they are freed without being checked.
+ */
+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)
+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();
@@ -161,30 +184,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 +864,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 57ab090072c2..5d0351453c2f 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -4590,7 +4590,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);
-- 
2.34.1

Re: [PATCH 2/2] fs/resctrl: Skip monitor context allocation when force-freeing RMIDs

[Reinette Chatre]

Hi Jinjie,

There is no "force-freeing" of RMIDs. How about
	fs/resctrl: Do not allocate a monitor context when releasing limbo RMIDs

On 10/29/26 1:15 AM, Jinjie Ruan wrote:
> __check_limbo() allocates the architecture monitor context before the
> scan loop, even when the domain is going offline and the RMIDs will
> be freed without reading their occupancy. So if the allocation fails,
> the offline path returns early and leaves the busy RMIDs in limbo
> which will be leaked instead of being released.

(please separate the context, problem, and solution descriptions).

"will be leaked" is a bit vague to gain confidence that there is an issue here.
Compare with, for example, "The caller goes on to destroy the domain's
monitoring state, freeing the bitmap that records which RMIDs are busy,
so nothing can clear those bits afterwards and the busy counts never reach zero."

I think that there is an argument for this change even in the successful
allocation path. If I understand correctly MPAM's resctrl_arch_mon_ctx_alloc()
may need to block as it waits for a monitor? This would cause the CPU-offline
path to block on a counter that will never be read.

> 
> Split out release_limbo_rmids() for the offline path, which doesn't need
> the monitor context.

Considering the shape of this fix and the "Fixes:" tag I think it may be
worth a mention that this is not a revert of the indicated commit. This change
maintains the intent of the original fix that avoids reading a counter in the
offline path.

> 
> Cc: James Morse <james.morse@arm.com>
> Cc: Reinette Chatre <reinette.chatre@intel.com>
> Fixes: fc16126cc11d ("x86,fs/resctrl: Prevent out-of-bounds access while offlining CPU when SNC enabled")

The changelog is missing a clear description of the impact of this issue.
Nowhere does it mention that this only impacts MPAM and how likely this issue is. 

This is important because this "Fixes:" commit has a "Cc: <stable@kernel.org>" so
we need to help maintainers determine the impact of this issue to 
guide where, if at all, it needs to be backported.

> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
>  fs/resctrl/internal.h |  3 +-
>  fs/resctrl/monitor.c  | 69 ++++++++++++++++++++++++++++---------------
>  fs/resctrl/rdtgroup.c |  2 +-
>  3 files changed, 48 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
> index e62a277dee85..a1837679b27a 100644
> --- a/fs/resctrl/internal.h
> +++ b/fs/resctrl/internal.h
> @@ -405,7 +405,8 @@ 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 __check_limbo(struct rdt_l3_mon_domain *d);

Looks like __check_limbo() can be static now?

> +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 b8f4e1ff4083..020140b794a7 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -124,13 +124,36 @@ static void limbo_release_entry(struct rmid_entry *entry)
>  		closid_num_dirty_rmid[entry->closid]--;
>  }
>  
> +/*
> + * Release the busy RMIDs of a domain that is going offline. Their occupancy
> + * can't be read any more, so they are freed without being checked.

Please document the function self, not the caller. Considering the comments in
resctrl_offline_mon_domain() the caller already does a good job at this.

The "Their occupancy can't be read any more" also does not look right. I think
this claim is based on x86's cpumask_empty() guard in resctrl_arch_rmid_read(), for
x86 that does not actually mean that the occupancy cannot be read anymore, just that
the CPU may already been removed from the domain at that point. This is really
a platform specific detail that resctrl fs cannot assume.

How about something like:

	/*
	 * 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.
	 */

I think such comment pairs well with the comment in the caller - it documents
the consequence that the caller documents it is willing to take.

> + */
> +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)
> +void __check_limbo(struct rdt_l3_mon_domain *d)

Now __check_limbO() can be static?

>  {
>  	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
>  	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
> @@ -161,30 +184,28 @@ void __check_limbo(struct rdt_l3_mon_domain *d, bool force_free)
>  			break;

To be true to the fix I think the rmid_dirty initializer added by fc16126cc11d
can also be removed?
	"bool rmid_dirty = true;" -> "bool rmid_dirty;"

>  
>  		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 +864,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 57ab090072c2..5d0351453c2f 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -4590,7 +4590,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);

Thank you very much.

Reinette