Message-ID: <3b6a8ec9-5e4f-e4ff-cd01-96ecc366565a@amd.com>
Date: Fri, 4 Oct 2024 11:38:02 -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 v7 24/24] x86/resctrl: Introduce interface to modify
 assignment states of the groups

Hi Reinette,

On 10/3/2024 9:17 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 10/3/24 6:11 PM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 10/2/2024 1:19 PM, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 9/27/24 10:47 AM, Moger, Babu wrote:
>>>> On 9/19/2024 12:59 PM, Reinette Chatre wrote:
>>>>> On 9/4/24 3:21 PM, Babu Moger wrote:
>>>
>>>>>> v7: Simplified the parsing (strsep(&token, "//") in rdtgroup_mbm_assign_control_write().
>>>>>>        Added mutex lock in rdtgroup_mbm_assign_control_write() while processing.
>>>>>>        Renamed rdtgroup_find_grp to rdtgroup_find_grp_by_name.
>>>>>>        Fixed rdtgroup_str_to_mon_state to return error for invalid flags.
>>>>>>        Simplified the calls rdtgroup_assign_cntr by merging few functions earlier.
>>>>>>        Removed ABMC reference in FS code.
>>>>>>        Reinette commented about handling the combination of flags like 'lt_' and '_lt'.
>>>>>>        Not sure if we need to change the behaviour here. Processed them sequencially right now.
>>>>>>        Users have the liberty to pass the flags. Restricting it might be a problem later.
>>>>>
>>>>> Could you please give an example of what problem may be encountered later? An assignment
>>>>> like "domain=_lt" seems like a contradiction to me since user space essentially asks
>>>>> for "None of the MBM events" as well as "MBM total event" and "MBM local event".
>>>>
>>>> I agree it is contradiction. But user is the one who decides to do that. I think we should allow it. Also, there is some value to it as well.
>>>>
>>>> "domain=_lt" This will also reset the counters if the total and local events are assigned earlier this action.
>>>
>>> The last sentence is not clear to me. Could you please elaborate what
>>> you mean with "are assigned earlier this action"?
>>>
>>
>> I think I confused you here. "domain=_lt" is equivalent to "domain=lt".  My reasoning is handling all the combination in the code adds code complexity and leave it the user what he wants to do.
> 
> hmmm ... and how about "domain=lt_"? Do you think this should also be equivalent to
> "domain=lt" or perhaps an expectation that counters should be assigned to the two events
> and then immediately unassigned?

Yes. "domain=lt_" should be "domain=lt".

> 
> Giving user such flexibility may be interpreted as the assignment seen as acting
> sequentially through the flags provided. Ideally the interface should behave in
> a predictable way if the goal is to provide flexibility to the user.
> 

My only concern is adding the check now and reverting it back later.
Basically process the flags sequentially and don't differentiate between 
the flags. I feel it fits the predictable behavior. No?

-- 
- Babu Moger

Powered by blists - more mailing lists