mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Moger, Babu" <bmoger@amd.com>
To: Reinette Chatre <reinette.chatre@intel.com>,
	Peter Newman <peternewman@google.com>
Cc: babu.moger@amd.com, corbet@lwn.net, fenghua.yu@intel.com,
	tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	dave.hansen@linux.intel.com, 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, maciej.wieczor-retman@intel.com,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	eranian@google.com, james.morse@arm.com
Subject: Re: [PATCH v5 00/20] x86/resctrl : Support AMD Assignable Bandwidth Monitoring Counters (ABMC)
Date: Fri, 2 Aug 2024 13:49:06 -0500	[thread overview]
Message-ID: <05786b1c-cc4e-26ea-581d-3aad3b594f91@amd.com> (raw)
In-Reply-To: <b3babdac-da08-4dfd-9544-47db31d574f5@intel.com>

Hi Peter/Reinette,

On 8/2/2024 11:13 AM, Reinette Chatre wrote:
> Hi Peter,
> 
> On 8/1/24 3:45 PM, Peter Newman wrote:
>> On Thu, Aug 1, 2024 at 2:50 PM Reinette Chatre
>> <reinette.chatre@intel.com> wrote:
>>> On 7/17/24 10:19 AM, Moger, Babu wrote:
>>>> On 7/12/24 17:03, Reinette Chatre wrote:
>>>>> On 7/3/24 2:48 PM, Babu Moger wrote:
> 
>>>>>> # Examples
>>>>>>
>>>>>> a. Check if ABMC support is available
>>>>>>       #mount -t resctrl resctrl /sys/fs/resctrl/
>>>>>>
>>>>>>       #cat /sys/fs/resctrl/info/L3_MON/mbm_mode
>>>>>>       [abmc]
>>>>>>       legacy
>>>>>>
>>>>>>       Linux kernel detected ABMC feature and it is enabled.
>>>>>
>>>>> How about renaming "abmc" to "mbm_cntrs"? This will match the 
>>>>> num_mbm_cntrs
>>>>> info file and be the final step to make this generic so that another
>>>>> architecture
>>>>> can more easily support assignining hardware counters without 
>>>>> needing to call
>>>>> the feature AMD's "abmc".
>>>>
>>>> I think we aleady settled this with "mbm_cntr_assignable".
>>>>
>>>> For soft-RMID" it will be mbm_sw_assignable.
>>>
>>> Maybe getting a bit long but how about "mbm_cntr_sw_assignable" to match
>>> with the term "mbm_cntr" in accompanying "num_mbm_cntrs"?
>>
>> My users are pushing for a consistent interface regardless of whether
>> counter assignment is implemented in hardware or software, so I would
>> like to avoid exposing implementation differences in the interface
>> where possible.
> 
> This seems a reasonable ask but can we be confident that if hardware
> supports assignable counters then there will never be a reason to use
> software assignable counters? (This needs to also consider how/if Arm
> may use this feature.)
> 
> I am of course not familiar with details of the software implementation
> - could there be benefits to using it even if hardware counters are
> supported?
> 
> What I would like to avoid is future complexity of needing a new 
> mount/config
> option that user space needs to use to select if a single 
> "mbm_cntr_assignable"
> is backed by hardware or software.
> 
>> The main semantic difference with SW assignments is that it is not
>> possible to assign counters to individual events. Because the
>> implementation is assigning RMIDs to groups, assignment results in all
>> events being counted.
>>
>> I was considering introducing a boolean mbm_assign_events node to
>> indicate whether assigning individual events is supported. If true,
>> num_mbm_cntrs indicates the number of events which can be counted,
>> otherwise it indicates the number of groups to which counters can be
>> assigned and attempting to assign a single event is silently upgraded
>> to assigning counters to all events in the group.
> 
> How were you envisioning your users using the control file ("mbm_control")
> in these scenarios? Does this file's interface even work for SW assignment
> scenarios?
> 
> Users should expect consistent interface for "mbm_control" also.
> 
> It sounds to me that a potential "mbm_assign_events" will be false for SW
> assignments. That would mean that "num_mbm_cntrs" will
> contain the number of groups to which counters can be assigned?
> Would user space be required to always enable all flags (enable all 
> events) of
> all domains to the same values ... or would enabling of one flag (one 
> event)
> in one domain automatically result in all flags (all events) enabled for 
> all
> domains ... or would enabling of one flag (one event) in one domain only 
> appear
> to user space to be enabled while in reality all flags/events are 
> actually enabled?
> 
>> However, If we don't expect to see these semantics in any other
>> implementation, these semantics could be implicit in the definition of
>> a SW assignable counter.
> 
> It is not clear to me how implementation differences between hardware
> and software assignment can be hidden from user space. It is possible
> to let user space enable individual events and then silently upgrade it
> to all events. I see two options here, either "mbm_control" needs to
> explicitly show this "silent upgrade" so that user space knows which
> events are actually enabled, or "mbm_control" only shows flags/events 
> enabled
> from user space perspective. In the former scenario, this needs more
> user space support since a generic user space cannot be confident which
> flags are set after writing to "mbm_control". In the latter scenario,
> meaning of "num_mbm_cntrs" becomes unclear since user space is expected
> to rely on it to know which events can be enabled and if some are
> actually "silently enabled" when user space still thinks it needs to be
> enabled the number of available counters becomes vague.
> 
> It is not clear to me how to present hardware and software assignable
> counters with a single consistent interface. Actually, what if the
> "mbm_mode" is what distinguishes how counters are assigned instead of how
> it is backed (hw vs sw)? What if, instead of "mbm_cntr_assignable" and
> "mbm_cntr_sw_assignable" MBM modes the terms "mbm_cntr_event_assignable"
> and "mbm_cntr_group_assignable" is used? Could that replace a
> potential "mbm_assign_events" while also supporting user space in
> interactions with "mbm_control"?

If I understand correctly, current interface might work for both the sw 
and hw assignments.

In case of SW assignment, you need to manage two counters at context 
switch time. One for total event and one for local event. Basically, you 
need to calculate delta for both events. You need to do rmid read for 
both events and then calculate the delta.

If the user assigns only one event you do the calculations only for the 
event user is interested in. That will save cycles as well. In this case 
"mbm_control" will report as one one event is assigned.

In many cases user will not interested in both the events. Also events 
are configurable so users can get what they want with just one event.

Does that make sense?

-- 
- Babu Moger

  reply	other threads:[~2024-08-02 18:49 UTC|newest]

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-03 21:48 Babu Moger
2024-07-03 21:48 ` [PATCH v5 01/20] x86/cpufeatures: Add support for " Babu Moger
2024-07-12 21:55   ` Reinette Chatre
2024-07-15 18:36     ` Moger, Babu
2024-07-03 21:48 ` [PATCH v5 02/20] x86/resctrl: Add ABMC feature in the command line options Babu Moger
2024-07-03 21:48 ` [PATCH v5 03/20] x86/resctrl: Consolidate monitoring related data from rdt_resource Babu Moger
2024-07-12 21:57   ` Reinette Chatre
2024-07-15 19:05     ` Moger, Babu
2024-07-03 21:48 ` [PATCH v5 04/20] x86/resctrl: Detect Assignable Bandwidth Monitoring feature details Babu Moger
2024-07-12 22:04   ` Reinette Chatre
2024-07-15 20:04     ` Moger, Babu
2024-07-16 15:11       ` Reinette Chatre
2024-07-03 21:48 ` [PATCH v5 05/20] x86/resctrl: Introduce resctrl_file_fflags_init() to initialize fflags Babu Moger
2024-07-12 22:04   ` Reinette Chatre
2024-07-03 21:48 ` [PATCH v5 06/20] x86/resctrl: Add support to enable/disable AMD ABMC feature Babu Moger
2024-07-12 22:05   ` Reinette Chatre
2024-07-16 15:13     ` Moger, Babu
2024-07-16 17:51       ` Reinette Chatre
2024-07-16 18:48         ` Moger, Babu
2024-07-16 20:41           ` Reinette Chatre
2024-07-18 21:11         ` Moger, Babu
2024-08-16 16:29     ` James Morse
2024-07-03 21:48 ` [PATCH v5 07/20] x86/resctrl: Introduce the interface to display monitor mode Babu Moger
2024-07-12 22:06   ` Reinette Chatre
2024-07-16 16:51     ` Moger, Babu
2024-07-03 21:48 ` [PATCH v5 08/20] x86/resctrl: Introduce interface to display number of monitoring counters Babu Moger
2024-07-03 21:48 ` [PATCH v5 09/20] x86/resctrl: Initialize monitor counters bitmap Babu Moger
2024-07-12 22:07   ` Reinette Chatre
2024-07-16 17:59     ` Moger, Babu
2024-07-26 22:48   ` Peter Newman
2024-07-26 23:53     ` Moger, Babu
2024-08-01 21:05     ` Reinette Chatre
2024-07-03 21:48 ` [PATCH v5 10/20] x86/resctrl: Introduce mbm_total_cfg and mbm_local_cfg Babu Moger
2024-07-12 22:08   ` Reinette Chatre
2024-07-16 19:21     ` Moger, Babu
2024-07-16 20:42       ` Reinette Chatre
2024-07-16 22:43         ` Moger, Babu
2024-07-03 21:48 ` [PATCH v5 11/20] x86/resctrl: Remove MSR reading of event configuration value Babu Moger
2024-07-12 22:10   ` Reinette Chatre
2024-07-16 19:34     ` Moger, Babu
2024-07-03 21:48 ` [PATCH v5 12/20] x86/resctrl: Add data structures and definitions for ABMC assignment Babu Moger
2024-07-12 22:13   ` Reinette Chatre
2024-07-16 20:24     ` Moger, Babu
2024-07-03 21:48 ` [PATCH v5 13/20] x86/resctrl: Add the interface to assign hardware counter Babu Moger
2024-07-12 22:09   ` Reinette Chatre
2024-07-16 20:45     ` Moger, Babu
2024-07-03 21:48 ` [PATCH v5 14/20] x86/resctrl: Add the interface to unassign " Babu Moger
2024-07-03 21:48 ` [PATCH v5 15/20] x86/resctrl: Assign/unassign counters by default when ABMC is enabled Babu Moger
2024-07-12 22:10   ` Reinette Chatre
2024-07-16 20:58     ` Moger, Babu
2024-07-26 23:22   ` Peter Newman
2024-07-26 23:57     ` Moger, Babu
2024-07-03 21:48 ` [PATCH v5 16/20] x86/resctrl: Report "Unassigned" for MBM events in ABMC mode Babu Moger
2024-07-12 22:13   ` Reinette Chatre
2024-07-16 21:04     ` Moger, Babu
2024-07-13 20:26   ` Markus Elfring
2024-07-03 21:48 ` [PATCH v5 17/20] x86/resctrl: Introduce the interface switch between monitor modes Babu Moger
2024-07-12 22:14   ` Reinette Chatre
2024-07-16 22:46     ` Moger, Babu
2024-07-13  7:15   ` Markus Elfring
2024-07-03 21:48 ` [PATCH v5 18/20] x86/resctrl: Enable AMD ABMC feature by default when supported Babu Moger
2024-07-12 22:15   ` Reinette Chatre
2024-07-16 23:23     ` Moger, Babu
2024-07-26  0:16       ` Moger, Babu
2024-08-01 21:40         ` Reinette Chatre
2024-07-03 21:48 ` [PATCH v5 19/20] x86/resctrl: Introduce interface to list monitor states of all the groups Babu Moger
2024-07-12 22:16   ` Reinette Chatre
2024-07-17 15:22     ` Moger, Babu
2024-08-01 21:37       ` Reinette Chatre
2024-08-02 16:10         ` Moger, Babu
2024-07-03 21:48 ` [PATCH v5 20/20] x86/resctrl: Introduce interface to modify assignment states of " Babu Moger
2024-07-12 22:17   ` Reinette Chatre
2024-07-17 16:22     ` Moger, Babu
2024-07-25  0:03   ` Peter Newman
2024-07-25  1:22     ` Moger, Babu
2024-07-25 17:11       ` Peter Newman
2024-07-25 17:28         ` Moger, Babu
2024-08-01 18:56           ` Reinette Chatre
2024-08-01 19:40             ` Moger, Babu
2024-07-12 22:03 ` [PATCH v5 00/20] x86/resctrl : Support AMD Assignable Bandwidth Monitoring Counters (ABMC) Reinette Chatre
2024-07-17 17:19   ` Moger, Babu
2024-08-01 21:49     ` Reinette Chatre
2024-08-01 22:45       ` Peter Newman
2024-08-02 16:13         ` Reinette Chatre
2024-08-02 18:49           ` Moger, Babu [this message]
2024-08-02 19:13             ` Peter Newman
2024-08-02 20:23               ` Moger, Babu
2024-08-02 18:49           ` Peter Newman
2024-08-02 20:38             ` Moger, Babu
2024-08-02 20:55             ` Reinette Chatre
2024-08-02 22:50               ` Peter Newman
2024-08-14 17:37                 ` Reinette Chatre
2024-08-15 23:06                   ` Peter Newman
2024-08-16  1:45                     ` Reinette Chatre
2024-08-03  0:49               ` Moger, Babu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=05786b1c-cc4e-26ea-581d-3aad3b594f91@amd.com \
    --to=bmoger@amd.com \
    --cc=babu.moger@amd.com \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=daniel.sneddon@linux.intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=eranian@google.com \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=james.morse@arm.com \
    --cc=jithu.joseph@intel.com \
    --cc=jmattson@google.com \
    --cc=jpoimboe@kernel.org \
    --cc=kai.huang@intel.com \
    --cc=kan.liang@linux.intel.com \
    --cc=kim.phillips@amd.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=leitao@debian.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=maciej.wieczor-retman@intel.com \
    --cc=mingo@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peternewman@google.com \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=reinette.chatre@intel.com \
    --cc=rick.p.edgecombe@intel.com \
    --cc=sandipan.das@amd.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=x86@kernel.org \
    --cc=yanjiewtw@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®