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

[Peter Zijlstra]

On Fri, Nov 22, 2019 at 01:25:45PM -0800, Andy Lutomirski wrote:
> On Fri, Nov 22, 2019 at 12:29 PM Fenghua Yu <fenghua.yu@xxxxxxxxx> wrote:
> >
> > On Fri, Nov 22, 2019 at 09:23:45PM +0100, Peter Zijlstra wrote:
> > > On Fri, Nov 22, 2019 at 06:02:04PM +0000, Luck, Tony wrote:
> > > > > it requires we get the kernel and firmware clean, but only warns about
> > > > > dodgy userspace, which I really don't think there is much of.
> > > > >
> > > > > getting the kernel clean should be pretty simple.
> > > >
> > > > Fenghua has a half dozen additional patches (I think they were
> > > > all posted in previous iterations of the patch) that were found by
> > > > code inspection, rather than by actually hitting them.
> > >
> > > I thought we merged at least some of that, but maybe my recollection is
> > > faulty.
> >
> > At least 2 key fixes are in TIP tree:
> > https://lore.kernel.org/lkml/157384597983.12247.8995835529288193538.tip-bot2@tip-bot2/
> > https://lore.kernel.org/lkml/157384597947.12247.7200239597382357556.tip-bot2@tip-bot2/
> 
> I do not like these patches at all.  I would *much* rather see the
> bitops fixed and those patches reverted.
> 
> Is there any Linux architecture that doesn't have 32-bit atomic
> operations?

Of course! The right question is if there's any architecture that has
SMP and doesn't have 32bit atomic instructions, and then I'd have to
tell you that yes we have those too :/

Personally I'd love to mandate any SMP system has proper atomic ops, but
for now we sorta have to make PARISC and SPARC32 (and some ARC variant
IIRC) limp along.

PARISC and SPARC32 only have the equivalent of an xchgb or something.
Using that you can build a test-and-set spinlock, and then you have to
build atomic primitives using a hashtable of spinlocks.

Awesome, right?

> If all architectures can support them, then we should add
> set_bit_u32(), etc and/or make x86's set_bit() work for a
> 4-byte-aligned pointer.

I object to _u32() variants of the atomic bitops; the bitops interface
is a big enough trainwreck already, lets not make it worse. Making the
existing bitops use 32bit atomics on the inside should be fine though.

If anything we could switch the entire bitmap interface to unsigned int,
but I'm not sure that'd actually help much.

Anyway, many of the unaligned usages appear not to require atomicicity
in the first place, see the other patches he sent [*]. And like pointed out
elsewhere, any code that casts random pointers to (unsigned long *) is
probably already broken due to endian issues. Just making the unaligned
check go away isn't fixing it.

[*] https://lkml.kernel.org/r/1574710984-208305-1-git-send-email-fenghua.yu@xxxxxxxxx