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

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

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

So, I was thinking of  adding small patch to move both MAX_NUMNODES and
NR_NODE_MEMBLKS to asm-x86/numa.h

And in code movement patch, I will move to xen/numa.h along with ASSERT code.

>
> But I still don't understand what is the exact error here... If it fails on
> this patch, likely this should have failed after applying patch #1. And
> *all* patch should be able to build without the rest of the series.

Yes, all patches are tested for compilation individually.

>
> Cheers,
>
> --
> Julien Grall

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