Re: [PATCH RFC v5 13/18] riscv_cbqri: resctrl: Add MB_MIN bandwidth allocation via Rbwb

[Drew Fustini <fustini@kernel.org>]

On Mon, May 25, 2026 at 12:55:23AM +0000, sashiko-bot@kernel.org wrote:
> > +	case RDT_RESOURCE_MB_MIN:
> > +		/* sum(Rbwb) <= MRBWB validation runs inside cbqri_apply_rbwb(). */
> > +		return cbqri_apply_rbwb(dom->hw_ctrl, closid, cfg_val, true);
> 
> Does this validation approach leak bandwidth when groups are deleted?
> 
> When a resctrl group is deleted via rmdir, the CLOSID is returned to the free
> pool, but standard resctrl flows do not trigger an architecture-specific
> callback to reset the resource's value. 
> 
> Because cbqri_apply_rbwb() iterates over all RCIDs (including inactive ones)
> to enforce sum(Rbwb) <= MRBWB, the bandwidth assigned to deleted groups remains
> permanently accounted against the global MRBWB budget.
> 
> If a user deletes a group to free up bandwidth and tries to assign it to an
> active group, could it be falsely rejected with -EINVAL, effectively leaking
> that capacity until the exact CLOSID is randomly reused?

This is already listed in the open issues in the cover letter. The fix
would require some changes to resctrl core. I would prefer to work on
a patch series to address this issue after this series lands.

> > +static int cbqri_attach_cpu_to_one_bw_res(struct cbqri_controller *ctrl,
> > +					  enum resctrl_res_level rid,
> > +					  unsigned int cpu)
> > +{
> > +	struct cbqri_resctrl_res *hw_res = &cbqri_resctrl_resources[rid];
> > +	struct rdt_resource *res = &hw_res->resctrl_res;
> > +	struct rdt_ctrl_domain *domain;
> > +	int dom_id = ctrl->mem.prox_dom;

> Can this alias multiple memory controllers to a single domain?
> 
> A single NUMA node (proximity domain) can contain multiple memory controllers.
> If multiple bandwidth controllers share the same proximity domain, the first one
> processed will create a domain and assign its hw_ctrl pointer to it.
> 
> When the second controller is processed, cbqri_find_ctrl_domain() will match
> the existing dom_id, and simply add its CPUs to the cpu_mask without saving
> the second controller's hw_ctrl pointer.
> 
> Will subsequent schemata writes to this domain exclusively program the first
> bandwidth controller, leaving the second unmanaged by resctrl?

This is a known limitation that can be addressed in a future series.

> > -		err = cbqri_attach_cpu_to_cap_ctrl(ctrl, cpu);
> > +		}
> >  		if (err)
> >  			break;
> >  	}
> 
> This is a pre-existing issue, but does this leak domain state if an error
> occurs here?
> 
> If an attachment fails, the loop breaks and returns the error immediately. The
> CPU hotplug core catches the error and aborts the online state transition.
> However, it does not invoke the corresponding offline teardown callback because
> this state failed during its startup sequence.
> 
> Since cbqri_resctrl_online_cpu() fails to manually roll back the domain state
> for the controllers that successfully processed the CPU in earlier iterations,
> will the CPU remain permanently set in their cpu_mask?

I will change the next rev to track the last successfully attached
controller and detach in reverse on the error path.  

-Drew