Nathan Fontenot <nf...@linux.vnet.ibm.com> writes: > On 08/23/2017 06:41 AM, Michael Ellerman wrote: >> Michael Bringmann <m...@linux.vnet.ibm.com> writes: >> >>> powerpc/numa: Correct the currently broken capability to set the >>> topology for shared CPUs in LPARs. At boot time for shared CPU >>> lpars, the topology for each shared CPU is set to node zero, however, >>> this is now updated correctly using the Virtual Processor Home Node >>> (VPHN) capabilities information provided by the pHyp. >>> >>> Also, update initialization checks for device-tree attributes to >>> independently recognize PRRN or VPHN usage. >> >> Did you ever do anything to address Nathan's comments on v4 ? >> >> http://patchwork.ozlabs.org/patch/767587/ > > Looking at this patch I do not see that VPHN is always enabled.
You mean *this* patch? Or v4? I think you mean this patch, in which case I agree. >> Also your change log doesn't describe anything about what the patch does >> and why it is the correct fix for the problem. >> >> When a DLPAR happens you modify the VPHN timer to run in 1 nsec, but you >> don't wait for it. Why would we not just run the logic synchronously? >> >> It also seems to make VPHN and PRRN no longer exclusive, which looking >> at PAPR seems like it might be correct, but is also a major change so >> please justify it in detail. > > This is correct, they are not exclusive. When we first added PRRN support > we mistakenly thought they were exclusive which is why the code currently > only starts PRRN, or VPHN if PRRN is not present. OK. So we need a patch that does that and only that, and clearly explains why we're doing that and why it's the correct thing to do. Then a 2nd patch can fiddle with the timer, if we must. ... >>> +static int topology_timer_secs = TOPOLOGY_DEF_TIMER_SECS; >>> +static int topology_inited; >>> +static int topology_update_needed; >> >> None of this code should be in numa.c. Which is not your fault but I'm >> inclined to move it before we make it worse. > > Agreed. Perhaps this should all be in mm/vphn.c Actually I was thinking platforms/pseries/vphn.c cheers
