[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2e83227f-be2b-7833-0ae5-85be3dd7bdfb@amd.com>
Date: Fri, 23 Aug 2024 17:14:04 -0500
From: "Moger, Babu" <bmoger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, babu.moger@....com,
corbet@....net, fenghua.yu@...el.com, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, dave.hansen@...ux.intel.com
Cc: x86@...nel.org, hpa@...or.com, paulmck@...nel.org, rdunlap@...radead.org,
tj@...nel.org, peterz@...radead.org, yanjiewtw@...il.com,
kim.phillips@....com, lukas.bulwahn@...il.com, seanjc@...gle.com,
jmattson@...gle.com, leitao@...ian.org, jpoimboe@...nel.org,
rick.p.edgecombe@...el.com, kirill.shutemov@...ux.intel.com,
jithu.joseph@...el.com, kai.huang@...el.com, kan.liang@...ux.intel.com,
daniel.sneddon@...ux.intel.com, pbonzini@...hat.com, sandipan.das@....com,
ilpo.jarvinen@...ux.intel.com, peternewman@...gle.com,
maciej.wieczor-retman@...el.com, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, eranian@...gle.com, james.morse@....com
Subject: Re: [PATCH v6 00/22] x86/resctrl : Support AMD Assignable Bandwidth
Monitoring Counters (ABMC)
Hi Reinette,
On 8/23/2024 3:29 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 8/21/24 6:31 PM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 8/16/24 16:28, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 8/6/24 3:00 PM, Babu Moger wrote:
>>>>
>>>> Feature adds following interface files:
>>>>
>>>> /sys/fs/resctrl/info/L3_MON/mbm_mode: Reports the list of assignable
>>>> monitoring features supported. The enclosed brackets indicate which
>>>> feature is enabled.
>>>
>>> I've been considering this file as a generic file where all future "MBM
>>> modes"
>>> can be captured, while this series treats it as specific to "assignable
>>> monitoring
>>> features" (btw, should this be "assignable monitoring modes" to match
>>> the
>>> name?).
>>> Looking closer at this implementation it does make things easier that
>>> "mbm_mode" is
>>> specific to "assignable monitoring features" but when doing so I
>>> think it
>>> should have
>>> a less generic name to avoid the obstacles we have with the existing
>>> "mon_features".
>>> Apologies that this goes back to be close to what you had earlier ...
>>> maybe
>>> "mbm_assign_mode"?
>>
>> Lets see:
>> #cat /sys/fs/resctrl/info/L3_MON/mbm_mode
>> [mbm_cntr_assign] <- This already says 'assign'. Isn't that enough?
>
> It will be enough if "mbm_mode" is intended to be used for all current
> and future MBM modes/features but this series instead dedicates this file
> to just "assignable monitoring counters" feature. Doing so prevents us
> from, in the future, expanding this file to, for example, contain
> a new entry representing a new feature.
>
>>
>> default <- Default mode is not related assignable features.
>
> If not assignable features, what is it related to? "default" being the
> absence of assignable features still seems related to me.
>
>>
>> I would think mbm_mode is fine. Let me know.
>
> If this work is reworded that it is intended to support any MBM mode then
> it is fine, if this work remains to dedicate this file just to assignable
> features then I think its name should be changed.
Ok. Will change it to "mbm_assign_mode".
>
> ...
>
>>>>
>>>> Flags can be one of the following:
>>>>
>>>> t MBM total event.
>>>> l MBM local event.
>>>> tl Both total and local MBM events.
>>>> _ None of the MBM events. Only works with '=' opcode.
>>>
>>> Please take care with the implementation that seems to support a
>>> variety of
>>> combinations. If I understand correctly the implementation support flags
>>> like,
>>> for example, "tttt", "llll", "ltlt" ... those may not be an issue but
>>> of most
>>> concern is, for example, a pattern like "_lt" that (unexpectedly)
>>> appears to
>>> result in set of total and local.
>>
>> Yes. Should we not allow flag combinations with "_"?
>> I am not very sure about how to go about this.
>>
>
> This topic seems to have moved to patch #22.
Yes. got it.
>
> ...
>
>>>> # echo "legacy" > /sys/fs/resctrl/info/L3_MON/mbm_mode
>>>> # cat /sys/fs/resctrl/info/L3_MON/mbm_mode
>>>> mbm_cntr_assign
>>>> [legacy]
>>>>
>>>> k. Unmount the resctrl
>>>> #umount /sys/fs/resctrl/
>>>> ---
>>>> v6:
>>>> We still need to finalize few interface details on mbm_mode and
>>>> mbm_control
>>>> in case of ABMC and Soft-ABMC. We can continue the discussion with
>>>> this series.
>>>
>>> Could you please list the details that need to be finalized?
>>
>> 1. mbm_mode display
>> # cat /sys/fs/resctrl/info/L3_MON/mbm_mode
>> mbm_cntr_assign
>> [legacy]
>>
>> "mbm_cntr_assign"
>> Are we sticking with ""mbm_cntr_assign" for ABMC?
>> What should we name for soft-ABMC?
>>
>> 2. Also we had some concerns about Individual event assignment(ABMC)
>> and group assignment(soft-ABMC)?
>> Are the flags "t" and 'l' good for both these modes?
>
> If I remember correctly the previous discussion ended with the need for
> "modes" that indicate to user space what to expect when interacting with
> the
> MBM flags in the "mbm_control" file. The term used by ABMC should
> reflect that
> each MBM flag/event can be set independently, while the term used by
> soft-ABMC
> reflects that setting one flag/event makes the same change to the other
> MBM flag/event.
>
Will add text in the resctrl.rst make it clear about it.
thanks
--
- Babu Moger
Powered by blists - more mailing lists