[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 5/7] xen/memory: Improve compat XENMEM_acquire_resource handling
On 15/01/2021 15:37, Jan Beulich wrote: > On 12.01.2021 20:48, Andrew Cooper wrote: >> @@ -446,6 +430,31 @@ int compat_memory_op(unsigned int cmd, >> XEN_GUEST_HANDLE_PARAM(void) compat) >> >> #undef XLAT_mem_acquire_resource_HNDL_frame_list >> >> + if ( xen_frame_list && cmp.mar.nr_frames ) >> + { >> + /* >> + * frame_list is an input for translated guests, and an >> output >> + * for untranslated guests. Only copy in for translated >> guests. >> + */ >> + if ( paging_mode_translate(currd) ) >> + { >> + compat_pfn_t *compat_frame_list = (void >> *)xen_frame_list; >> + >> + if ( !compat_handle_okay(cmp.mar.frame_list, >> + cmp.mar.nr_frames) || >> + __copy_from_compat_offset( >> + compat_frame_list, cmp.mar.frame_list, >> + 0, cmp.mar.nr_frames) ) >> + return -EFAULT; >> + >> + /* >> + * Iterate backwards over compat_frame_list[] expanding >> + * compat_pfn_t to xen_pfn_t in place. >> + */ >> + for ( int x = cmp.mar.nr_frames - 1; x >= 0; --x ) >> + xen_frame_list[x] = compat_frame_list[x]; > Just as a nit, without requiring you to adjust (but with the > request to consider adjusting) - x getting used as array index > would generally suggest it wants to be an unsigned type (despite > me guessing the compiler ought to be able to avoid an explicit > sign-extension for the actual memory accesses): > > for ( unsigned int x = cmp.mar.nr_frames; x--; ) > xen_frame_list[x] = compat_frame_list[x]; Signed numbers are not inherently evil. The range of x is between 0 and 1020 so there is no issue with failing to enter the loop. It is the compilers job to make this optimisation. It is a very poor use of a developers time to write logic which takes extra effort to figure out whether it is correct or not. You know what my attitude will be towards a compiler which is incapable of making the optimisation, and you've got to go back a decade to find a processor old enough to not have identical performance between the unoptimised signed and unsigned forms. Both signs of numbers have their uses, and a rigid policy of using unsigned numbers does more harm than good (in this case, concerning the simplicity of the code). ~Andrew
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |