[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5f7fa4e8-4bce-4bf4-a557-6993ae5f2e4c@intel.com>
Date: Mon, 30 Oct 2023 14:18:31 -0700
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 v9 1/8] x86/resctrl: Prepare for new domain scope
Hi Tony,
On 10/20/2023 2:30 PM, Tony Luck wrote:
...
> @@ -506,12 +519,17 @@ static int arch_domain_mbm_alloc(u32 num_rmid, struct rdt_hw_domain *hw_dom)
> */
> static void domain_add_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 list_head *add_pos = NULL;
> struct rdt_hw_domain *hw_dom;
> struct rdt_domain *d;
> int err;
>
> + if (id < 0) {
> + pr_warn_once("Can't find domain id for CPU:%d scope:%d for resource %s\n",
> + cpu, r->scope, r->name);
> + return;
> + }
> d = rdt_find_domain(r, id, &add_pos);
> if (IS_ERR(d)) {
> pr_warn("Couldn't find cache id for CPU %d\n", cpu);
>From what I can tell the original implementation relied on implementation of
rdt_find_domain() to do error checking of the id value, printing the above pr_warn()
if id was found to be invalid. In your change the error checking on id is moved
earlier yet this original behavior is maintained. How could rdt_find_domain()
possibly fail for this reason at this point?
> diff --git a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> index 8f559eeae08e..8c5f932bc00b 100644
> --- a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> +++ b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> @@ -292,10 +292,14 @@ static void pseudo_lock_region_clear(struct pseudo_lock_region *plr)
> */
> static int pseudo_lock_region_init(struct pseudo_lock_region *plr)
> {
> + int scope = plr->s->res->scope;
enum resctrl_scope ?
> struct cpu_cacheinfo *ci;
> int ret;
> int i;
Reinette
Powered by blists - more mailing lists