Re: [PATCH v12 06/25] arm,x86,fs/resctrl: Replace architecture resctrl_arch_{alloc,mon}_capable()

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

Hi Tony,

On 9/25/26 3:14 PM, Luck, Tony wrote:
> On Thu, Sep 24, 2026 at 08:22:47AM -0700, Reinette Chatre wrote:
>> Hi Tony,
>>
>> On 9/16/26 4:13 PM, Tony Luck wrote:
>>> All the information to determine whether a system is alloc, or mon,
>>> capable is available to the file system. Architecture helpers are not
>>
>> nit: "is alloc, or mon, capable" -> "is capable of allocation or monitoring"?
>>
>>> needed.
>>>
>>> Define generic resctrl_alloc_capable() and resctrl_mon_capable() and
>>> replace all architecture versions.
>>>
>>> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
>>> Signed-off-by: Tony Luck <tony.luck@intel.com>
>>> ---
>>
>> Please note that the pending RISC-V enabling [1] is not compatible with this
>> change but there is a change planned that will let resctrl_alloc_capable()
>> introduced here work on RISC-V also. We need to coordinate these two
>> series.
> 
> The first few patches in this series have relatively simple cleanups
> suggested. Perhaps when I post v13 you could focus on getting these to
> the finish line and have Boris consider them for inclusion.

I was hoping that me rewriting so many changelogs entirely for you would
already make it clear that I am already trying to get this over the finish
line. 

Alas, me writing the changelogs for you seems to be the wrong strategy because,
as the new changelog below shows, the points I keep making about changelogs
appear to end up in /dev/null.

> That would get this change into TIP x86/cache and let the RISC-V
> folks move forward.
> 
> Note there will be one new patch in the initial sequence. The bug
> fix for is_closid_match() that you suggested I separate out from
> patch 9.
> 
> Draft version of new patch below.
> 
>>
>> | Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
>>
>> Reinette
>>
>> [1] https://lore.kernel.org/lkml/20260917-dfustini-atl-sc-cbqri-dt-v8-2-7964e8d73fe8@kernel.org/
> 
> -Tony
> 
> commit 986bf1a4e789de917e955fe549bdd71e51dc2e48
> Author: Tony Luck <tony.luck@intel.com>
> Date:   Thu Sep 24 12:40:41 2026 -0700
> 
>     fs/resctrl: Ensure default group reports tasks on monitor-only systems
>     
>     Sashiko reported[1]:

The custom for this is to use tags. For this patch it would be:	
	Closes: https://sashiko.dev/#/patchset/20260831174421.13921-1-tony.luck%40intel.com?part=9
	Reported-by: Sashiko <sashiko-bot@kernel.org>

>     
>       This is a pre-existing issue, but the resctrl_alloc_capable() check in
>       is_closid_match() actively breaks the default group on systems that only
>       support monitoring capabilities.
>       When a user reads the root /sys/fs/resctrl/tasks file on a system with
>       monitoring capabilities but no allocation capabilities, the following call
>       chain occurs:
>       rdtgroup_tasks_show()
>         show_rdt_tasks()
>           is_closid_match()
>       Since resctrl_alloc_capable() evaluates to false on such systems,
>       is_closid_match() unconditionally returns false for all tasks. Furthermore,
>       because the default group has type RDTCTRL_GROUP, is_rmid_match() will also
>       return false.
>       This causes the root tasks file to appear completely empty, hiding all tasks
>       on the system that have not been explicitly moved to a monitoring group.

Please separate paragraphs with empty lines. It is easier to read.

>     
>     Remove the resctrl_alloc_capable() check from is_closid_match().

This is obvious from the patch. <sigh>. Please do not just verbatim describe the code
in the changelog.  

Could you instead describe why this check is removed? I understand that the bug
description already mentions that it "actively breaks the default group" but it
highlights that this only happens on systems that only support monitoring capabilities.

Could the description of the fix describe *why* removing it is sufficient
to address the reported issue without breaking anything else?

>     
>     Fixes: e6b2fac36fcc ("x86/resctrl: Use is_closid_match() in more places")
>     Suggested-by: Reinette Chatre <reinette.chatre@intel.com>

I do not recall suggesting this fix.

>     Signed-off-by: Tony Luck <tony.luck@intel.com>
>     Link: https://sashiko.dev/#/patchset/20260831174421.13921-1-tony.luck%40intel.com?part=9 # [1]
>     ---
>     v13:
>             New patch (split from patch 9)
> 
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 5dcbb0a964e8..6733ed174f51 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -685,7 +685,7 @@ static int __rdtgroup_move_task(struct task_struct *tsk,
>  
>  static bool is_closid_match(struct task_struct *t, struct rdtgroup *r)
>  {
> -	return (resctrl_arch_alloc_capable() && (r->type == RDTCTRL_GROUP) &&
> +	return (r->type == RDTCTRL_GROUP &&
>  		resctrl_arch_match_closid(t, r->closid));
>  }
>  

Reinette