Hi Reinette,
On 6/14/24 13:37, Reinette Chatre wrote:
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.
Sure.
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@amd.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.
Ok.
{ 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?
Yes.
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.
Will correct it.
- 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.
Sure.-- Thanks Babu Moger