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

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 2021年8月28日 18:45
> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> sstabellini@xxxxxxxxxx
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> Subject: Re: [XEN RFC PATCH 38/40] xen/arm: enable device tree based NUMA
> in system init
> 
> 
> 
> 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.
> 

Thanks for this clarification.

> > 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.
> 

Yes, I will try to re-order the patches in this way in next version.

> Cheers,
> 
> --
> Julien Grall