Re: [PATCH RFC v5 15/18] riscv_cbqri: resctrl: Add mbm_total_bytes bandwidth monitoring

[Drew Fustini <fustini@kernel.org>]

On Mon, May 25, 2026 at 01:27:42AM +0000, sashiko-bot@kernel.org wrote:
> > +		if (rmid >= bc->mcid_count) {
> > +			err = -ERANGE;
> > +			break;
> > +		}
> 
> This is a pre-existing issue, but will the core allocate out-of-bounds
> RMIDs that trigger this -ERANGE error? Looking at
> cbqri_resctrl_control_init(), num_rmid is bound to the local capacity
> rather than the global minimum (max_rmid). Could this lead to stale
> monitoring state and bypass the max_rmid-bounded reset loop?

I will fix in the revision.

> > +	if (l3_count > 1) {
> > +		pr_warn_once("multiple L3 domains (%u) detected. mbm_total_bytes disabled\n",
> > +			     l3_count);
> > +		return;
> > +	}
> 
> When mbm_total_bytes is disabled here, will the unused Bandwidth
> Controller's smaller mcid_count still artificially clamp the global
> max_rmid in cbqri_resctrl_accumulate_caps()? The unused BC remains
> mon_capable, which might restrict valid occupancy monitoring system-wide.

I will change it to walk only the controllers that were actually picked
as counter sources. 

> > +	hw_dom->paired_bc = cbqri_find_only_mon_bc();
> > +	if (hw_dom->paired_bc) {
> > +		err = cbqri_init_bc_mon_counters(hw_dom->paired_bc);
> 
> If mbm_total_bytes was disabled in cbqri_resctrl_pick_counters() due to
> multiple L3 domains, does this unconditionally allocate memory and issue
> MMIO resets for an unused BC? Could an initialization failure in this unused
> BC cause the entire L3 bringup to abort here?

I will change to skip the init when mbm_total_bytes is disabled.

> >  	cancel_delayed_work(&mon_dom->cqm_limbo);
> > +	if (resctrl_is_mon_event_enabled(QOS_L3_MBM_TOTAL_EVENT_ID))
> > +		cancel_delayed_work(&mon_dom->mbm_over);
> >  	resctrl_offline_mon_domain(res, &mon_dom->hdr);
> >  err_listdel:
> >  	list_del(&mon_dom->hdr.list);
> >  err_free:
> >  	kfree(mon_dom);
> >  	return err;
> >  }
> 
> [Severity: Critical]
> This is a pre-existing issue, but since this patch introduces a new trigger
> instance for mbm_over, does this create a use-after-free window?
> 
> Since cancel_delayed_work() is used without _sync, if a worker is currently
> stalled waiting on the cpus_read_lock, won't kfree(mon_dom) execute
> synchronously here? Once the write lock is dropped, couldn't the pending
> worker resume and dereference the freed memory?

I will address in the change to patch 12.

Drew