Re: [PATCH v5 3/4] spapr_numa.c: base FORM2 NUMA affinity support

[David Gibson]

On Tue, Sep 07, 2021 at 07:07:41AM -0300, Daniel Henrique Barboza wrote:
65;6402;1c> 
> 
> On 9/6/21 10:02 PM, David Gibson wrote:
> > On Mon, Sep 06, 2021 at 09:25:26PM -0300, Daniel Henrique Barboza wrote:
> > > The main feature of FORM2 affinity support is the separation of NUMA
> > > distances from ibm,associativity information. This allows for a more
> > > flexible and straightforward NUMA distance assignment without relying on
> > > complex associations between several levels of NUMA via
> > > ibm,associativity matches. Another feature is its extensibility. This base
> > > support contains the facilities for NUMA distance assignment, but in the
> > > future more facilities will be added for latency, performance, bandwidth
> > > and so on.
> > > 
> > > This patch implements the base FORM2 affinity support as follows:
> > > 
> > > - the use of FORM2 associativity is indicated by using bit 2 of byte 5
> > > of ibm,architecture-vec-5. A FORM2 aware guest can choose to use FORM1
> > > or FORM2 affinity. Setting both forms will default to FORM2. We're not
> > > advertising FORM2 for pseries-6.1 and older machine versions to prevent
> > > guest visible changes in those;
> > > 
> > > - call spapr_numa_associativity_reset() in 
> > > do_client_architecture_support()
> > > if FORM2 is chosen. This will avoid re-initializing FORM1 artifacts that
> > > were already initialized in spapr_machine_reset();
> > > 
> > > - ibm,associativity-reference-points has a new semantic. Instead of
> > > being used to calculate distances via NUMA levels, it's now used to
> > > indicate the primary domain index in the ibm,associativity domain of
> > > each resource. In our case it's set to {0x4}, matching the position
> > > where we already place logical_domain_id;
> > 
> > Hmm... I'm a bit torn on this.  The whole reason the ibm,associativity
> > things are arrays rather than just numbers was to enable the FORM1
> > nonsense. So we have a choice here: keep the associativity arrays in
> > the same form, for simplicity of the code, or reduce the associativity
> > arrays to one entry for FORM2, to simplify the overall DT contents in
> > the "modern" case.
> 
> I'm not against making it different from FORM2. I did it this way because
> it minimizes the amount of code being changed.
> 
> In fact, if we're going to add separated data structures for both FORM1 and
> FORM2, might as well start both FORM1 and FORM2 data structures during
> machine_init() and then just switch to the chosen affinity after CAS.
> 
> Something like a FORM1_assoc_array[N][MAX_DISTANCE_REF_POINTS], that contains
> all the initialization already done today and a FORM2_assoc_array[N][2],
> where FORM2_assoc_array[node_id] = {1, node_id}, changing reference-points
> accordingly of course.
> 
> spapr_numa_assoc_array would become a pointer that would point to either
> FORM1_assoc_array[][] or FORM2_assoc_array[][] depending on guest choice. I
> think this might be enough to make everything we already have just works, 
> although
> I need to check how much code is dependant on the MAX_DISTANCE_REF_POINTS
> macro and adapt it.
> 
> If no one opposes I'll go for this approach.

I think that's the way to go.  Thanks for working on this.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature