Re: [PATCH 11/13] x86_64: Remove pointless set_64bit() usage

Peter Zijlstra
2022-11-04T16:01:58Z

On Thu, Nov 03, 2022 at 01:39:17PM -0700, Linus Torvalds wrote:

> And even after I checked *that*, I then checked the 'struct irte' to
> check that it's actually properly defined, and it isn't. Considering
> that this all requires 16-byte alignment to work, I think that type
> should also be marked as being 16-byte aligned.
>
> In fact, I wonder if we should aim to actually force compile-time
> checking, because right now we have
>
> VM_BUG_ON((unsigned long)(p1) % (2 * sizeof(long)));
> VM_BUG_ON((unsigned long)((p1) + 1) != (unsigned long)(p2));
>
> in our x86-64 __cmpxchg_double() macro, and honestly, that first one
> might be better as a compile time check of __alignof__, and the second
> one shouldn't exisrt at all because our interface shouldn't be using
> two different pointers when the only possible use is for one single
> aligned value.

So cmpxchg_double() does a cmpxchg on a double long value and is
currently supported by: i386, x86_64, arm64 and s390.

On all those, except i386, two longs are u128.

So how about we introduce u128 and cmpxchg128 -- then it directly
mirrors the u64 and cmpxchg64 usage we already have. It then also
naturally imposses the alignment thing.

Afaict the only cmpxchg_double() users are:

arch/s390/kernel/perf_cpum_sf.c
drivers/iommu/amd/iommu.c
drivers/iommu/intel/irq_remapping.c
mm/slub.c

Of those slub.c is the only one that cares about 32bit and would need
some 'compat' code to pick between cmpxchg64 / cmpxchg128, but it
already has everything wrapped in helpers so that shouldn't be too big
of a worry.

Then we can convert these few users over and simply delete the whole
cmpxchg_double() thing.