mbox series

[5.19,0/6] CQE32 fixes

Message ID cover.1655287457.git.asml.silence@gmail.com (mailing list archive)
Headers show
Series CQE32 fixes | expand

Message

Pavel Begunkov June 15, 2022, 10:23 a.m. UTC
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(-)

Comments

Jens Axboe June 15, 2022, 12:03 p.m. UTC | #1
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.
Pavel Begunkov June 15, 2022, 12:21 p.m. UTC | #2
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.
Jens Axboe June 15, 2022, 12:24 p.m. UTC | #3
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.
Jens Axboe June 15, 2022, 9:23 p.m. UTC | #4
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,