[v2] powerpc/vphn: fix endian issue in NUMA device node code

Michael Ellerman mpe at ellerman.id.au
Wed Oct 15 13:20:55 EST 2014


On Fri, 2014-10-10 at 08:20:33 UTC, Greg Kurz wrote:
> On Tue,  7 Oct 2014 20:28:23 +1100 (EST)
> Michael Ellerman <mpe at ellerman.id.au> wrote:
> 
> > On Fri, 2014-03-10 at 09:13:17 UTC, Greg Kurz wrote:
> > > The associativity domain numbers are obtained from the hypervisor through
> > > registers and written into memory by the guest: the packed array passed to
> > > vphn_unpack_associativity() is then native-endian, unlike what was assumed
> > > in the following commit:
> > > 
> > > This patch does two things:
> > > - extract values from the packed array with shifts, in order to be endian
> > >   neutral
> > > - convert the resulting values to be32 as expected
> > 
> > So to unpack them, can't we just iterate over those six 64-bit values, which if
> > we load them as 64-bit values will be back in cpu endian?
> 
> Hi Michael,
> 
> Do you mean something like the following ?

Not exactly, but that's certainly a lot simpler than the original patch.
 
> Sure it also works and is a lot simplier... but it adds an extra loop. Also,
> if the 3 first elements of the array contain 12 VPHN_FIELD_MSB fields, then
> we don't even need to swap the remaining elements: only the parsing code
> knows.
>
> On the other hand, I understand this is not a hot path... so what should we
> do ?

So I prefer this to your original patch. I know the original was cool and
tricky but it was also complicated :)

What I was thinking of was that the actual parsing code, in vphn_unpack_associativity()
would do the 64-bit loads. That will require changing the parsing loop quite a
bit, but I think it might be worth it.

So can you resend this version with a proper changelog, and I'll merge that
straight away as a bug fix. Then if you can look at reworking the parsing loop
to avoid all the endian swapping on the input side that'd be great.

cheers


More information about the Linuxppc-dev mailing list