linux-kernel - Re: [PATCH 11/13] x86_64: Remove pointless set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 4 Nov 2022 10:15:08 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Nathan Chancellor <nathan@...nel.org>,
        Uros Bizjak <ubizjak@...il.com>, x86@...nel.org,
        willy@...radead.org, akpm@...ux-foundation.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        aarcange@...hat.com, kirill.shutemov@...ux.intel.com,
        jroedel@...e.de
Subject: Re: [PATCH 11/13] x86_64: Remove pointless set_64bit() usage

On Fri, Nov 4, 2022 at 9:01 AM Peter Zijlstra <peterz@...radead.org> wrote:
>
> 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.

Ack, except that we might have some "u128" users that do *not*
necessarily want any alignment thing.

But maybe we could at least start with an u128 type that is marked as
being fully aligned, and if some other user comes in down the line
that wants relaxed alignment we can call it "u128_unaligned" or
something.

That would have avoided the pain we have on x86-32 where "u64" in a
UAPI structure is differently aligned than it is on actual 64-bit
architectures.

> 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.

Ack. Special case, and we can call it something very clear, and in
fact it would clean up the slab code too if we actually used some
explicit type for this.

Right now, slab does

        /* Double-word boundary */
        void *freelist;         /* first free object */
        union {
                unsigned long counters;
                struct {

where the alignment constraints are just a comment, and then it passes
pointers to the two words around manually.

But it should be fairly straightforward to make it use an actual
properly typed thing, and using an unnamed union member, do something
that takes an explicitly aligned "two word" thing instead.

IOW, make the 'compat' case *not* use those stupid two separate
pointers (that have to be consecutive and aligned), but actually do
something like

        struct cmpxchg_double_long {
                unsigned long a,b;
        } __aligned(2*sizeof(long));

and then the slab case can use that union of that and the existing
"freelist+word-union" to make this all not just a comment to people,
but something the compiler sees too.

            Linus