Re: [PATCH v2 02/70] xen/sort: Switch to an extern inline implementation

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 14/02/2022 13:13, Bertrand Marquis wrote:
> Hi Andrew,
>
>> On 14 Feb 2022, at 12:50, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>>
>> There are exactly 3 callers of sort() in the hypervisor.  Callbacks in a 
>> tight
>> loop like this are problematic for performance, especially with Spectre v2
>> protections, which is why extern inline is used commonly by libraries.
>>
>> Both ARM callers pass in NULL for the swap function, and while this might 
>> seem
>> like an attractive option at first, it causes generic_swap() to be used, 
>> which
>> forced a byte-wise copy.  Provide real swap functions so the compiler can
>> optimise properly, which is very important for ARM downstreams where
>> milliseconds until the system is up matters.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> Just one comment fix after, with it fixed for the arm part:
>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Thanks.

>> diff --git a/xen/include/xen/sort.h b/xen/include/xen/sort.h
>> index a403652948e7..01479ea44606 100644
>> --- a/xen/include/xen/sort.h
>> +++ b/xen/include/xen/sort.h
>> @@ -3,8 +3,61 @@
>>
>> #include <xen/types.h>
>>
>> +/*
>> + * sort - sort an array of elements
>> + * @base: pointer to data to sort
>> + * @num: number of elements
>> + * @size: size of each element
>> + * @cmp: pointer to comparison function
>> + * @swap: pointer to swap function or NULL
> The function is not accepting anymore to have NULL as parameter.
> The comment should be fixed here.

Will fix.

~Andrew