linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Reinette Chatre <reinette.chatre@intel.com>
To: Tony Luck <tony.luck@intel.com>
Cc: <linux-doc@vger.kernel.org>, Fenghua Yu <fenghua.yu@intel.com>,
	"Peter Newman" <peternewman@google.com>,
	James Morse <james.morse@arm.com>,
	"Babu Moger" <babu.moger@amd.com>,
	Drew Fustini <dfustini@baylibre.com>, <x86@kernel.org>,
	<linux-kernel@vger.kernel.org>, <patches@lists.linux.dev>
Subject: Re: [PATCH] Documentation/x86: Document resctrl bandwidth control units are MiB
Date: Mon, 1 Apr 2024 15:44:03 -0700	[thread overview]
Message-ID: <d6a649fd-0bd3-4777-acb3-2b9362131796@intel.com> (raw)
In-Reply-To: <cf59f587-9ca2-4f0d-b412-69b559acbabb@intel.com>

Hi Tony,

On 3/29/2024 9:37 AM, Reinette Chatre wrote:
> On 3/29/2024 8:31 AM, Tony Luck wrote:
>> On Thu, Mar 28, 2024 at 06:01:33PM -0700, Reinette Chatre wrote:
>>> On 3/22/2024 11:20 AM, Tony Luck wrote:
>>>> The memory bandwidth software controller uses 2^20 units rather than
>>>> 10^6. See mbm_bw_count() which computes bandwidth using the "SZ_1M"
>>>> Linux define for 0x00100000.
>>>>
>>>> Update the documentation to use MiB when describing this feature.
>>>> It's too late to fix the mount option "mba_MBps" as that is now an
>>>> established user interface.
>>>
>>> I see that this is merged already but I do not think this is correct.
>>
>> I was surprised that Ingo merged it without giving folks a chance to
>> comment.
>>
>>> Shouldn't the implementation be fixed instead? Looking at the implementation
>>> the intent appears to be clear that the goal is to have bandwidth be
>>> MBps .... that is when looking from documentation to the define
>>> (MBA_MAX_MBPS) to the comments of the function you reference above
>>> mbm_bw_count(). For example, "...and delta bandwidth in MBps ..."
>>> and "...maintain values in MBps..."
>>
>> Difficult to be sure of intent. But in general when people talk about
>> "megabytes" in the context of memory they mean 2^20. Storage capacity
>> on computers was originally in 2^20 units until the marketing teams
>> at disk drive manufacturers realized they could print numbers 4.8% bigger
>> on their products by using SI unit 10^6 Mega prefix (rising to 7.3% with
>> Giga and 10% with Tera).
> 
> This is not so obvious to me. I hear what you are saying about storage
> capacity but the topic here is memory bandwidth and here I find the custom
> to be that MB/s means 10^6 bytes per second. That is looking from how DDR
> bandwidth is documented to how benchmarks like
> https://github.com/intel/memory-bandwidth-benchmarks report the data, to
> what wikipedia says in https://en.wikipedia.org/wiki/Memory_bandwidth.
> 
> I also took a sample of what the perf side of things may look like
> and, for example, when looking at;
> tools/perf/pmu-events/arch/x86/sapphirerapids/spr-metrics.json
> I understand that the custom for bandwidth is MB/s. For example:
> 
>     {
>         "BriefDescription": "DDR memory read bandwidth (MB/sec)",
>         "MetricExpr": "UNC_M_CAS_COUNT.RD * 64 / 1e6 / duration_time",
>         "MetricName": "memory_bandwidth_read",
>         "ScaleUnit": "1MB/s"
>     },
> 

(Thanks to Kan Liang for explaining this to me.)

As an update on this, the perf side does not seem to be as consistent as
I first interpreted it to be. There appears to be a "kernel side" and
"user side" related to memory bandwidth data.

On the kernel side, users can refer directly to:
/sys/bus/event_source/devices/uncore_imc_*/events to read the
UNC_M_CAS_COUNT.RD and UNC_M_CAS_COUNT.WR data and this appears to
be intended to be consumed as MiB/s as per:
$ /sys/bus/event_source/devices/uncore_imc_0/events/cas_count_read.unit
MiB

On the user side, using perf from userspace the metrics are obtained
from the relevant json file as quoted above, and thus when using perf
from the command line the data is in MB/sec, for example:

$ perf list
[SNIP]
  llc_miss_local_memory_bandwidth_read
       [Bandwidth (MB/sec) of read requests that miss the last level cache (LLC) and go to local memory]
  llc_miss_local_memory_bandwidth_write
       [Bandwidth (MB/sec) of write requests that miss the last level cache (LLC) and go to local memory]
  llc_miss_remote_memory_bandwidth_read
       [Bandwidth (MB/sec) of read requests that miss the last level cache (LLC) and go to remote memory]
  llc_miss_remote_memory_bandwidth_write
       [Bandwidth (MB/sec) of write requests that miss the last level cache (LLC) and go to remote memory]
  loads_per_instr
       [The ratio of number of completed memory load instructions to the total number completed instructions]
  memory_bandwidth_read
       [DDR memory read bandwidth (MB/sec)]
  memory_bandwidth_total
       [DDR memory bandwidth (MB/sec)]
  memory_bandwidth_write
       [DDR memory write bandwidth (MB/sec)]
[SNIP]


It appears that there is no custom here and it may just be somebody's preference?

Reinette

  reply	other threads:[~2024-04-01 22:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-22 18:20 [PATCH] Documentation/x86: Document resctrl bandwidth control units are MiB Tony Luck
2024-03-24  3:07 ` [tip: x86/urgent] Documentation/x86: Document that " tip-bot2 for Tony Luck
2024-03-29  1:01 ` [PATCH] Documentation/x86: Document " Reinette Chatre
2024-03-29 15:31   ` Tony Luck
2024-03-29 16:37     ` Reinette Chatre
2024-04-01 22:44       ` Reinette Chatre [this message]
2024-04-01 23:03         ` Luck, Tony
2024-04-02  2:26           ` Reinette Chatre
2024-04-02 15:43             ` Luck, Tony

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=d6a649fd-0bd3-4777-acb3-2b9362131796@intel.com \
    --to=reinette.chatre@intel.com \
    --cc=babu.moger@amd.com \
    --cc=dfustini@baylibre.com \
    --cc=fenghua.yu@intel.com \
    --cc=james.morse@arm.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=peternewman@google.com \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).