Re: [PATCH v2 2/9] selftests/resctrl: Do not store iMC counter value in counter config structure

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Ilpo,

On 3/6/26 1:51 AM, Ilpo Järvinen wrote:
> On Tue, 3 Mar 2026, Reinette Chatre wrote:

  
>> @@ -312,23 +311,23 @@ static int get_read_mem_bw_imc(float *bw_imc)
>>  	 * Take overflow into consideration before calculating total bandwidth.
>>  	 */
>>  	for (imc = 0; imc < imcs; imc++) {
>> +		struct membw_read_format return_value;
>>  		struct imc_counter_config *r =
>>  			&imc_counters_config[imc];
>>  
>> -		if (read(r->fd, &r->return_value,
>> -			 sizeof(struct membw_read_format)) == -1) {
>> +		if (read(r->fd, &return_value, sizeof(return_value)) == -1) {
>>  			ksft_perror("Couldn't get read bandwidth through iMC");
>>  			return -1;
>>  		}
>>  
>> -		__u64 r_time_enabled = r->return_value.time_enabled;
>> -		__u64 r_time_running = r->return_value.time_running;
>> +		__u64 r_time_enabled = return_value.time_enabled;
>> +		__u64 r_time_running = return_value.time_running;
>>  
>>  		if (r_time_enabled != r_time_running)
>>  			of_mul_read = (float)r_time_enabled /
>>  					(float)r_time_running;
>>  
>> -		reads += r->return_value.value * of_mul_read * SCALE;
>> +		reads += return_value.value * of_mul_read * SCALE;
>>  	}
> 
> This looks mostly okay though here too I don't like the variable name. 
> Something like "measurement" would tell what it is much better than overly 
> vague "return_value".

I agree. Will change to "measurement".

> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 

Thank you very much.

Reinette