[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c0afa85d-d196-4873-acb7-08b515410fcd@amd.com>
Date: Fri, 21 Feb 2025 17:42:57 -0600
From: "Moger, Babu" <bmoger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>,
Dave Martin <Dave.Martin@....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 Reinette,
On 2/21/2025 4:48 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 2/21/25 10:23 AM, Moger, Babu wrote:
>> 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" ?
>
> I would like to be careful to not make it _too_ generic. Dave already pointed
> out that users may be surprised that counters are not auto-assigned when switching
> between the different modes so using the the name to help highlight when this
> auto-assignment can be expected to happen seems very useful.
In that case "mbm_assign_on_mkdir" seems on point and precise.
Thanks
Babu
Powered by blists - more mailing lists