[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7eef29f6-297b-bb2b-e0d-ccef1aa2f14@linux.intel.com>
Date: Mon, 17 Jul 2023 15:30:52 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Reinette Chatre <reinette.chatre@...el.com>
cc: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
"Wieczor-Retman, Maciej" <maciej.wieczor-retman@...el.com>,
linux-kselftest@...r.kernel.org, Shuah Khan <shuah@...nel.org>,
Shaopeng Tan <tan.shaopeng@...fujitsu.com>,
Fenghua Yu <fenghua.yu@...el.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 10/19] selftests/resctrl: Express span internally in
bytes
On Fri, 14 Jul 2023, Reinette Chatre wrote:
> On 7/14/2023 3:22 AM, Ilpo Järvinen wrote:
> > On Fri, 14 Jul 2023, Wieczor-Retman, Maciej wrote:
> >> On 14.07.2023 01:00, Reinette Chatre wrote:
> >>> Hi Ilpo,
> >>>
> >>> On 7/13/2023 6:19 AM, Ilpo Järvinen wrote:
> >>>> MBA and MBM tests to use megabytes to represent span. CMT test uses
> >>>> bytes. The difference requires run_benchmark() to size the buffer
> >>>> differently based on the test name, which in turn requires passing the
> >>>> test name into run_benchmark().
> >>>>
> >>>> Convert MBA and MBM tests to use internally bytes like CMT test to
> >>>> remove the internal inconsistency between the tests. Remove the test
> >>>> dependent buffer sizing from run_benchmark().
> >>>
> >>> If I understand correctly the intention is to always use bytes internally
> >>> and only convert to megabytes when displayed to user space. The above
> >>> implies that this takes care of the conversion but there still seems
> >>> to be places that that do not follow my understanding. For example,
> >>> resctrl_val.c:measure_vals() converts to megabytes before proceeding.
> >>
> >> Doesn't the use case inside resctrl_val.c:measure_vals() satisfy
> >> the idea of only displaying data to the user space? From my
> >> understanding it reads the number of bytes and only converts to
> >> MB when printing the value. Or did I miss some detail there?
> >
> > It's for printing there yes.
> >
> > But it's not about span in the first place so I'm not sure why it is
> > related.
> >
>
> If this change is just about how "span" is interpreted by the different
> tests then the changelog could be more specific to not create expectation
> that with this change there are no longer "bytes vs megabytes" internal
> inconsistency between MBA, MBM, and CMT tests.
The shortlog and changelog are already pretty specific in mentioning
"span" a few times :-). I added yet another "span" into the changelog's
2nd paragraph.
Your general observation about the other MB/bytes inconsistency is still
a good one so I added it also to my todo list, it just doesn't belong to
this patch (IMHO).
--
i.
Powered by blists - more mailing lists