On 14/02/2022 13:13, Bertrand Marquis wrote: > Hi Andrew, > >> On 14 Feb 2022, at 12:50, Andrew Cooper <andrew.coop...@citrix.com> 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.coop...@citrix.com> >> Reviewed-by: Jan Beulich <jbeul...@suse.com> > Just one comment fix after, with it fixed for the arm part: > > Reviewed-by: Bertrand Marquis <bertrand.marq...@arm.com>
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
