Re: [PATCH RFC v5 14/18] riscv_cbqri: resctrl: Add MB_WGHT bandwidth allocation via Mweight

[Drew Fustini <fustini@kernel.org>]

On Mon, May 25, 2026 at 12:52:19AM +0000, sashiko-bot@kernel.org wrote:
> > @@ -1019,7 +1065,13 @@ static int cbqri_attach_cpu_to_one_bw_res(struct cbqri_controller *ctrl,
> >  static int cbqri_attach_cpu_to_bw_ctrl(struct cbqri_controller *ctrl,
> >  				       unsigned int cpu)
> >  {
> > -	return cbqri_attach_cpu_to_one_bw_res(ctrl, RDT_RESOURCE_MB_MIN, cpu);
> > +	int err;
> > +
> > +	err = cbqri_attach_cpu_to_one_bw_res(ctrl, RDT_RESOURCE_MB_MIN, cpu);
> > +	if (err)
> > +		return err;
> > +
> > +	return cbqri_attach_cpu_to_one_bw_res(ctrl, RDT_RESOURCE_MB_WGHT, cpu);
> >  }
> 
> [Severity: High]
> If cbqri_attach_cpu_to_one_bw_res() fails for RDT_RESOURCE_MB_WGHT, does
> this leave the CPU permanently attached to the RDT_RESOURCE_MB_MIN domain?

I will change it to unwind the MB_MIN attach on MB_WGHT failure.

> This is a pre-existing issue, but looking at the caller
> cbqri_resctrl_online_cpu(), there appears to be a similar problem with the
> controller iteration:
> 
> drivers/resctrl/cbqri_resctrl.c:cbqri_resctrl_online_cpu() {
> 	...
> 	list_for_each_entry(ctrl, &cbqri_controllers, list) {
> 		switch (ctrl->type) {
> 		...
> 		case CBQRI_CONTROLLER_TYPE_BANDWIDTH:
> 			...
> 			err = cbqri_attach_cpu_to_bw_ctrl(ctrl, cpu);
> 			break;
> 		}
> 		if (err)
> 			break;
> 	}
> 
> 	mutex_unlock(&cbqri_domain_list_lock);
> 	return err;
> }
> 
> If the loop terminates early due to an error, are the domains that were
> successfully attached in earlier iterations left without being detached? Like
> the issue above, the offline callback won't be called, potentially leaking
> the domain allocations.

I will rework cbqri_resctrl_online_cpu() to track the last successfully
attached controller and walk the list backwards from there on error.

Drew