linux-kernel - Re: [PATCH v20 00/18] Add support for Sub-NUMA cluster (SNC) systems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bc52a051-4296-48ac-9a83-29139855553f@intel.com>
Date: Thu, 13 Jun 2024 13:32:10 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: <babu.moger@....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 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/

Reinette