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

From: Julien Grall <julien.grall@arm.com>
To: Vijay Kilari <vijay.kilari@gmail.com>
Cc: Tim Deegan <tim@xen.org>,
	kevin.tian@intel.com, Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Dario Faggioli <dario.faggioli@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Jan Beulich <jbeulich@suse.com>,
	Vijaya Kumar K <Vijaya.Kumar@cavium.com>
Subject: Re: [RFC PATCH v3 02/24] x86: NUMA: Clean up: Fix coding styles and drop unused code
Date: Thu, 20 Jul 2017 13:09:30 +0100	[thread overview]
Message-ID: <4a9e6859-2904-048c-d69e-cabb1b69fe2a@arm.com> (raw)
In-Reply-To: <CALicx6uw=R+k6+J=KD+Lj9E1QwPyTyZg+q74NuvLoDTYZCP_Xg@mail.gmail.com>

On 20/07/17 13:05, Vijay Kilari wrote:
> On Thu, Jul 20, 2017 at 4:30 PM, Julien Grall <julien.grall@arm.com> 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@arm.com>
>>> wrote:
>>>>
>>>> Hi Vijay,
>>>>
>>>> On 18/07/17 12:41, vijay.kilari@gmail.com wrote:
>>>>>
>>>>>
>>>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>>>
>>>>> 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@cavium.com>
>>>>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>>>>> ---
>>>>> 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@lists.xen.org
https://lists.xen.org/xen-devel