Message-ID: <91ccdf56-1a33-437e-9a55-ae2e1e89fa8d@amd.com>
Date: Mon, 6 May 2024 13:09:29 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>,
 Peter Newman <peternewman@...gle.com>
Cc: corbet@....net, fenghua.yu@...el.com, tglx@...utronix.de,
 mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com, 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, maciej.wieczor-retman@...el.com,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, eranian@...gle.com,
 james.morse@....com
Subject: Re: [RFC PATCH v3 17/17] x86/resctrl: Introduce interface to modify
 assignment states of the groups

Hi Reinette,

On 5/3/24 16:16, Reinette Chatre wrote:
> Hi Babu,
> 
> On 5/3/2024 9:14 AM, Moger, Babu wrote:
>> On 5/2/2024 6:00 PM, Reinette Chatre wrote:
>>> On 4/17/2024 3:52 PM, Moger, Babu wrote:
>>>> On 4/17/2024 3:56 PM, Peter Newman wrote:
>>>>> On Wed, Apr 17, 2024 at 12:39 PM Moger, Babu <babu.moger@....com> wrote:
>>>>>> On 4/17/24 12:45, Peter Newman wrote:
>>>>>>> On Thu, Mar 28, 2024 at 6:10 PM Babu Moger <babu.moger@....com> wrote:
>>>>>>>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
>>>>>>>> index 2d96565501ab..64ec70637c66 100644
>>>>>>>> --- a/Documentation/arch/x86/resctrl.rst
>>>>>>>> +++ b/Documentation/arch/x86/resctrl.rst
>>>>>>>> @@ -328,6 +328,77 @@ with the following files:
>>>>>>>>            None of events are assigned on this mon group. This is a child
>>>>>>>>            monitor group of the non default control mon group.
>>>>>>>>
>>>>>>>> +       Assignment state can be updated by writing to this interface.
>>>>>>>> +
>>>>>>>> +       NOTE: Assignment on one domain applied on all the domains. User can
>>>>>>>> +       pass one valid domain and assignment will be updated on all the
>>>>>>>> +       available domains.
>>>>>>> How would different assignments to different domains work? If the
>>>>>>> allocations are global, then the allocated monitor ID is available to
>>>>>>> all domains whether they use it or not.
>>>>>> That is correct.
>>>>>> [A] Hardware counters(max 2 per group) are allocated at the group level.
>>>>>> So, those counters are available to all the domains on that group. I will
>>>>>> maintain a bitmap at the domain level. The bitmap will be set on the
>>>>>> domains where assignment is applied and IPIs are sent. IPIs will not be
>>>>>> sent to other domains.
>>>>> Unless the monitor allocation is scoped at the domain level, I don't
>>>>> see much point in implementing the per-domain parsing today, as the
>>>>> only benefit is avoiding IPIs to domains whose counters you don't plan
>>>>> to read.
>>>>
>>>> In that case lets remove the domain specific assignments. We can avoid some code complexity.
>>>>
>>>
>>> As I understand counters are scoped at the domain level and it is
>>> an implementation choice to make the allocation global. (Similar to
>>> the decision to make CLOSIDs global.)
>>>
>>> Could you please elaborate how you plan to remove domain specific
>>> assignments? I do think it needs to remain as part of the user interface
>>> so I wonder if this may look like only "*=<flags>" is supported on
>>> these systems and attempting to assign an individual domain may fail
>>> with "not supported".
>>
>> This series applies the assignment to all the domains.
>>
>> For example:
>>
>> # echo "//0=t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control
>>
>> User here wants to assign a monitor to total event on domain 0.
>> But this series applies monitor to all the domains in the system. IPIs will be sent to all the domains.
> 
> I would like to recommend against this. (a) this is not what the API
> says will happen, (b) behavior like this may result in users having scripts
> with syntax like above expecting changes to all domains and when/if
> AMD or another architecture decides to implement per-domain assignment
> it will break user space.

Sure. We keep per-domain specific assignment.

"//0=t"   This will apply assignment only domain 0.
"//*=t"   This will apply assignment one all the domains.

Hope it clarifies.

> 
>> Basically this is equivalent to
>>
>> # echo "//*=t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control
>>
>>
>> I was thinking of adding domain specific assignment in next version.
>> That involves adding a new field in rdt_domain to keep track of
>> assignment. Peter suggested it may not be much of a value add for his
>> usage model.
> 
> I do not have insight into how all users will end up using this.
> 
> Reinette
> 

-- 
Thanks
Babu Moger

Powered by blists - more mailing lists