Re: [PATCH 2/3] powerpc/numa: Early request for home node associativity

[Srikar Dronamraju <srikar@linux.vnet.ibm.com>]

* Nathan Lynch <nathanl@linux.ibm.com> [2019-08-22 12:17:48]:

> Hi Srikar,

Thanks Nathan for the review.

> 
> > However home node associativity requires cpu's hwid which is set in
> > smp_setup_pacas. Hence call smp_setup_pacas before numa_setup_cpus.
> 
> But this seems like it would negatively affect pacas' NUMA placements?
> 
> Would it be less risky to figure out a way to do "early" VPHN hcalls
> before mem_topology_setup, getting the hwids from the cpu_to_phys_id
> array perhaps?
> 

Do you mean for calls from mem_topology_setup(), stuff we use cpu_to_phys_id
but for the calls from ppc_numa_cpu_prepare() we use the
get_hard_smp_processor_id()?

Thats doable.

> 
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index 88b5157..7965d3b 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -461,6 +461,21 @@ static int of_drconf_to_nid_single(struct drmem_lmb *lmb)
> >  	return nid;
> >  }
> >  
> > +static int vphn_get_nid(unsigned long cpu)
> > +{
> > +	__be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
> > +	long rc;
> > +
> > +	/* Use associativity from first thread for all siblings */
> 
> I don't understand how this comment corresponds to the code it
> accompanies.


Okay will rephrase
> 
> 
> > +	rc = hcall_vphn(get_hard_smp_processor_id(cpu),
> > +				VPHN_FLAG_VCPU, associativity);
> > +
> > +	if (rc == H_SUCCESS)
> > +		return  associativity_to_nid(associativity);
>                       ^^ extra space
> 
> > @@ -490,7 +505,18 @@ static int numa_setup_cpu(unsigned long lcpu)
> >  			goto out;
> >  	}
> >  
> > -	nid = of_node_to_nid_single(cpu);
> > +	/*
> > +	 * On a shared lpar, the device tree might not have the correct node
> > +	 * associativity.  At this time lppaca, or its __old_status field
> 
> Sorry but I'm going to quibble with this phrasing a bit. On SPLPAR the
> CPU nodes have no affinity information in the device tree at all. This
> comment implies that they may have incorrect information, which is
> AFAIK not the case.
> 

Okay will clarify.

-- 
Thanks and Regards
Srikar Dronamraju