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 <[email protected]>
> 

Thank you very much.

Reinette


Reply via email to