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

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