Re: [Xen-devel] [PATCH v4 01/16] xen: Relocate mem_access and mem_event into common.

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

On Fri, Sep 5, 2014 at 12:07 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 05.09.14 at 11:45, <tamas.lengyel@xxxxxxxxxxxx> wrote:
> On Fri, Sep 5, 2014 at 11:14 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
>> >>> On 05.09.14 at 10:58, <tklengyel@xxxxxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/x86_64/mm.c
>> > +++ b/xen/arch/x86/x86_64/mm.c
>> > @@ -35,9 +35,9 @@
>> >  #include <asm/msr.h>
>> >  #include <asm/setup.h>
>> >  #include <asm/numa.h>
>> > -#include <asm/mem_event.h>
>> > +#include <xen/mem_event.h>
>> >  #include <asm/mem_sharing.h>
>> > -#include <asm/mem_access.h>
>> > +#include <xen/mem_access.h>
>> >  #include <public/memory.h>
>>
>> This is not the only place, but a specifically bad example: I'm pretty
>> sure I asked you before to not make a mess by mixing asm/ and
>> xen/ included - move the now xen/ ones to the other ones already
>> coming from that directory. And do so consistently throughout the
>> patch.
>>
>>
> Sure. Is this a style-thing or does it have some other effect on the code?

A style thing that is not supposed to have an effect (we should
strive for headers to include their dependencies rather than relying
on their users doing so up front).

>> Looking at this again I wonder though
>> whether we really need these - the use sites could easily check
>> whether p2m_is_{paging,shared} is defined instead.
>
>  I would still need to wrap them in CONFIG_X86 blocks as common/memory.c
> does, so ultimately I'm not sure there is much difference.

I don't understand why - ARM doesn't define p2m_is_paging()
or p2m_is_shared().

Jan

Hm, I guess I could do that, just define them to return 0 and hope the compiler just drops the always-dead if blocks.

Tamas

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