linux-kernel - Re: [PATCH v11 1/8] x86/resctrl: Prepare for new domain scope

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: <f8e46bd8-fc27-4bde-bdbe-daac8ed7f27a@intel.com>
Date:   Mon, 20 Nov 2023 14:16:56 -0800
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghua.yu@...el.com>,
        "Peter Newman" <peternewman@...gle.com>,
        Jonathan Corbet <corbet@....net>,
        "Shuah Khan" <skhan@...uxfoundation.org>, <x86@...nel.org>
CC:     Shaopeng Tan <tan.shaopeng@...itsu.com>,
        James Morse <james.morse@....com>,
        Jamie Iles <quic_jiles@...cinc.com>,
        Babu Moger <babu.moger@....com>,
        Randy Dunlap <rdunlap@...radead.org>,
        <linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <patches@...ts.linux.dev>
Subject: Re: [PATCH v11 1/8] x86/resctrl: Prepare for new domain scope

Hi Tony,

On 11/9/2023 3:09 PM, Tony Luck wrote:
> @@ -556,13 +567,16 @@ static void domain_add_cpu(int cpu, struct rdt_resource *r)
>  
>  static void domain_remove_cpu(int cpu, struct rdt_resource *r)
>  {
> -	int id = get_cpu_cacheinfo_id(cpu, r->cache_level);
> +	int id = get_domain_id_from_scope(cpu, r->scope);
>  	struct rdt_hw_domain *hw_dom;
>  	struct rdt_domain *d;
>  
> +	if (id < 0)
> +		return;
> +

Apologies for this late comment to this code that has been like this for
a few versions. Something must have gone really wrong if id < 0 so I do
not think a silent failure is ideal. Could the same warning as above be
included here? That is:

	pr_warn_once("Can't find domain id for CPU:%d scope:%d for resource %s\n",
		     cpu, r->scope, r->name);


If you agree, please do ensure that this change propagates to
domain_remove_cpu_ctrl() and domain_remove_cpu_mon() in patches that
follow, in which case the text is expected to adjust to "Can't find control
domain id" and "Can't find monitor domain id" respectively. (btw ... is
the switching between "Can't" and "Couldn't" intentional?)

With this change please feel free to include:
Reviewed-by: Reinette Chatre <reinette.chatre@...el.com>

Reinette