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]
Message-ID: <7cff31f8-f94f-4b76-beb9-073c369e5080@intel.com>
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ