Re: [PATCH v11 17/23] x86/resctrl: Auto assign/unassign counters when mbm_cntr_assign is enabled

[Moger, Babu]

Hi All,

On 2/21/2025 11:14 AM, Dave Martin wrote:
> Hi,
>
> On Thu, Feb 20, 2025 at 09:08:17AM -0800, Reinette Chatre wrote:
> > Hi Dave,
> >
> > On 2/20/25 5:40 AM, Dave Martin wrote:
> > > On Thu, Feb 20, 2025 at 11:35:56AM +0100, Peter Newman wrote:
> > > > Hi Reinette,
> > > >
> > > > On Wed, Feb 19, 2025 at 6:55 PM Reinette Chatre
> > > > <reinette.chatre@xxxxxxxxx> wrote:
>
> [...]
>
> > > > > Could you please remind me how a user will set this flag?
> > > >
> > > > Quoting my original suggestion[1]:
> > > >
> > > >   "info/L3_MON/mbm_assign_on_mkdir?
> > > >
> > > >    boolean (parsed with kstrtobool()), defaulting to true?"
> > > >
> > > > After mount, any groups that got counters on creation would have to be
> > > > cleaned up, but at least that can be done with forward progress once
> > > > the flag is cleared.
> > > >
> > > > I was able to live with that as long as there aren't users polling for
> > > > resctrl to be mounted and immediately creating groups. For us, a
> > > > single container manager service manages resctrl.
>
> [...]
>
> > > +1
> > >
> > > That's basically my position -- the auto-assignment feels like a
> > > _potential_ nuisance for ABMC-aware users, but it depends on what they
> > > are trying to do.  Migration of non-ABMC-aware users will be easier for
> > > basic use cases if auto-assignment occurs by default (as in this
> > > series).
> > >
> > > Having an explicit way to turn this off seems perfectly reasonable
> > > (and could be added later on, if not provided in this series).
> > >
> > >
> > > What about the question re whether turning mbm_cntr_assign mode on
> > > should trigger auto-assignment?
> > >
> > > Currently turning this mode off and then on again has the effect of
> > > removing all automatic assignments for extant groups.  This feels
> > > surprising and/or unintentional (?)
> >
> > Connecting to what you start off by saying I also see auto-assignment
> > as the way to provide a smooth transition for "non-ABMC-aware" users.
>
> I agree, and having this on by default also helps non-ABMC-aware users.
>
> > To me a user that turns this mode off and then on again can be
> > considered as a user that is "ABMC-aware" and turning it "off and then
> > on again" seems like an intuitive way to get to a "clean slate"
> > wrt counter assignments. This may also be a convenient way for
> > an "ABMC-aware" user space to unassign all counters and thus also
> > helpful if resctrl supports the flag that Peter proposed. The flag
> > seems to already keep something like this in its context with
> > a name of "mbm_assign_on_mkdir" that could be interpreted as
> > "only auto assign on mkdir"?
>
> Yes, that's reasonable.  It could be a good idea to document this
> behaviour of switching the mbm_cntr_assign mode, if we think it is
> useful and people are likely to rely on it.
>
> Since mkdir is an implementation detail of the resctrl interface, I'd
> be tempted to go for a more generic name, say,
> "mbm_assign_new_mon_groups".  But that's just bikeshedding.
> The proposed behaviour seems fine.
>
> Either way, if this is not included in this series, it could be added
> later without breaking anything.

How about more generic "mbm_cntr_assign_auto" ?

We can add it as part of "struct resctrl_mon" and set it "on" when ABMC 
is detected. It will be part of check in rdtgroup_assign_cntrs() which 
is called when new groups are created. Also, provide  user interface to 
disable it. Seems simple to me.

Thanks
Babu


> > I am not taking a stand for one or the other approach but instead
> > trying to be more specific about pros/cons. Could you please provide
> > more insight in the use case you have in mind so that we can see how
> > resctrl could behave with few surprises?
> >
> > Reinette
>
> I don't have a strong view either.
>
> I don't have a concrete use case here -- I was just trying to imagine
> the experience of an ABMC-aware user who wants full control over what
> counters get assigned.
>
> I agree that the convenience of the non-ABMC-aware user should probably
> take priority over that of the ABMC-aware user, at least in situations
> where the expected behaviour is achievable (i.e., where we didn't run
> out of counters to auto-assign.)
>
> Cheers
> ---Dave