On 9/8/21 2:22 PM, Pavel Begunkov wrote: > On 9/8/21 9:15 PM, Jens Axboe wrote: >> On 9/8/21 2:09 PM, Pavel Begunkov wrote: >>> On 9/8/21 8:57 PM, Jens Axboe wrote: >>>> On 9/8/21 1:49 PM, Pavel Begunkov wrote: >>>>> In case of !SQPOLL, io_cqring_ev_posted_iopoll() doesn't provide a >>>>> memory barrier required by waitqueue_active(&ctx->poll_wait). There is >>>>> a wq_has_sleeper(), which does smb_mb() inside, but it's called only for >>>>> SQPOLL. >>>> >>>> We can probably get rid of the need to even do so by having the slow >>>> path (eg someone waiting on cq_wait or poll_wait) a bit more expensive, >>>> but this should do for now. >>> >>> You have probably seen smp_mb__after_spin_unlock() trick [1], easy way >>> to get rid of it for !IOPOLL. Haven't figured it out for IOPOLL, though >>> >>> [1] https://github.com/isilence/linux/commit/bb391b10d0555ba2d55aa8ee0a08dff8701a6a57 >> >> We can just synchronize the poll_wait() with a spinlock. It's kind of silly, >> and it's especially silly since I bet nobody does poll(2) on the ring fd for >> IOPOLL, but... > > fwiw, for the ebpf cat ev_posted() -> smb_mb() for taking ~3-5%. > And there are non-bpf cases that may benefit from it. > > On my list to publish a refined version of the patch. Maybe let's postpone this patch then and see if we can't do better... -- Jens Axboe
