Re: [PATCH RFC v3 06/11] RISC-V: QoS: add resctrl setup and domain management

Drew Fustini
2026-05-01T22:56:07Z

On Thu, Apr 30, 2026 at 04:20:09PM -0700, Reinette Chatre wrote:
> Hi Drew,
>
> On 4/14/26 6:54 PM, Drew Fustini wrote:
> > +
> > +static int qos_resctrl_add_controller_domain(struct cbqri_controller *ctrl)
> > +{
> > + struct rdt_ctrl_domain *domain;
> > + struct cbqri_resctrl_res *cbqri_res = NULL;
> > + struct rdt_resource *res = NULL;
> > + struct list_head *pos = NULL;
> > + int err;
> > +
> > + domain = qos_new_domain(ctrl);
> > + if (!domain)
> > + return -ENOSPC;
> > +
> > + switch (ctrl->type) {
> > + case CBQRI_CONTROLLER_TYPE_CAPACITY:
> > + cpumask_copy(&domain->hdr.cpu_mask, &ctrl->cache.cpu_mask);
>
> Looking at patch #10 ctrl->cache.cpu_mask contains all CPUs associated with cache
> even if they are offline. This is not what resctrl expects. Instead the expectation is
> that a domain exists and is online (hence "resctrl_online_ctrl_domain()") if at least one CPU
> belonging to that domain is online and domain->hdr.cpu_mask lists all the *online* CPUs
> associated with that domain.
> This is why resctrl always takes the CPU hotplug lock when traversing the domain
> lists.
>
> I thus expected this initialization to be split between an early initialization of
> resource capabilities and then domain initialization as part of the CPU online/offline
> handlers.

Good point. I'll rework this so that domain allocation moves to the cpu
online/offline handlers.

[..]
> > + if (pos)
> > + list_add_tail(&domain->hdr.list, pos);
> > + else
> > + list_add_tail(&domain->hdr.list, &res->ctrl_domains);
>
> resctrl_find_domain() returns NULL if it cannot find an existing domain, in that
> case it initializes "pos" to support adding a new domain in a sorted list.
> Expectation is that domains are managed as part of CPU hotplug handlers. When
> a CPU comes online then handler can check if the domain it belongs to already exists,
> if it does then the CPU can just be added to that domain's cpu_mask, if it does
> not then a new domain is created and added in the the appropriate spot in the
> sorted list (based on domain ID) of domains.

Okay, I'll move this from probe to qos_resctrl_online_cpu().

> > +int qos_resctrl_online_cpu(unsigned int cpu)
> > +{
> > + resctrl_online_cpu(cpu);
>
> This is where a domain is expected to be added when its first CPU comes online.
[..]
> > +int qos_resctrl_offline_cpu(unsigned int cpu)
> > +{
> > + resctrl_offline_cpu(cpu);
>
> This is where a domain is expected to be removed when its last CPU goes offline.

Okay, I'll update v4 to add and remove domains in these hooks.

Thanks,
Drew