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: <b00fb8d5-d440-4020-8f26-a4c25e00a379@intel.com>
Date: Thu, 20 Jun 2024 14:30:33 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: 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>, Babu Moger
	<babu.moger@....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 13/18] x86/resctrl: Handle removing directories in
 Sub-NUMA Cluster (SNC) mode

Hi Tony,

On 6/10/24 11:35 AM, Tony Luck wrote:
> In SNC mode there are multiple subdirectories in each L3 level monitor
> directory (one for each SNC node). If all the CPUs in an SNC node are
> taken offline, just remove the SNC  directory for that node. In

(nit: watch for random extra spaces)

> non-SNC mode, or when the last SNC node directory is removed, also
> remove the L3 monitor directory.

Perhaps drop the "also" since it is not relevant to non-SNC mode?

> 
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 41 +++++++++++++++++++++-----
>   1 file changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index fc7f3f139800..5142ce43ac13 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -3004,20 +3004,47 @@ static int mon_addfile(struct kernfs_node *parent_kn, const char *name,
>   
>   /*
>    * Remove all subdirectories of mon_data of ctrl_mon groups
> - * and monitor groups with given domain id.
> + * and monitor groups for the given domain.
>    */
>   static void rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
> -					   unsigned int dom_id)
> +					   struct rdt_mon_domain *d)
>   {
>   	struct rdtgroup *prgrp, *crgrp;
> +	struct kernfs_node *kn;
> +	char subname[32];
>   	char name[32];
>   
> +	sprintf(name, "mon_%s_%02d", r->name, d->ci->id);
> +	if (r->mon_scope != RESCTRL_L3_CACHE) {

Same comments about positive check and subtle assignment as in previous
patch.

> +		/*
> +		 * SNC mode: Unless the last domain is being removed must
> +		 * just remove the SNC subdomain.
> +		 */

Can this comment be moved to be part of the top function comments? It is
not relevant to code being commented here and only seems to be here to
avoid duplicating it in the spots where it is relevant.

> +		sprintf(subname, "mon_sub_%s_%02d", r->name, d->hdr.id);
> +	}
> +

Reinette

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ