Re: [PATCH] x86/pmstat: fold two allocations in get_cpufreq_para()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 25/03/2025 2:00 pm, Jan Beulich wrote:
> On 25.03.2025 14:52, Andrew Cooper wrote:
>> On 25/03/2025 12:53 pm, Jan Beulich wrote:
>>> --- a/xen/drivers/acpi/pmstat.c
>>> +++ b/xen/drivers/acpi/pmstat.c
>>> @@ -219,25 +218,22 @@ static int get_cpufreq_para(struct xen_s
>>>          return -EAGAIN;
>>>      }
>>>  
>>> -    if ( !(affected_cpus = xzalloc_array(uint32_t, 
>>> op->u.get_para.cpu_num)) )
>>> +    if ( !(data = xzalloc_array(uint32_t,
>>> +                                max(op->u.get_para.cpu_num,
>>> +                                    op->u.get_para.freq_num))) )
>>>          return -ENOMEM;
>>> +
>>>      for_each_cpu(cpu, policy->cpus)
>>> -        affected_cpus[j++] = cpu;
>>> +        data[i++] = cpu;
>>>      ret = copy_to_guest(op->u.get_para.affected_cpus,
>>> -                       affected_cpus, op->u.get_para.cpu_num);
>>> -    xfree(affected_cpus);
>>> -    if ( ret )
>>> -        return ret;
>>> +                        data, op->u.get_para.cpu_num);
>>>  
>>> -    if ( !(scaling_available_frequencies =
>>> -           xzalloc_array(uint32_t, op->u.get_para.freq_num)) )
>>> -        return -ENOMEM;
>>>      for ( i = 0; i < op->u.get_para.freq_num; i++ )
>>> -        scaling_available_frequencies[i] =
>>> -                        pmpt->perf.states[i].core_frequency * 1000;
>>> +        data[i] = pmpt->perf.states[i].core_frequency * 1000;
>>>      ret = copy_to_guest(op->u.get_para.scaling_available_frequencies,
>>> -                   scaling_available_frequencies, op->u.get_para.freq_num);
>>> -    xfree(scaling_available_frequencies);
>>> +                        data, op->u.get_para.freq_num) ?: ret;
>>> +
>>> +    xfree(data);
>>>      if ( ret )
>>>          return ret;
>>>  
>> Not altered by this patch, but `ret` is bogus here.
>>
>> It's the number of bytes not copied, and needs transforming into -EFAULT
>> here and later.
> Oh, right - I noticed this when making the patch, then forgot again. I can
> make another patch, unless you have one in the works already.

I've not started one.  Please go ahead.

~Andrew