linux-kernel - Re: [PATCH v11 17/23] x86/resctrl: Auto assign/unassign counters when mbm_cntr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5f651b5a-3470-4ca8-80dd-4593acef6df4@amd.com>
Date: Fri, 21 Feb 2025 12:23:08 -0600
From: "Moger, Babu" <bmoger@....com>
To: Dave Martin <Dave.Martin@....com>,
 Reinette Chatre <reinette.chatre@...el.com>
Cc: Peter Newman <peternewman@...gle.com>, Babu Moger <babu.moger@....com>,
 corbet@....net, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, tony.luck@...el.com, x86@...nel.org,
 hpa@...or.com, paulmck@...nel.org, akpm@...ux-foundation.org,
 thuth@...hat.com, rostedt@...dmis.org, xiongwei.song@...driver.com,
 pawan.kumar.gupta@...ux.intel.com, daniel.sneddon@...ux.intel.com,
 jpoimboe@...nel.org, perry.yuan@....com, sandipan.das@....com,
 kai.huang@...el.com, xiaoyao.li@...el.com, seanjc@...gle.com,
 xin3.li@...el.com, andrew.cooper3@...rix.com, ebiggers@...gle.com,
 mario.limonciello@....com, james.morse@....com, tan.shaopeng@...itsu.com,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 maciej.wieczor-retman@...el.com, eranian@...gle.com
Subject: Re: [PATCH v11 17/23] x86/resctrl: Auto assign/unassign counters when
 mbm_cntr_assign is enabled

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@...el.com> 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
>