[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e17d3a79-9c07-44cd-b0ea-2d0545680342@amd.com>
Date: Fri, 14 Jun 2024 11:27:48 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>,
Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghua.yu@...el.com>,
Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>,
Peter Newman <peternewman@...gle.com>, James Morse <james.morse@....com>,
Drew Fustini <dfustini@...libre.com>, Dave Martin <Dave.Martin@....com>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH v20 00/18] Add support for Sub-NUMA cluster (SNC) systems
Hi Reinette,
On 6/13/24 15:32, Reinette Chatre wrote:
> Hi Babu,
>
> On 6/13/24 12:17 PM, Moger, Babu wrote:
>> I may be little bit out of sync here. Also, sorry to come back late in the
>> series.
>>
>> Looking at the series again, I see this approach adds lots of code.
>> Look at this structure.
>>
>>
>> @@ -187,10 +196,12 @@ struct rdt_resource {
>> bool alloc_capable;
>> bool mon_capable;
>> int num_rmid;
>> - enum resctrl_scope scope;
>> + enum resctrl_scope ctrl_scope;
>> + enum resctrl_scope mon_scope;
>> struct resctrl_cache cache;
>> struct resctrl_membw membw;
>> - struct list_head domains;
>> + struct list_head ctrl_domains;
>> + struct list_head mon_domains;
>> char *name;
>> int data_width;
>> u32 default_ctrl;
>>
>> There are two scope fields.
>> There are two domains fields.
>>
>> These are very confusing and very hard to maintain. Also, I am not sure if
>> these fields are useful for anything other than SNC feature. This approach
>> adds quite a bit of code for no specific advantage.
>>
>> Why don't we just split the RDT_RESOURCE_L3 resource
>> into separate resources, one for control, one for monitoring.
>> We already have "control" only resources (MBA, SMBA, L2). Lets create new
>> "monitor" only resource. I feel it will be much cleaner approach.
>>
>> Tony has already tried that approach and showed that it is much simpler.
>>
>> v15-RFC :
>> https://lore.kernel.org/lkml/20240130222034.37181-1-tony.luck@intel.com/
>>
>> What do you think?
>>
>
> Some highlights of my thoughts in response to that series, but the whole
> thread
> may be of interest to you:
> https://lore.kernel.org/lkml/78c88c6d-2e8d-42d1-a6f2-1c5ac38fb258@intel.com/
> https://lore.kernel.org/lkml/59944211-d34a-4ba3-a1de-095822c0b3f0@intel.com/
>
Went through the thread, in summary:
The main concerns are related to duplication of code and data structures.
The solutions are
a) Split the domains.
This is what this series is doing now. This creates members like
ctrl_scope, mon_scope, ctrl_domains etc.. These fields are added to all
the resources (MBA, SMBA and L2). Then there is additional domain header.
b) Split the resource.
Split RDT_RESOURCE_L3 into two, one for "monitor" and one for "control".
There will be one domain structure for "monitor" and one for "control"
Both these approaches have code and data duplication. So, there is no
difference that way.
But complexity wise, approach (a) adds quite bit of complexity. Doesn't it?
For me, solution (b) looks simple and easy. Eventually, we may have to
restructure these data structures anyways. I feel it is the right direction.
--
Thanks
Babu Moger
Powered by blists - more mailing lists