On 10/9/24 1:27 PM, Pavel Begunkov wrote: >>>>> + /* All data completions are posted as aux CQEs. */ >>>>> + req->flags |= REQ_F_APOLL_MULTISHOT; >>>> >>>> This puzzles me a bit... >>> >>> Well, it's a multishot request. And that flag protects from cq >>> locking rules violations, i.e. avoiding multishot reqs from >>> posting from io-wq. >> >> Maybe make it more like the others and require that >> IORING_RECV_MULTISHOT is set then, and set it based on that? > > if (IORING_RECV_MULTISHOT) > return -EINVAL; > req->flags |= REQ_F_APOLL_MULTISHOT; > > It can be this if that's the preference. It's a bit more consistent, > but might be harder to use. Though I can just hide the flag behind > liburing helpers, would spare from neverending GH issues asking > why it's -EINVAL'ed Maybe I'm missing something, but why not make it: /* multishot required */ if (!(flags & IORING_RECV_MULTISHOT)) return -EINVAL; req->flags |= REQ_F_APOLL_MULTISHOT; and yeah just put it in the io_uring_prep_recv_zc() or whatever helper. That would seem to be a lot more consistent with other users, no? >>>>> + zc->flags = READ_ONCE(sqe->ioprio); >>>>> + zc->msg_flags = READ_ONCE(sqe->msg_flags); >>>>> + if (zc->msg_flags) >>>>> + return -EINVAL; >>>> >>>> Maybe allow MSG_DONTWAIT at least? You already pass that in anyway. >>> >>> What would the semantics be? The io_uring nowait has always >>> been a pure mess because it's not even clear what it supposed >>> to mean for async requests. >> >> Yeah can't disagree with that. Not a big deal, doesn't really matter, >> can stay as-is. > > I went through the MSG_* flags before looking which ones might > even make sense here and be useful... Let's better enable if > it'd be needed. Yep that's fine. -- Jens Axboe
