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