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

[Reinette Chatre <reinette.chatre@intel.com>]

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