linux-kernel - Re: [PATCH v4 08/16] selftests/resctrl: Simplify mem bandwidth file code for MBA & MBM tests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 29 May 2024 10:45:51 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	<linux-kselftest@...r.kernel.org>, Shuah Khan <shuah@...nel.org>, Babu Moger
	<babu.moger@....com>, Maciej Wieczór-Retman
	<maciej.wieczor-retman@...el.com>
CC: <linux-kernel@...r.kernel.org>, Fenghua Yu <fenghua.yu@...el.com>, "Shuah
 Khan" <skhan@...uxfoundation.org>
Subject: Re: [PATCH v4 08/16] selftests/resctrl: Simplify mem bandwidth file
 code for MBA & MBM tests

Hi Ilpo,

On 5/20/24 5:30 AM, Ilpo Järvinen wrote:
> initialize_mem_bw_resctrl() and set_mbm_path() contain complicated set
> of conditions, each yielding different file to be opened to measure
> memory bandwidth through resctrl FS. In practice, only two of them are
> used. For MBA test, ctrlgrp is always provided, and for MBM test both
> ctrlgrp and mongrp are set.
> 
> The file used differ between MBA/MBM test, however, MBM test
> unnecessarily create monitor group because resctrl FS already provides
> monitoring interface underneath any ctrlgrp too, which is what the MBA
> selftest uses.
> 
> Consolidate memory bandwidth file used to the one used by the MBA
> selftest. Remove all unused branches opening other files to simplify
> the code.
> 
> Suggested-by: Reinette Chatre <reinette.chatre@...el.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> ---

fyi ... if the write_bm_pid_to_resctrl() fix in patch #16
was closer to this change it would have made this series easier to
review. That fix is clearly needed here and its omission makes this
patch as well as later patches #14 and #15 harder to review. No
need to rework the series at this point. It is just some comment on
how a simple patch ordering change can make a series easier to
understand.

Reviewed-by: Reinette Chatre <reinette.chatre@...el.com>

Reinette