Re: [XEN RFC PATCH 38/40] xen/arm: enable device tree based NUMA in system init

[Julien Grall <julien@xxxxxxx>]

On 28/08/2021 04:17, Wei Chen wrote:
> Hi Julien,
Hi Wei,

> > -----Original Message-----
> > From: Julien Grall <julien@xxxxxxx>
> > Sent: 2021年8月27日 22:33
> > To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> > sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> > Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> > Subject: Re: [XEN RFC PATCH 38/40] xen/arm: enable device tree based NUMA
> > in system init
> >
> > Hi Wei,
> >
> > On 11/08/2021 11:24, Wei Chen wrote:
> > > Everything is ready, we can remove the fake NUMA node and
> > > depends on device tree to create NUMA system.
> > So you just added code a few patches before that are now completely
> > rewritten. Can you please re-order this series so it doesn't happen?
> >
> > This may mean that CONFIG_NUMA is only selected until late in this series.
> Why I did like this is because my original concerns are:
> 1. When I introduced the CONFIG_NUMA. Users will be using a code base on
>     this commit by accident.
> 2. If users select CONFIG_NUMA, but not all NUMA data are not initialized
>     properly. The system may not work properly.
We have to make sure we don't break any existing use case when writing a 
new feature. However, a user should not expect a new feature to work it 
is using a random commit in the middle of the series.
This is also why I suggested that maybe CONFIG_NUMA is only selected for 
Arm towards the end of the series. So you reduce the risk of someone 
selecting it.
> 3. So I created the fake node to initialize the NUMA data, before we parser
>     real data from DTB.
> 4. In this case, user can work well with CONFIG_NUMA is enabled, without
>     this series is completed.
The flip side is you are adding more load on the reviewers because there 
are extra code. The series is already quite big (40 patches), any way to 
ease the review will definitely be appreciated.
Another possible way to ease the review is to move the patch that 
rework/move code at the beginning of the series and leave the Arm part 
for the second part of the series. This way, we can start to merge the 
series without waiting for the Arm bits to be completed.
Cheers,

--
Julien Grall