I mostly agree, see my reply to this patch, but... On 12/26, Linus Torvalds wrote: > > If the optimization is correct, there is no point to having a config option. > > If the optimization is incorrect, there is no point to having the code. > > Either way, there's no way we'd ever have a config option for this. Agreed, > > + if (was_full && pipe_check_wq_has_sleeper(&pipe->wr_wait)) > > wake_up_interruptible_sync_poll(&pipe->wr_wait, EPOLLOUT | EPOLLWRNORM); > > End result: you need to explain why the race cannot exist. Agreed, > And I think your patch is simply buggy Agreed again, but probably my reasoning was wrong, > But now waiters use "wait_event_interruptible_exclusive()" explicitly > outside the pipe mutex, so the waiters and wakers aren't actually > serialized. This is what I don't understand... Could you spell ? I _think_ that wait_event_whatever(WQ, CONDITION); vs CONDITION = 1; if (wq_has_sleeper(WQ)) wake_up_xxx(WQ, ...); is fine. Both wq_has_sleeper() and wait_event_whatever()->prepare_to_wait_event() have the necessary barriers to serialize the waiters and wakers. Damn I am sure I missed something ;) > And no, you are unlikely to see the races in practice (particularly > the memory ordering ones). So you have to *think* about them, not test > them. Yes! agreed again. Oleg.
