linux-kernel - Re: [PATCH v3 1/4] selftests/resctrl: Rename variables and functions to generic names

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: Fri, 14 Jun 2024 11:37:42 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Babu Moger <babu.moger@....com>, <fenghua.yu@...el.com>,
	<shuah@...nel.org>
CC: <linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
	<ilpo.jarvinen@...ux.intel.com>, <maciej.wieczor-retman@...el.com>,
	<peternewman@...gle.com>, <eranian@...gle.com>
Subject: Re: [PATCH v3 1/4] selftests/resctrl: Rename variables and functions
 to generic names

Hi Babu,

On 6/5/24 3:45 PM, Babu Moger wrote:
> In an effort to support MBM and MBA tests for AMD, renaming for variable
> and functions to generic names. For Intel, the memory controller is called

Changelog usually starts with some context and then problem to be solved. What
the patch does follows that. Maybe just something like:

	For Intel, the memory controller is called Integrated Memory
	Controller (IMC). For AMD, it is called Unified Memory Controller (UMC).
                                                                                 
	Change the names of variables and functions from imc to mc in preparation
	for support of MBM and MBA tests for AMD.
                                                                                 
	No functional change.

> Integrated Memory Controllers (IMC). For AMD, it is called Unified
> Memory Controller (UMC).
> 
> Change the names of variables and functions from imc (Integrated memory
> controller) to mc(Memory Controller). No functional change.
> 
> Signed-off-by: Babu Moger <babu.moger@....com>
> ---

...

> @@ -349,10 +350,10 @@ static void do_imc_mem_bw_test(void)
>    *
>    * Return: = 0 on success. < 0 on failure.
>    */
> -static int get_mem_bw_imc(const char *bw_report, float *bw_imc)
> +static int get_mem_bw_mc(const char *bw_report, float *bw_mc)

The name of the function is expected to be changed in the function comments
also.

>   {
>   	float reads, writes, of_mul_read, of_mul_write;
> -	int imc;
> +	int mc;
>   
>   	/* Start all iMC counters to log values (both read and write) */

Was this intended to be MC?

>   	reads = 0, writes = 0, of_mul_read = 1, of_mul_write = 1;
> @@ -361,21 +362,21 @@ static int get_mem_bw_imc(const char *bw_report, float *bw_imc)
>   	 * Get results which are stored in struct type imc_counter_config
>   	 * Take overflow into consideration before calculating total bandwidth.
>   	 */

In below snippet imc_counter_config is renamed to mc_counter_config yet the
comment above was not changed to match.

> -	for (imc = 0; imc < imcs; imc++) {
> -		struct imc_counter_config *r =
> -			&imc_counters_config[imc][READ];
> -		struct imc_counter_config *w =
> -			&imc_counters_config[imc][WRITE];
> +	for (mc = 0; mc < mcs; mc++) {
> +		struct mc_counter_config *r =
> +			&mc_counters_config[mc][READ];
> +		struct mc_counter_config *w =
> +			&mc_counters_config[mc][WRITE];
>   

I noticed a couple of misses by just looking at this patch. You can
use grep to ensure that this patch does what you intend.

Reinette