Re: [Xen-devel] [RFC PATCH v3 02/24] x86: NUMA: Clean up: Fix coding styles and drop unused code

[Julien Grall <julien.grall@xxxxxxx>]

On 20/07/17 13:05, Vijay Kilari wrote:
> On Thu, Jul 20, 2017 at 4:30 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> > Hi Vijay,
> >
> >
> > On 20/07/17 08:00, Vijay Kilari wrote:
> > > On Wed, Jul 19, 2017 at 9:53 PM, Julien Grall <julien.grall@xxxxxxx>
> > > wrote:
> > > > Hi Vijay,
> > > >
> > > > On 18/07/17 12:41, vijay.kilari@xxxxxxxxx wrote:
> > > > > From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
> > > > >
> > > > > Fix coding style, trailing spaces, tabs in NUMA code.
> > > > > Also drop unused macros and functions.
> > > > > There is no functional change.
> > > > >
> > > > > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
> > > > > Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > > ---
> > > > > v3: - Change commit message
> > > > >     - Changed VIRTUAL_BUG_ON to ASSERT
> > > >
> > > >
> > > > Looking at the commit message you don't mention any renaming...
> > > >
> > > > >     - Dropped useless inner paranthesis for some macros
> > > >
> > > >
> > > > [...]
> > > >
> > > > > diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
> > > > > index 3cf26c2..c0de57b 100644
> > > > > --- a/xen/include/asm-x86/numa.h
> > > > > +++ b/xen/include/asm-x86/numa.h
> > > > > @@ -1,8 +1,11 @@
> > > > > -#ifndef _ASM_X8664_NUMA_H
> > > > > +#ifndef _ASM_X8664_NUMA_H
> > > > >  #define _ASM_X8664_NUMA_H 1
> > > > >
> > > > >  #include <xen/cpumask.h>
> > > > >
> > > > > +#define MAX_NUMNODES    NR_NODES
> > > > > +#define NR_NODE_MEMBLKS (MAX_NUMNODES * 2)
> > > >
> > > >
> > > > I don't understand why this suddenly appears in the code when you moved
> > > > away
> > > > in patch #1 in xen/numa.h.
> > >
> > > Particularly MAX_NUMNODES required by this header file with this
> > > patch changes for compilation.
> > > Though I can include xen/numa.h here but xen/numa.h is including
> > > asm/numa.h back.
> > >
> > > I will add separate patch for this defines movement and drop from
> > > this patch.
> >
> > Why adding a separate patch? The code should not have been moved away in
> > patch #1 as you did.
> In patch#1 , I have not moved MAX_NUMNODES. It is kept in xen/numa.h file
> In this patch, when VIRTUAL_BUG_ON is changed to ASSERT, in asm-x86/numa.h,
> it requires MAX_NUMNODES define. So I have moved it from xen/numa.h to
> asm-x86/numa.h
I am sorry but looked at your patch #1. You moved NR_NODE_MEMBLKS in 
patch #1 from asm-x86/numa.h to xen/numa.h. And then you moved it again 
here.
> So, I was thinking of  adding small patch to move both MAX_NUMNODES and
> NR_NODE_MEMBLKS to asm-x86/numa.h
Or better, you can do in xen/numa.h:

#define MAX_NUMNODES ...
#define NR_NODE_...

#include <asm/numa.h>

Cheers,

--
Julien Grall

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