Message ID | cover.1655287457.git.asml.silence@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | CQE32 fixes | expand |
On 6/15/22 4:23 AM, Pavel Begunkov wrote: > Several fixes for IORING_SETUP_CQE32 > > Pavel Begunkov (6): > io_uring: get rid of __io_fill_cqe{32}_req() > io_uring: unite fill_cqe and the 32B version > io_uring: fill extra big cqe fields from req > io_uring: fix ->extra{1,2} misuse > io_uring: inline __io_fill_cqe() > io_uring: make io_fill_cqe_aux to honour CQE32 > > fs/io_uring.c | 209 +++++++++++++++++++------------------------------- > 1 file changed, 77 insertions(+), 132 deletions(-) Looks good to me, thanks a lot for doing this work. One minor thing that I'd like to change, but can wait until 5.20, is the completion spots where we pass in both ctx and req. Would be cleaner just to pass in req, and 2 out of 3 spots always do (req->ctx, req) anyway.
On 6/15/22 13:03, Jens Axboe wrote: > On 6/15/22 4:23 AM, Pavel Begunkov wrote: >> Several fixes for IORING_SETUP_CQE32 >> >> Pavel Begunkov (6): >> io_uring: get rid of __io_fill_cqe{32}_req() >> io_uring: unite fill_cqe and the 32B version >> io_uring: fill extra big cqe fields from req >> io_uring: fix ->extra{1,2} misuse >> io_uring: inline __io_fill_cqe() >> io_uring: make io_fill_cqe_aux to honour CQE32 >> >> fs/io_uring.c | 209 +++++++++++++++++++------------------------------- >> 1 file changed, 77 insertions(+), 132 deletions(-) > > Looks good to me, thanks a lot for doing this work. One minor thing that > I'd like to change, but can wait until 5.20, is the completion spots > where we pass in both ctx and req. Would be cleaner just to pass in req, > and 2 out of 3 spots always do (req->ctx, req) anyway. That's because __io_submit_flush_completions() should already have ctx in a register and we care about its performance. We can add a helper if that's an eyesore.
On 6/15/22 6:21 AM, Pavel Begunkov wrote: > On 6/15/22 13:03, Jens Axboe wrote: >> On 6/15/22 4:23 AM, Pavel Begunkov wrote: >>> Several fixes for IORING_SETUP_CQE32 >>> >>> Pavel Begunkov (6): >>> io_uring: get rid of __io_fill_cqe{32}_req() >>> io_uring: unite fill_cqe and the 32B version >>> io_uring: fill extra big cqe fields from req >>> io_uring: fix ->extra{1,2} misuse >>> io_uring: inline __io_fill_cqe() >>> io_uring: make io_fill_cqe_aux to honour CQE32 >>> >>> fs/io_uring.c | 209 +++++++++++++++++++------------------------------- >>> 1 file changed, 77 insertions(+), 132 deletions(-) >> >> Looks good to me, thanks a lot for doing this work. One minor thing that >> I'd like to change, but can wait until 5.20, is the completion spots >> where we pass in both ctx and req. Would be cleaner just to pass in req, >> and 2 out of 3 spots always do (req->ctx, req) anyway. > > That's because __io_submit_flush_completions() should already have > ctx in a register and we care about its performance. We can add > a helper if that's an eyesore. Yeah I realize that, just bothers the eye. Not sure it really matters as we're going to pull that cacheline hot in io_kiocb anyway. We can just leave it for now, it's not a big deal.
On Wed, 15 Jun 2022 11:23:01 +0100, Pavel Begunkov wrote: > Several fixes for IORING_SETUP_CQE32 > > Pavel Begunkov (6): > io_uring: get rid of __io_fill_cqe{32}_req() > io_uring: unite fill_cqe and the 32B version > io_uring: fill extra big cqe fields from req > io_uring: fix ->extra{1,2} misuse > io_uring: inline __io_fill_cqe() > io_uring: make io_fill_cqe_aux to honour CQE32 > > [...] Applied, thanks! [1/6] io_uring: get rid of __io_fill_cqe{32}_req() (no commit info) [2/6] io_uring: unite fill_cqe and the 32B version (no commit info) [3/6] io_uring: fill extra big cqe fields from req (no commit info) [4/6] io_uring: fix ->extra{1,2} misuse (no commit info) [5/6] io_uring: inline __io_fill_cqe() (no commit info) [6/6] io_uring: make io_fill_cqe_aux to honour CQE32 (no commit info) Best regards,