Re: [PATCH 3/3] x86/split_lock: Align the x86_capability array to size of unsigned long

From: Luck, Tony
Date: Tue Sep 17 2019 - 15:14:04 EST


On Tue, Sep 17, 2019 at 08:29:28AM +0000, David Laight wrote:
> From: Tony Luck
> > Sent: 16 September 2019 23:40
> > From: Fenghua Yu <fenghua.yu@xxxxxxxxx>
> >
> > The x86_capability array in cpuinfo_x86 is defined as u32 and thus is
> > naturally aligned to 4 bytes. But, set_bit() and clear_bit() require
> > the array to be aligned to size of unsigned long (i.e. 8 bytes in
> > 64-bit).
> >
> > To fix the alignment issue, align the x86_capability array to size of
> > unsigned long by using unnamed union and 'unsigned long array_align'
> > to force the alignment.
> >
> > Changing the x86_capability array's type to unsigned long may also fix
> > the issue because the x86_capability array will be naturally aligned
> > to size of unsigned long. But this needs additional code changes.
> > So choose the simpler solution by setting the array's alignment to size
> > of unsigned long.
> >
> > Suggested-by: David Laight <David.Laight@xxxxxxxxxx>
>
> While this is probably the only play where this 'capabilities' array
> has been detected as misaligned, ISTR there are several other places
> where the identical array is defined and used.
> These all need fixing as well.

Agree 100% These three patches cover the places *detected* so
far. For bisectability reasons they need to be upstream before
the patches that add WARN_ON, or the one that turns on alignment
traps. As we find other places, we can fix alignments in other
structures too.

If you remember what those other places are, please let us know
so we can push patches to fix those.

If you have a better strategy to find them ... that also would
be very interesting.

-Tony