[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6778a8af-5312-419e-a064-bcb6a495a207@intel.com>
Date: Thu, 9 Oct 2025 13:29:43 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: "Luck, Tony" <tony.luck@...el.com>
CC: Fenghua Yu <fenghuay@...dia.com>, "Wieczor-Retman, Maciej"
<maciej.wieczor-retman@...el.com>, Peter Newman <peternewman@...gle.com>,
James Morse <james.morse@....com>, Babu Moger <babu.moger@....com>, "Drew
Fustini" <dfustini@...libre.com>, Dave Martin <Dave.Martin@....com>, "Chen,
Yu C" <yu.c.chen@...el.com>, "x86@...nel.org" <x86@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"patches@...ts.linux.dev" <patches@...ts.linux.dev>
Subject: Re: [PATCH v11 20/31] fs/resctrl: Refactor Sub-NUMA Cluster (SNC) in
mkdir/rmdir code flow
Hi Tony,
On 10/9/25 10:45 AM, Luck, Tony wrote:
> On Wed, Oct 08, 2025 at 07:16:07PM -0700, Reinette Chatre wrote:
>> Understood. This is not about correctness but making the code easier to understand.
>> What I am aiming for is consistency in the code where the pattern
>> in existing flows use the resource ID as check to direct code flow to resource
>> specific code. In the above flow it uses the monitoring scope. This works of course,
>> but it is an implicit check because the L3 resource is the only one that currently
>> supports the "node" scope and does so when SNC is enabled.
>> My preference is for the code to be consistent in patterns used and find doing so
>> makes the code easier to read and understand.
>>
> Reinette,
>
> Should I address this "only one that currently" issue now? Maybe by
> adding a bool "rdt_resource:snc_mode" so the implicit
>
> if (r->mon_scope == RESCTRL_L3_NODE)
>
> changes to:
>
> if (r->snc_mode)
>
> There are only two places where SNC mode is checked in this way. The
> others rely on seeing that mon_data::sum is set, or that rr->hdr is
> NULL. So it seems like a very small improvement.
This is not about SNC mode or not but instead about this code being L3
resource specific.
I see the mon_data::sum and rr->hdr checks as supporting a separate
feature that was introduced to support SNC - it should not be used as
a check for SNC support even though it currently implies this due to SNC
being the only user. Could we not, hypothetically, even use these properties
in the region aware MBM work?
> If we ever add a node scoped resource that isn't related to SNC, it
> would be needed at that point. But I'm not sure why hardware would
> ever do that.
Right. This is not about just what is needed to enable this feature but
about making the code easy to follow for those that attempt to understand,
debug, and/or build on top.
Reinette
Powered by blists - more mailing lists