Received-SPF: pass (google.com: domain of linux-kernel+bounces-85491-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <5ddb7031-a828-4001-bfdf-5477cb85e9ed@amd.com>
Date: Wed, 28 Feb 2024 11:59:19 -0600
User-Agent: Mozilla Thunderbird
Reply-To: babu.moger@amd.com
Subject: Re: [PATCH v2 00/17] x86/resctrl : Support AMD Assignable Bandwidth
 Monitoring Counters (ABMC)
Content-Language: en-US
To: Reinette Chatre <reinette.chatre@intel.com>,
 James Morse <james.morse@arm.com>, corbet@lwn.net, fenghua.yu@intel.com,
 tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
 dave.hansen@linux.intel.com
Cc: x86@kernel.org, hpa@zytor.com, paulmck@kernel.org, rdunlap@infradead.org,
 tj@kernel.org, peterz@infradead.org, yanjiewtw@gmail.com,
 kim.phillips@amd.com, lukas.bulwahn@gmail.com, seanjc@google.com,
 jmattson@google.com, leitao@debian.org, jpoimboe@kernel.org,
 rick.p.edgecombe@intel.com, kirill.shutemov@linux.intel.com,
 jithu.joseph@intel.com, kai.huang@intel.com, kan.liang@linux.intel.com,
 daniel.sneddon@linux.intel.com, pbonzini@redhat.com, sandipan.das@amd.com,
 ilpo.jarvinen@linux.intel.com, peternewman@google.com,
 maciej.wieczor-retman@intel.com, linux-doc@vger.kernel.org,
 linux-kernel@vger.kernel.org, eranian@google.com
References: <20231201005720.235639-1-babu.moger@amd.com>
 <cover.1705688538.git.babu.moger@amd.com>
 <2f373abf-f0c0-4f5d-9e22-1039a40a57f0@arm.com>
 <474ebe02-2d24-4ce3-b26a-46c520efd453@amd.com>
 <b6bb6a59-67c2-47bc-b8d3-04cf8fd21219@intel.com>
 <3fe3f235-d8a6-453b-b69d-6b7f81c07ae1@amd.com>
 <9b94b97e-4a8c-415e-af7a-d3f832592cf9@intel.com>
 <1ae73c9a-cec4-4496-86c6-3ffcef7940d6@amd.com>
 <32a588e2-7b09-4257-b838-4268583a724d@intel.com>
 <088878bd-7533-492d-838c-6b39a93aad4d@amd.com>
 <9b20589b-6220-4ae7-bfc4-4a826b7114b1@intel.com>
From: "Moger, Babu" <babu.moger@amd.com>
In-Reply-To: <9b20589b-6220-4ae7-bfc4-4a826b7114b1@intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?NEVLbGZ6ZXhHU1J6NUZBNlhodWZBSURhV1B4M0RaSFVSRFI4OW5NYU9EbVFJ?=
 =?utf-8?B?Wk5zcGpiVExpSWtGYlVrMU1EL3pPWHVUcmVJRFlsUTE4aHhpd1VNS25jYUhX?=
 =?utf-8?B?ckpTcUhzVURGdldzaHdkejhRREMva3l6bHFoeTVSWWVJNURjbitYRGc3a2NI?=
 =?utf-8?B?L29rRysxZUpINVJwTnk3WURrbm16a2R3T0FxNGJnSWtydHhEZHJDK0hJQXd6?=
 =?utf-8?B?M01DRFozdUtxY2VmU1ZFVEhrc09CYWRDZ21ldW9FWWVQbHc0bGVRL1lWZXZC?=
 =?utf-8?B?RTNyMUJtOTYzQ1V6RGJEM3JZQUM3TG5yWWZwQmpBU2NUcEsrUWZhZUp2R0k5?=
 =?utf-8?B?ZzFjbGxGMzRLbEFJNGw0QjFtd3YyTTVySTdBOXZuMzhXNU80Sll1NmJRcmla?=
 =?utf-8?B?UVpQVnI3MHphWGZHaVdsQ29yaE9ibXRZQmoxMHRPdjRBV2gvdzg5aGI3T0sy?=
 =?utf-8?B?dll2R2Q4M3FoMTFXRHVnSlFzSmhMQ3VpcmM1emQ0bjNwQ1B3VTNYcEpRcW15?=
 =?utf-8?B?RWlweGtLaDVKMWwxWFhhR1BiOEdLMGJpeDJ1dldXUERRREZwT2QxOWtWM291?=
 =?utf-8?B?Z0lqYUs4S2FPeVdsSEtCb244SXpwcnlxemtOQnNjODZKTkRKQ3Q1aGJhZU1q?=
 =?utf-8?B?UzgySDdrRUt3UjVGNmFWeExZTlRHR3VIQzJicENZdWoraS9pb2x1NHVObGZL?=
 =?utf-8?B?S2Zmdk4wSWtFTWdadnN4ZWZLYjNiNHl3TzQ5TmZ6L1JMenFjZE9MTTRjeDRW?=
 =?utf-8?B?YlNXaEk1UDlUUnJzNWo0WnE3bjRFYk9TbWVTaEdyYVd2VG8xd2VKTTFxMDgw?=
 =?utf-8?B?d08rbUlMNk51cktwUGlIOHNvbkFIOThLeFBTcEoySFJ1SDR6OEwydkIyamh3?=
 =?utf-8?B?OG8xdnphTVpGYjZ5TU5LTStJOEI0WXlyczZ0WVZVWWsvQ0lNR2JxRm1GWjcz?=
 =?utf-8?B?Q0hsaEU5RUUzdmRPNlNaOEFwVHdFUnUyN0pjdWpHSFB2UzJsQ1FrdVNQMEMr?=
 =?utf-8?B?K1lrcnBzZ2lKaXVMOTJEL1B3cFlYd0JTYW1PRlRBQ3ZFMTlBaFNaZ2tyRmZS?=
 =?utf-8?B?dHBaZVpLVEhlUjVPWHVtR1M4ODF2bE9HaGVjWHplaUhJaFhaYkZWN3dhdWhE?=
 =?utf-8?B?NGNySGVRbmJodE5Rc1dTUytWd3pTVEc2Qy8vUFBGRTQ3M3dhRnpWQ3J4M25m?=
 =?utf-8?B?c0VrckFXOVdMSUMzRENIOFVHMGJNYkVVejlpd2FycU1abktoaitrcFZjVjZY?=
 =?utf-8?B?NFZLT1hlWTM4Zy9MamRGQkw0a1FXY0ZCbmZhMk1SMzVjZ1V4SjM3alRRK0Vk?=
 =?utf-8?B?L1dEaERYVVdwU09iR0ZLdVRDOHRUcVdUdzZyU1NQaUlvcUtReTM2M2VQTHpX?=
 =?utf-8?B?NFIybTRTdFNpaEdKRjl4V1pYVDVTNnJ4SGpPdTVIQnBYckdzdG5QRFN1ZDVE?=
 =?utf-8?B?ZVgrT0tNMFdYZ09aYkV6RGFjSlZMdXNBT25RTGJuL1U0KzJZZUFsWkc0V2cr?=
 =?utf-8?B?WDQrcGZ3UWlYWXg3cGJaQVlFNGFSTnpXU3ZjNkJkNzVFUXdnUUdRYkVIU25S?=
 =?utf-8?B?OGtPNVZzVytFbEFCQm5SNlVERkVtcUZiZklUZCtLK1dvTlU3dmJqeithQ0p0?=
 =?utf-8?B?R1BpVnpNK1Zjcmt2WWpTUjdmeWxGeTJKK0hWbjBGRHdCY0FvNktXa2hVdVI2?=
 =?utf-8?B?K1A5S1ZuVUFKMFpRNHpuaTFLM3h2WnFscE1MNFVEMkNudk9MczRjRnRuWER6?=
 =?utf-8?B?N21NUUs4UjZXR01UNVpXR1lTSGRRVFV3TDdFQnVzS1BlRjhsNmFvREZKRlNa?=
 =?utf-8?B?VkdhQzNOSmpqb0daTTJSRUFEeXM5eWhFNXlNT2xJelJPOHFMemlZV1hNU2gr?=
 =?utf-8?B?SW52anBoOXBBOTZOMlpXdVNqVG15V1BrOFY0QlM5TkFscDUwT0J5STB6ZnFV?=
 =?utf-8?B?N0QvUjJIc3pVcStaNWVFMUNTay95UlRzOXRGQTFoYVJmOWVibWJzQlNBdjBN?=
 =?utf-8?B?MzBRUkVwN0p3bmFlM28ra29JNTFGQkttK0FmS29lWjRvUm5QbjlKNkxxTS9N?=
 =?utf-8?B?YTVqR0tDcHE5eXlOTHVIeEFFZ2VYQjZDbEtjanVIN252blBFMExVRDlaU0xj?=
 =?utf-8?Q?siBY=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a9b44aee-8959-40d9-a8ec-08dc3886fecf
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2024 17:59:23.3263
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: QNXhSwyU6ujYZBCkzumpLh7alvFKux33xRVQcdqUf/AjPoj4fxtdvEsPW14WCBIe
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB5786

Hi Reinette,

On 2/27/24 17:50, Reinette Chatre wrote:
> Hi Babu,
> 
> On 2/27/2024 10:12 AM, Moger, Babu wrote:
>> On 2/26/24 15:20, Reinette Chatre wrote:
>>> On 2/26/2024 9:59 AM, Moger, Babu wrote:
>>>> On 2/23/24 16:21, Reinette Chatre wrote:
> 
>>>>> Apart from the "default behavior" there are two options to consider ...
>>>>> (a) the "original" behavior(? I do not know what to call it) - this would be
>>>>>     where user space wants(?) to have the current non-ABMC behavior on an ABMC
>>>>>     system, where the previous "num_rmids" monitor groups can be created but
>>>>>     the counters are reset unpredictably ... should this still be supported
>>>>>     on ABMC systems though?
>>>>
>>>> I would say yes. For some reason user(hardware or software issues) is not
>>>> able to use ABMC mode, they have an option to go back to legacy mode.
>>>
>>> I see. Should this perhaps be protected behind the resctrl "debug" mount option?
>>
>> The debug option gives wrong impression. It is better to keep the option
>> open to enable the feature in normal mode.
> 
> You mentioned that it would only be needed when there are hardware or
> software issues ... so debug does sound appropriate. Could you please give
> an example of how debug option gives wrong impression? Why would you want
> users to keep using "legacy" mode on an ABMC system?

I don't have a strong argument here. I am fine as long as there is a way
to go back to legacy mode if required. We can provide legacy option in
debug mode.

> 
> ...
> 
>>> For example, if I understand correctly, theoretically, when ABMC is enabled then
>>> "num_rmids" can be U32_MAX (after a quick look it is not clear to me why r->num_rmid
>>> is not unsigned, tbd if number of directories may also be limited by kernfs).
>>> User space could theoretically create more monitor groups than the number of
>>> rmids that a resource claims to support using current upstream enumeration.
>>
>> CPU or task association still uses PQR_ASSOC(MSR C8Fh). There are only 11
>> bits(depends on specific h/w) to represent RMIDs. So, we cannot create
>> more than this limit(r->num_rmid).
>>
>> In case of ABMC, h/w uses another counter(mbm_assignable_counters) with
>> RMID to assign the monitoring. So, assignment limit is
>> mbm_assignable_counters. The number of mon groups limit is still r->num_rmid.
> 
> I see. Thank you for clarifying. This does make enabling simpler and one
> less user interface item that needs changing.
> 
> ...
> 
>>>> 2. /sys/fs/resctrl/monitor_state.
>>>> This can used to individually assign or unassign the counters in each group.
>>>>
>>>> When assigned:
>>>> #cat /sys/fs/resctrl/monitor_state
>>>> 0=total-assign,local-assign;1=total-assign,local-assign
>>>>
>>>> When unassigned:
>>>> #cat /sys/fs/resctrl/monitor_state
>>>> 0=total-unassign,local-unassign;1=total-unassign,local-unassign
>>>>
>>>>
>>>> Thoughts?
>>>
>>> How do you expect this interface to be used? I understand the mechanics
>>> of this interface but on a higher level, do you expect user space to
>>> once in a while assign a new counter to a single event or monitor group
>>> (for which a fine grained interface works) or do you expect user space to
>>> shift multiple counters across several monitor events at intervals?
>>
>> I think we should provide both the options. I was thinking of providing
>> fine grained interface first.
> 
> Could you please provide a motivation for why two interfaces, one inefficient
> and one not, should be created and maintained? Users can still do fine grained
> assignment with a global assignment interface.

Lets consider one by one.

1. Fine grained assignment.

It will be part of the mongroup(or control mongroup). User has the access
to the group and can query the group's current status before assigning or
unassigning.

   $cd /sys/fs/resctrl/ctrl_mon1
   $cat /sys/fs/resctrl/ctrl_mon1/monitor_state
       0=total-unassign,local-unassign;1=total-unassign,local-unassign;

Assign the total event

  $echo 0=total-assign > /sys/fs/resctrl/ctrl_mon1/monitor_state

Assign the local event

   $echo 0=local-assign > /sys/fs/resctrl/ctrl_mon1/monitor_state

Assign both events:

   $echo 0=total-assign,local-assign > /sys/fs/resctrl/ctrl_mon1/monitor_state

Check the assignment status.

   $cat /sys/fs/resctrl/ctrl_mon1/monitor_state
       0=total-assign,local-assign;1=total-unassign,local-unassign;

-User interface is simple.

-Assignment will fail if all the h/w counters are exhausted. User needs to
unassign a counter from another group and use that counter here. This can
be done just querying the monitor state of another group.

-Monitor group's details(cpus, tasks) are part of the group. So, it is
better to have assignment state inside the group.

Note: Used interface names here just to give example.


2. global assignment:

I would assume the interface file will be in /sys/fs/resctrl/info/L3_MON/
directory.

In case there are 100 mongroups, we need to have a way to list current
assignment status for these groups. I am not sure how to list status of
these 100 groups.

If user is wants to assign the local event(or total) in a specific group
in this list of 100 groups, I am not sure how to provide interface for
that. Should we pass the name of mongroup? That will involve looping
through using the call kernfs_walk_and_get. This may be ok if we are
dealing with very small number of groups.

-- 
Thanks
Babu Moger