diff mbox

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

Message ID CALicx6tCTkwmm3sp3SbZe+Rmn4GSCq5BfK5T7osP+gca92xYUw@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vijay Kilari July 20, 2017, 12:29 p.m. UTC 
On Thu, Jul 20, 2017 at 5:39 PM, Julien Grall <julien.grall@arm.com> wrote:
>
>
> 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>

But still compilation issue comes from below code.
where only asm/numa.h is included.


>
> Cheers,
>
> --
> Julien Grall

Comments

Julien Grall July 20, 2017, 12:33 p.m. UTC | #1 
On 20/07/17 13:29, Vijay Kilari wrote:
> On Thu, Jul 20, 2017 at 5:39 PM, Julien Grall <julien.grall@arm.com> wrote:
>
> But still compilation issue comes from below code.
> where only asm/numa.h is included.
>
> --- a/xen/include/asm-x86/irq.h
> +++ b/xen/include/asm-x86/irq.h
> @@ -4,7 +4,7 @@
>  /* (C) 1992, 1993 Linus Torvalds, (C) 1997 Ingo Molnar */
>
>  #include <asm/atomic.h>
> -#include <asm/numa.h>
> +#include <xen/numa.h>
>  #include <xen/cpumask.h>
>  #include <xen/smp.h>
>  #include <xen/hvm/irq.h>

Send a patch to fix that then.

Cheers,
diff mbox

Patch

--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -4,7 +4,7 @@ 
 /* (C) 1992, 1993 Linus Torvalds, (C) 1997 Ingo Molnar */

 #include <asm/atomic.h>
-#include <asm/numa.h>
+#include <xen/numa.h>
 #include <xen/cpumask.h>
 #include <xen/smp.h>
 #include <xen/hvm/irq.h>