Re: [PATCH] x86-32: fix cmpxchg8b_emu build error with clang

From: Linus Torvalds
Date: Wed Jun 26 2024 - 11:25:52 EST


On Wed, 26 Jun 2024 at 00:39, Uros Bizjak <ubizjak@xxxxxxxxx> wrote:
>
> >
> > Still doing the 'oldp' writeback unconmditionally. The code generation
> > for the case I checked were the same for both clang and gcc, but until
> > Uros hits me with the big clue-hammer, I think it's the simpler code
> > that leaves room for potentially better optimizations too.
>
> You probably want to look at 44fe84459faf1 ("locking/atomic: Fix
> atomic_try_cmpxchg() semantics") [1] and the long LKML discussion at
> [2].

Christ. That use should be invalid.

The only _atomic_ pointer is "_ptr", not "old". Anybody who gives
something that can change during the operation in "old" is basically
already doing random things.

> --quote--
> This code is broken with the current implementation, the problem is
> with unconditional update of *__po.

I think the only thing broken is that quote, and the crazy expectation
that "old" can change.

But obviously, I had completely forgotten that whole discussion from
seven years ago.

I don't actually find a single use of that invalid code sequence where
somebody would pass a non-private pointer as "oldp". So I really think
that part of the whole discussion was bogus to begin with, and
presumably from some other code base.

IOW, I think that example of a "classical lock-free stack push" is just broken.

That said, I can't find a case where it would matter for code
generation (every use will always do a conditional branch based on the
result, so the conditional assignment is practically speaking always
"static" anyway by the time you do branch following.

So I'll just send out a minimal patch with *only* the %esi changes.

Linus