RE: [PATCH v10 6/6] x86/split_lock: Enable split lock detection by kernel parameter

From: David Laight <David.Laight@ACULAB.COM>
To: 'Andy Lutomirski' <luto@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Ingo Molnar" <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	H Peter Anvin <hpa@zytor.com>, Tony Luck <tony.luck@intel.com>,
	Ashok Raj <ashok.raj@intel.com>,
	Ravi V Shankar <ravi.v.shankar@intel.com>,
	linux-kernel <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>
Subject: RE: [PATCH v10 6/6] x86/split_lock: Enable split lock detection by kernel parameter
Date: Fri, 22 Nov 2019 09:46:16 +0000	[thread overview]
Message-ID: <397239502dad4bc3819c49c7d569c70c@AcuMS.aculab.com> (raw)
In-Reply-To: <CALCETrW+qxrE633qetS4c1Rn2AX_hk5OgneZRtoZPFN1J395Ng@mail.gmail.com>

From Andy Lutomirski
> Sent: 21 November 2019 17:51
> On Thu, Nov 21, 2019 at 9:43 AM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Ingo Molnar
> > > Sent: 21 November 2019 17:12
> > > * Peter Zijlstra <peterz@infradead.org> wrote:
> > ...
> > > > This feature MUST be default enabled, otherwise everything will
> > > > be/remain broken and we'll end up in the situation where you can't use
> > > > it even if you wanted to.
> > >
> > > Agreed.
> >
> > Before it can be enabled by default someone needs to go through the
> > kernel and fix all the code that abuses the 'bit' functions by using them
> > on int[] instead of long[].
> >
> > I've only seen one fix go through for one use case of one piece of code
> > that repeatedly uses potentially misaligned int[] arrays for bitmasks.
> >
> 
> Can we really not just change the lock asm to use 32-bit accesses for
> set_bit(), etc?  Sure, it will fail if the bit index is greater than
> 2^32, but that seems nuts.

For little endian 64bit cpu it is safe(ish) to cast int [] to long [] for the bitops.
On BE 64bit cpu all hell breaks loose if you do that.
It really wasn't obvious that all the casts I found were anywhere near right
on 64bit BE systems.

So while it is almost certainly safe to change the x86-64 bitops to use
32 bit accesses, some of the code is horribly broken.

> (Why the *hell* do the bitops use long anyway?  They're *bit masks*
> for crying out loud.  As in, users generally want to operate on fixed
> numbers of bits.)

The bitops functions were (probably) written for large bitmaps that
are bigger than the size of a 'word'  (> 32 bits) and likely to be
variable size.
Quite why they use long [] is anybody's guess, but that is the definition.
It also isn't quite clear to me why that are required to be atomic.
On x86 atomicity doesn't cost much, on other architectures the cost
is significant.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)