Re: [Xen-devel] [PATCH] x86: xen: size struct xen_spinlock to alwaysfit in arch_spinlock_t

[David Vrabel]

On 23/01/2012 19:50, Konrad Rzeszutek Wilk wrote:
> On Mon, Jan 23, 2012 at 07:32:25PM +0000, David Vrabel wrote:
> > From: David Vrabel<david.vrabel@xxxxxxxxxx>
> >
> > If NR_CPUS<  256 then arch_spinlock_t is only 16 bits wide but struct
> > xen_spinlock is 32 bits.  When a spin lock is contended and
> > xl->spinners is modified the two bytes immediately after the spin lock
> > would be corrupted.
> >
> > This is a regression caused by 84eb950db13ca40a0572ce9957e14723500943d6
> > (x86, ticketlock: Clean up types and accessors) which reduced the size
> > of arch_spinlock_t.
> >
> > Fix this by making xl->spinners a u8 if NR_CPUS<  256.  A
> > BUILD_BUG_ON() is also added to check the sizes of the two structures
> > are compatible.
> >
> > In many cases this was not noticable as there would often be padding
> > bytes after the lock (e.g., if any of CONFIG_GENERIC_LOCKBREAK,
> > CONFIG_DEBUG_SPINLOCK, or CONFIG_DEBUG_LOCK_ALLOC were enabled).
> >
> > The bnx2 driver is affected. In struct bnx2, phy_lock and
> > indirect_lock may have no padding after them.  Contention on phy_lock
> > would corrupt indirect_lock making it appear locked and the driver
> > would deadlock.
>
> Nice find. I think it also affected the ahci driver, and some of the USB
> ones - at least those I saw starting to hang with:

It's possible but keep in mind that this isn't a recent regression.  I 
think it's been around since 3.0 and maybe even earlier.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/