mbox series

[v3,00/16] selftests/resctrl: resctrl_val() related cleanups & improvements

Message ID 20240408163247.3224-1-ilpo.jarvinen@linux.intel.com (mailing list archive)
Headers show
Series selftests/resctrl: resctrl_val() related cleanups & improvements | expand

Message

Ilpo Järvinen April 8, 2024, 4:32 p.m. UTC
Hi all,

This series does a number of cleanups into resctrl_val() and
generalizes it by removing test name specific handling from the
function.

One of the changes improves MBA/MBM measurement by narrowing down the
period the resctrl FS derived memory bandwidth numbers are measured
over. My feel is it didn't cause noticeable difference into the numbers
because they're generally good anyway except for the small number of
outliers. To see the impact on outliers, I'd need to setup a test to
run large number of replications and do a statistical analysis, which
I've not spent my time on. Even without the statistical analysis, the
new way to measure seems obviously better and makes sense even if I
cannot see a major improvement with the setup I'm using.

This series has some conflicts with SNC series from Maciej (Maciej has
privately agreed to base his series on top of this series) and also
with the MBA/MBM series from Babu.

--
 i.

v3:
- Rename init functions to <testname>_init()
- Replace for loops with READ+WRITE statements for clarity
- Don't drop Return: entry from perf_open_imc_mem_bw() func comment
- New patch: Fix closing of IMC fds in case of error
- New patch: Make "bandwidth" consistent in comments & prints
- New patch: Simplify mem bandwidth file code
- Remove wrong comment
- Changed grp_name check to return -1 on fail (internal sanity check)

v2:
- Resolved conflicts with kselftest/next
- Spaces -> tabs correction

Ilpo Järvinen (16):
  selftests/resctrl: Open get_mem_bw_imc() fd for loops
  selftests/resctrl: Calculate resctrl FS derived mem bw over sleep(1)
    only
  selftests/resctrl: Fix closing IMC fds on error
  selftests/resctrl: Make "bandwidth" consistent in comments & prints
  selftests/resctrl: Consolidate get_domain_id() into resctrl_val()
  selftests/resctrl: Use correct type for pids
  selftests/resctrl: Cleanup bm_pid and ppid usage & limit scope
  selftests/resctrl: Rename measure_vals() to measure_mem_bw_vals() &
    document
  selftests/resctrl: Simplify mem bandwidth file code for MBA & MBM
    tests
  selftests/resctrl: Add ->measure() callback to resctrl_val_param
  selftests/resctrl: Add ->init() callback into resctrl_val_param
  selftests/resctrl: Simplify bandwidth report type handling
  selftests/resctrl: Make some strings passed to resctrlfs functions
    const
  selftests/resctrl: Convert ctrlgrp & mongrp to pointers
  selftests/resctrl: Remove mongrp from MBA test
  selftests/resctrl: Remove test name comparing from
    write_bm_pid_to_resctrl()

 tools/testing/selftests/resctrl/cache.c       |   6 +-
 tools/testing/selftests/resctrl/cat_test.c    |   5 +-
 tools/testing/selftests/resctrl/cmt_test.c    |  21 +-
 tools/testing/selftests/resctrl/mba_test.c    |  26 +-
 tools/testing/selftests/resctrl/mbm_test.c    |  25 +-
 tools/testing/selftests/resctrl/resctrl.h     |  49 ++-
 tools/testing/selftests/resctrl/resctrl_val.c | 295 +++++++-----------
 tools/testing/selftests/resctrl/resctrlfs.c   |  64 ++--
 8 files changed, 238 insertions(+), 253 deletions(-)

Comments

Shuah Khan April 24, 2024, 1:49 p.m. UTC | #1
On 4/8/24 10:32, Ilpo Järvinen wrote:
> Hi all,
> 
> This series does a number of cleanups into resctrl_val() and
> generalizes it by removing test name specific handling from the
> function.
> 
> One of the changes improves MBA/MBM measurement by narrowing down the
> period the resctrl FS derived memory bandwidth numbers are measured
> over. My feel is it didn't cause noticeable difference into the numbers
> because they're generally good anyway except for the small number of
> outliers. To see the impact on outliers, I'd need to setup a test to
> run large number of replications and do a statistical analysis, which
> I've not spent my time on. Even without the statistical analysis, the
> new way to measure seems obviously better and makes sense even if I
> cannot see a major improvement with the setup I'm using.
> 
> This series has some conflicts with SNC series from Maciej (Maciej has
> privately agreed to base his series on top of this series) and also
> with the MBA/MBM series from Babu.
> 
> --
>   i.
> 
> v3:
> - Rename init functions to <testname>_init()
> - Replace for loops with READ+WRITE statements for clarity
> - Don't drop Return: entry from perf_open_imc_mem_bw() func comment
> - New patch: Fix closing of IMC fds in case of error
> - New patch: Make "bandwidth" consistent in comments & prints
> - New patch: Simplify mem bandwidth file code
> - Remove wrong comment
> - Changed grp_name check to return -1 on fail (internal sanity check)
> 

I can apply these for Linux 6.10-rc1 once I get an Ack from Reinette.

thanks,
-- Shuah
Reinette Chatre April 25, 2024, 4:46 a.m. UTC | #2
Hi Shuah and Ilpo,

On 4/24/2024 6:49 AM, Shuah Khan wrote:
> On 4/8/24 10:32, Ilpo Järvinen wrote:
>> Hi all,
>>
>> This series does a number of cleanups into resctrl_val() and
>> generalizes it by removing test name specific handling from the
>> function.
>>
>> One of the changes improves MBA/MBM measurement by narrowing down the
>> period the resctrl FS derived memory bandwidth numbers are measured
>> over. My feel is it didn't cause noticeable difference into the numbers
>> because they're generally good anyway except for the small number of
>> outliers. To see the impact on outliers, I'd need to setup a test to
>> run large number of replications and do a statistical analysis, which
>> I've not spent my time on. Even without the statistical analysis, the
>> new way to measure seems obviously better and makes sense even if I
>> cannot see a major improvement with the setup I'm using.
>>
>> This series has some conflicts with SNC series from Maciej (Maciej has
>> privately agreed to base his series on top of this series) and also
>> with the MBA/MBM series from Babu.
>>
>> -- 
>>   i.
>>
>> v3:
>> - Rename init functions to <testname>_init()
>> - Replace for loops with READ+WRITE statements for clarity
>> - Don't drop Return: entry from perf_open_imc_mem_bw() func comment
>> - New patch: Fix closing of IMC fds in case of error
>> - New patch: Make "bandwidth" consistent in comments & prints
>> - New patch: Simplify mem bandwidth file code
>> - Remove wrong comment
>> - Changed grp_name check to return -1 on fail (internal sanity check)
>>
> 
> I can apply these for Linux 6.10-rc1 once I get an Ack from Reinette.
> 

Apologies for the delay in reviewing this. I've now provided feedback for
consideration.

Reinette