Messages in this thread | | | Date | Tue, 16 Aug 2022 15:28:50 +0900 | Subject | Re: [PATCH] workqueue: Fix memory ordering race in queue_work*() | From | 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@marcan.st) Public Key: https://mrcn.st/pub
| |