Re: [PATCH] workqueue: Fix memory ordering race in queue_work*()

[Hector Martin]

On 2022/08/16 14:52, Linus Torvalds wrote:
> I think I understand *why* it's broken - it looks like a "harmless"
> optimization. After all, if the bitop doesn't do anything, there's
> nothing to order it with.
> 
> It makes a certain amount of sense - as long as you don't think about
> it too hard.
> 
> The reason it is completely and utterly broken is that it's not
> actually just "the bitop doesn't do anything". Even when it doesn't
> change the bit value, just the ordering of the read of the old bit
> value can be meaningful, exactly for that case of "I added more work
> to the queue, I need to set the bit to tell the consumers, and if I'm
> the first person to set the bit I may need to wake the consumer up".

This is the same reason I argued queue_work() itself needs to have a
similar guarantee, even when it doesn't queue work (and I updated the
doc to match). If test_and_set_bit() is used in this kind of context
often in the kernel, clearly the current implementation/doc clashes with
that.

As I said, I don't have any particular beef in this fight, but this is
horribly broken on M1/2 right now, so I'll send a patch to change the
bitops instead and you all can fight it out over which way is correct :)

-- 
Hector Martin (marcan@xxxxxxxxx)
Public Key: https://mrcn.st/pub