Messages in this thread | | | Date | Mon, 20 Nov 2023 14:19:04 -0800 | Subject | Re: [PATCH v11 3/8] x86/resctrl: Prepare for different scope for control/monitor operations | From | Reinette Chatre <> |
| |
Hi Tony,
On 11/9/2023 3:09 PM, Tony Luck wrote:
...
> +static void domain_remove_cpu_ctrl(int cpu, struct rdt_resource *r) > +{ > + int id = get_domain_id_from_scope(cpu, r->ctrl_scope); > struct rdt_hw_domain *hw_dom; > + struct rdt_domain_hdr *hdr; > struct rdt_domain *d; > > if (id < 0) > return; > > - d = rdt_find_domain(r, id, NULL); > - if (!d) { > - pr_warn("Couldn't find domain with id=%d for CPU %d\n", id, cpu); > + hdr = rdt_find_domain(&r->ctrl_domains, id, NULL); > + if (!hdr) { > + pr_warn("Couldn't find control scope id=%d for CPU %d\n", id, cpu);
This error message seems strange to me. Shouldn't this just be "Couldn't find control domain with id=..."? Also, can the resource name be added in error message to help user dig into cause of error?
> return; > } > + > + if (WARN_ON_ONCE(hdr->type != RESCTRL_CTRL_DOMAIN)) > + return; > + > + d = container_of(hdr, struct rdt_domain, hdr); > hw_dom = resctrl_to_arch_dom(d); > > cpumask_clear_cpu(cpu, &d->hdr.cpu_mask); > if (cpumask_empty(&d->hdr.cpu_mask)) { > - resctrl_offline_domain(r, d); > + resctrl_offline_ctrl_domain(r, d); > list_del(&d->hdr.list); > > /* > @@ -596,6 +654,38 @@ static void domain_remove_cpu(int cpu, struct rdt_resource *r) > > return; > } > +} > + > +static void domain_remove_cpu_mon(int cpu, struct rdt_resource *r) > +{ > + int id = get_domain_id_from_scope(cpu, r->mon_scope); > + struct rdt_hw_domain *hw_dom; > + struct rdt_domain_hdr *hdr; > + struct rdt_domain *d; > + > + if (id < 0) > + return; > + > + hdr = rdt_find_domain(&r->mon_domains, id, NULL); > + if (!hdr) { > + pr_warn("Couldn't find monitor scope id=%d for CPU %d\n", id, cpu); > + return; > + }
Similar to above, can this be "Couldn't find monitor domain with id..."? Can resource name be added here also?
The rest of the patch looks good to me.
Reinette
| |