Re: [Xen-devel] [PATCH] p2m: split mem_access into separate files

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Fri, Dec 9, 2016 at 2:27 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 08.12.16 at 23:57, <tamas.lengyel@xxxxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/mm/Makefile
>> +++ b/xen/arch/x86/mm/Makefile
>> @@ -9,6 +9,7 @@ obj-y += guest_walk_3.o
>>  obj-y += guest_walk_4.o
>>  obj-y += mem_paging.o
>>  obj-y += mem_sharing.o
>> +obj-y += mem_access.o
>
> Please honor prior (mostly?) alphabetical ordering.

I don't think there is any alphabetical ordering here. The list begins
with paging.o then goes to altp2m.o and then to guest_walk_2.o.. IMHO
sorting the list is something that should be done in a separate patch.

>
>> --- a/xen/common/mem_access.c
>> +++ b/xen/common/mem_access.c
>> @@ -24,8 +24,9 @@
>>  #include <xen/guest_access.h>
>>  #include <xen/hypercall.h>
>>  #include <xen/vm_event.h>
>> +#include <xen/mem_access.h>
>>  #include <public/memory.h>
>> -#include <asm/p2m.h>
>> +#include <asm/mem_access.h>
>>  #include <xsm/xsm.h>
>
> Normally asm/ includes xen/ of the same name or the other way
> around, depending on how they relate to one another; you
> shouldn't ever need both includes, and I'd be surprised if the
> two headers really are (even conceptionally) completely
> independent of each other.

Sure, xen/mem_access.h can include the asm specific one.

>
> Otherwise this all looks like pure code motion (except for the
> adjustments described), but it would be nice if you could
> clarify that's indeed (intended to be) the case.

I do say in the commit message this is mechanical code motion: "There
are no code-changes introduced, the patch is mechanical code
movement."

>
> Jan
>

Thanks,
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel