Re: [PATCH v1 8/9] x86/resctrl: Use mbm_update() to push soft RMID counts

From: Peter Newman
Date: Fri Jun 02 2023 - 08:42:58 EST


Hi Reinette,

On Thu, May 11, 2023 at 11:40 PM Reinette Chatre
<reinette.chatre@xxxxxxxxx> wrote:
> On 4/21/2023 7:17 AM, Peter Newman wrote:
> > list_for_each_entry(prgrp, &rdt_all_groups, rdtgroup_list) {
> > - mbm_update(r, d, prgrp->mon.rmid);
> > + /*
> > + * mbm_update() on every RMID would result in excessive IPIs
> > + * when RMIDs are soft.
> > + */
> > + if (!rdt_mon_soft_rmid) {
> > + mbm_update(r, d, prgrp->mon.rmid);
> >
> > - head = &prgrp->mon.crdtgrp_list;
> > - list_for_each_entry(crgrp, head, mon.crdtgrp_list)
> > - mbm_update(r, d, crgrp->mon.rmid);
> > + head = &prgrp->mon.crdtgrp_list;
> > + list_for_each_entry(crgrp, head, mon.crdtgrp_list)
> > + mbm_update(r, d, crgrp->mon.rmid);
> > + }
> >
> > if (is_mba_sc(NULL))
> > update_mba_bw(prgrp, d);
>
>
> hmmm ... I think that update_mba_bw() relies on mbm_update() to call
> mbm_bw_count() to update the data it depends on. Keeping update_mba_bw()
> while dropping mbm_update() thus seems problematic. AMD does not support the
> software controller though so it may make things simpler if support for
> software RMIDs disables support for software controller (in a clear way).

I looked over this again and realized that the rationale for skipping
mbm_update() in this patch is incorrect.
__mon_event_count_soft_rmid() does not send any IPIs, so it's
perfectly fine to call mbm_update() and update_mba_bw() when using
soft RMIDs.

Even if we don't use the software controller on AMD, it seems
conceptually cleaner to just allow soft and hard RMIDs to be used
interchangeably wherever they work.

-Peter