RE: [PATCH v8 0/6] Device tree based NUMA support for Arm - Part#2

Wei Chen posted 6 patches 1 year, 5 months ago
Only 0 patches received!
There is a newer version of this series
RE: [PATCH v8 0/6] Device tree based NUMA support for Arm - Part#2
Posted by Wei Chen 1 year, 5 months ago
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2022年11月14日 16:23
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau
> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
> <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano
> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v8 0/6] Device tree based NUMA support for Arm -
> Part#2
> 
> On 14.11.2022 09:14, Wei Chen wrote:
> > Hi Jan,
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: 2022年11月14日 16:05
> >> To: Wei Chen <Wei.Chen@arm.com>
> >> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger
> Pau
> >> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
> >> <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano
> >> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org
> >> Subject: Re: [PATCH v8 0/6] Device tree based NUMA support for Arm -
> >> Part#2
> >>> So in this patch series, we implement a set of NUMA API to use
> >>> device tree to describe the NUMA layout. We reuse most of the
> >>> code of x86 NUMA to create and maintain the mapping between
> >>> memory and CPU, create the matrix between any two NUMA nodes.
> >>> Except ACPI and some x86 specified code, we have moved other
> >>> code to common. In next stage, when we implement ACPI based
> >>> NUMA for Arm64, we may move the ACPI NUMA code to common too,
> >>> but in current stage, we keep it as x86 only.
> >>>
> >>> This patch serires has been tested and booted well on one
> >>> Arm64 NUMA machine and one HPE x86 NUMA machine.
> >>>
> >>> [1] https://lists.xenproject.org/archives/html/xen-devel/2022-
> >> 06/msg00499.html
> >>> [2] https://lists.xenproject.org/archives/html/xen-devel/2021-
> >> 09/msg01903.html
> >>>
> >>> ---
> >>> v7 -> v8:
> >>>  1. Rebase code to resolve merge conflict.
> >>
> >> You mention this here but not in any of the patches. Which leaves
> >> reviewers guessing where the re-base actually was: Re-bases, at
> >> least sometimes, also need (re-)reviewing.
> >>
> >
> > I just applied the v7 to the latest staging branch, this work has not
> > Generated any new change for this series. I should have described it
> > clear or not mentioned this in cover letter. Sorry for confusing you!
> 
> But you talk about a merge conflict. And that's what I refer to when
> saying "may need (re-)reviewing". The same happened during earlier
> versions of the series, except there I was aware of what you needed
> to re-base over because it was changes I had done (addressing
> observations made while reviewing your changes). This time round I'm
> simply not aware of what change(s) you needed to re-base over (which
> is why I pointed out that it is generally helpful to indicate on a
> per-patch basis when non-trivial re-basing was involved).
> 

I had thought it was a code conflict before, because our internal gerrit
system marked that this series has a merge conflict. But the actual
situation is our gerrit setting policy problem. There are no code conflicts
in these patches themselves. We also did not modify the patch to resolve
the gerrit conflicts. Regardless of whether it is a new or old version,
if I modify the patch, I will remove the reviewed-by.

Sorry for writing this ambiguous description in the change log.

Cheers,
Wei Chen

> Jan
Re: [PATCH v8 0/6] Device tree based NUMA support for Arm - Part#2
Posted by Jan Beulich 1 year, 5 months ago
On 14.11.2022 09:33, Wei Chen wrote:
> Hi Jan,
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 2022年11月14日 16:23
>> To: Wei Chen <Wei.Chen@arm.com>
>> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau
>> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
>> <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano
>> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org
>> Subject: Re: [PATCH v8 0/6] Device tree based NUMA support for Arm -
>> Part#2
>>
>> On 14.11.2022 09:14, Wei Chen wrote:
>>> Hi Jan,
>>>
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: 2022年11月14日 16:05
>>>> To: Wei Chen <Wei.Chen@arm.com>
>>>> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger
>> Pau
>>>> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
>>>> <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano
>>>> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org
>>>> Subject: Re: [PATCH v8 0/6] Device tree based NUMA support for Arm -
>>>> Part#2
>>>>> So in this patch series, we implement a set of NUMA API to use
>>>>> device tree to describe the NUMA layout. We reuse most of the
>>>>> code of x86 NUMA to create and maintain the mapping between
>>>>> memory and CPU, create the matrix between any two NUMA nodes.
>>>>> Except ACPI and some x86 specified code, we have moved other
>>>>> code to common. In next stage, when we implement ACPI based
>>>>> NUMA for Arm64, we may move the ACPI NUMA code to common too,
>>>>> but in current stage, we keep it as x86 only.
>>>>>
>>>>> This patch serires has been tested and booted well on one
>>>>> Arm64 NUMA machine and one HPE x86 NUMA machine.
>>>>>
>>>>> [1] https://lists.xenproject.org/archives/html/xen-devel/2022-
>>>> 06/msg00499.html
>>>>> [2] https://lists.xenproject.org/archives/html/xen-devel/2021-
>>>> 09/msg01903.html
>>>>>
>>>>> ---
>>>>> v7 -> v8:
>>>>>  1. Rebase code to resolve merge conflict.
>>>>
>>>> You mention this here but not in any of the patches. Which leaves
>>>> reviewers guessing where the re-base actually was: Re-bases, at
>>>> least sometimes, also need (re-)reviewing.
>>>>
>>>
>>> I just applied the v7 to the latest staging branch, this work has not
>>> Generated any new change for this series. I should have described it
>>> clear or not mentioned this in cover letter. Sorry for confusing you!
>>
>> But you talk about a merge conflict. And that's what I refer to when
>> saying "may need (re-)reviewing". The same happened during earlier
>> versions of the series, except there I was aware of what you needed
>> to re-base over because it was changes I had done (addressing
>> observations made while reviewing your changes). This time round I'm
>> simply not aware of what change(s) you needed to re-base over (which
>> is why I pointed out that it is generally helpful to indicate on a
>> per-patch basis when non-trivial re-basing was involved).
>>
> 
> I had thought it was a code conflict before, because our internal gerrit
> system marked that this series has a merge conflict. But the actual
> situation is our gerrit setting policy problem. There are no code conflicts
> in these patches themselves. We also did not modify the patch to resolve
> the gerrit conflicts. Regardless of whether it is a new or old version,
> if I modify the patch, I will remove the reviewed-by.

I'd prefer if you didn't unilaterally. Instead I'd like to suggest that
you apply common sense as to whether mere re-basing might actually
invalidate previously supplied tags.

Jan

RE: [PATCH v8 0/6] Device tree based NUMA support for Arm - Part#2
Posted by Wei Chen 1 year, 5 months ago
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2022年11月14日 17:29
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau
> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
> <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano
> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v8 0/6] Device tree based NUMA support for Arm -
> Part#2
> 
> On 14.11.2022 09:33, Wei Chen wrote:
> > Hi Jan,
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: 2022年11月14日 16:23
> >> To: Wei Chen <Wei.Chen@arm.com>
> >> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger
> Pau
> >> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
> >> <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano
> >> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org
> >> Subject: Re: [PATCH v8 0/6] Device tree based NUMA support for Arm -
> >> Part#2
> >>
> >> On 14.11.2022 09:14, Wei Chen wrote:
> >>> Hi Jan,
> >>>
> >>>> -----Original Message-----
> >>>> From: Jan Beulich <jbeulich@suse.com>
> >>>> Sent: 2022年11月14日 16:05
> >>>> To: Wei Chen <Wei.Chen@arm.com>
> >>>> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger
> >> Pau
> >>>> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
> >>>> <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano
> >>>> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org
> >>>> Subject: Re: [PATCH v8 0/6] Device tree based NUMA support for Arm -
> >>>> Part#2
> >>>>> So in this patch series, we implement a set of NUMA API to use
> >>>>> device tree to describe the NUMA layout. We reuse most of the
> >>>>> code of x86 NUMA to create and maintain the mapping between
> >>>>> memory and CPU, create the matrix between any two NUMA nodes.
> >>>>> Except ACPI and some x86 specified code, we have moved other
> >>>>> code to common. In next stage, when we implement ACPI based
> >>>>> NUMA for Arm64, we may move the ACPI NUMA code to common too,
> >>>>> but in current stage, we keep it as x86 only.
> >>>>>
> >>>>> This patch serires has been tested and booted well on one
> >>>>> Arm64 NUMA machine and one HPE x86 NUMA machine.
> >>>>>
> >>>>> [1] https://lists.xenproject.org/archives/html/xen-devel/2022-
> >>>> 06/msg00499.html
> >>>>> [2] https://lists.xenproject.org/archives/html/xen-devel/2021-
> >>>> 09/msg01903.html
> >>>>>
> >>>>> ---
> >>>>> v7 -> v8:
> >>>>>  1. Rebase code to resolve merge conflict.
> >>>>
> >>>> You mention this here but not in any of the patches. Which leaves
> >>>> reviewers guessing where the re-base actually was: Re-bases, at
> >>>> least sometimes, also need (re-)reviewing.
> >>>>
> >>>
> >>> I just applied the v7 to the latest staging branch, this work has not
> >>> Generated any new change for this series. I should have described it
> >>> clear or not mentioned this in cover letter. Sorry for confusing you!
> >>
> >> But you talk about a merge conflict. And that's what I refer to when
> >> saying "may need (re-)reviewing". The same happened during earlier
> >> versions of the series, except there I was aware of what you needed
> >> to re-base over because it was changes I had done (addressing
> >> observations made while reviewing your changes). This time round I'm
> >> simply not aware of what change(s) you needed to re-base over (which
> >> is why I pointed out that it is generally helpful to indicate on a
> >> per-patch basis when non-trivial re-basing was involved).
> >>
> >
> > I had thought it was a code conflict before, because our internal gerrit
> > system marked that this series has a merge conflict. But the actual
> > situation is our gerrit setting policy problem. There are no code
> conflicts
> > in these patches themselves. We also did not modify the patch to resolve
> > the gerrit conflicts. Regardless of whether it is a new or old version,
> > if I modify the patch, I will remove the reviewed-by.
> 
> I'd prefer if you didn't unilaterally. Instead I'd like to suggest that
> you apply common sense as to whether mere re-basing might actually
> invalidate previously supplied tags.
> 

I will keep this in mind in the future. Since for v8 there is actually no
change (except patch 5 to fix the comment) compared to in the rebase
compared to v7, should I invalidate your tags this time?

Thanks,
Wei Chen

> Jan
Re: [PATCH v8 0/6] Device tree based NUMA support for Arm - Part#2
Posted by Jan Beulich 1 year, 5 months ago
On 14.11.2022 10:37, Wei Chen wrote:
> Hi Jan,
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 2022年11月14日 17:29
>> To: Wei Chen <Wei.Chen@arm.com>
>> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau
>> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
>> <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano
>> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org
>> Subject: Re: [PATCH v8 0/6] Device tree based NUMA support for Arm -
>> Part#2
>>
>> On 14.11.2022 09:33, Wei Chen wrote:
>>> Hi Jan,
>>>
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: 2022年11月14日 16:23
>>>> To: Wei Chen <Wei.Chen@arm.com>
>>>> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger
>> Pau
>>>> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
>>>> <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano
>>>> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org
>>>> Subject: Re: [PATCH v8 0/6] Device tree based NUMA support for Arm -
>>>> Part#2
>>>>
>>>> On 14.11.2022 09:14, Wei Chen wrote:
>>>>> Hi Jan,
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Jan Beulich <jbeulich@suse.com>
>>>>>> Sent: 2022年11月14日 16:05
>>>>>> To: Wei Chen <Wei.Chen@arm.com>
>>>>>> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger
>>>> Pau
>>>>>> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
>>>>>> <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano
>>>>>> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org
>>>>>> Subject: Re: [PATCH v8 0/6] Device tree based NUMA support for Arm -
>>>>>> Part#2
>>>>>>> So in this patch series, we implement a set of NUMA API to use
>>>>>>> device tree to describe the NUMA layout. We reuse most of the
>>>>>>> code of x86 NUMA to create and maintain the mapping between
>>>>>>> memory and CPU, create the matrix between any two NUMA nodes.
>>>>>>> Except ACPI and some x86 specified code, we have moved other
>>>>>>> code to common. In next stage, when we implement ACPI based
>>>>>>> NUMA for Arm64, we may move the ACPI NUMA code to common too,
>>>>>>> but in current stage, we keep it as x86 only.
>>>>>>>
>>>>>>> This patch serires has been tested and booted well on one
>>>>>>> Arm64 NUMA machine and one HPE x86 NUMA machine.
>>>>>>>
>>>>>>> [1] https://lists.xenproject.org/archives/html/xen-devel/2022-
>>>>>> 06/msg00499.html
>>>>>>> [2] https://lists.xenproject.org/archives/html/xen-devel/2021-
>>>>>> 09/msg01903.html
>>>>>>>
>>>>>>> ---
>>>>>>> v7 -> v8:
>>>>>>>  1. Rebase code to resolve merge conflict.
>>>>>>
>>>>>> You mention this here but not in any of the patches. Which leaves
>>>>>> reviewers guessing where the re-base actually was: Re-bases, at
>>>>>> least sometimes, also need (re-)reviewing.
>>>>>>
>>>>>
>>>>> I just applied the v7 to the latest staging branch, this work has not
>>>>> Generated any new change for this series. I should have described it
>>>>> clear or not mentioned this in cover letter. Sorry for confusing you!
>>>>
>>>> But you talk about a merge conflict. And that's what I refer to when
>>>> saying "may need (re-)reviewing". The same happened during earlier
>>>> versions of the series, except there I was aware of what you needed
>>>> to re-base over because it was changes I had done (addressing
>>>> observations made while reviewing your changes). This time round I'm
>>>> simply not aware of what change(s) you needed to re-base over (which
>>>> is why I pointed out that it is generally helpful to indicate on a
>>>> per-patch basis when non-trivial re-basing was involved).
>>>>
>>>
>>> I had thought it was a code conflict before, because our internal gerrit
>>> system marked that this series has a merge conflict. But the actual
>>> situation is our gerrit setting policy problem. There are no code
>> conflicts
>>> in these patches themselves. We also did not modify the patch to resolve
>>> the gerrit conflicts. Regardless of whether it is a new or old version,
>>> if I modify the patch, I will remove the reviewed-by.
>>
>> I'd prefer if you didn't unilaterally. Instead I'd like to suggest that
>> you apply common sense as to whether mere re-basing might actually
>> invalidate previously supplied tags.
>>
> 
> I will keep this in mind in the future. Since for v8 there is actually no
> change (except patch 5 to fix the comment) compared to in the rebase
> compared to v7, should I invalidate your tags this time?

No (with me now understanding that the statement in the 0/6 changelog
simply was wrong).

Jan