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

Reinette Chatre
2026-03-06T19:25:16Z

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