On 26/02/2020 01:18, Jens Axboe wrote:
> So this found something funky, we really should only be picking up
> the next request if we're dropping the final reference to the
> request. And io_put_req_find_next() also says that in the comment,
> but it always looks it up. That doesn't seem safe at all, I think
> this is what it should be:
It was weird indeed, it looks good. And now it's safe to do the same in
io_wq_submit_work().
Interestingly, this means that passing @nxt into the handlers is useless, as
they won't ever return !=NULL, isn't it? I'll prepare the cleanup.
>
> commit eff5fe974f332c1b86c9bb274627e88b4ecbbc85
> Author: Jens Axboe <axboe@xxxxxxxxx>
> Date: Tue Feb 25 13:25:41 2020 -0700
>
> io_uring: pick up link work on submit reference drop
>
> If work completes inline, then we should pick up a dependent link item
> in __io_queue_sqe() as well. If we don't do so, we're forced to go async
> with that item, which is suboptimal.
>
> This also fixes an issue with io_put_req_find_next(), which always looks
> up the next work item. That should only be done if we're dropping the
> last reference to the request, to prevent multiple lookups of the same
> work item.
>
> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index ffd9bfa84d86..f79ca494bb56 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -1483,10 +1483,10 @@ static void io_free_req(struct io_kiocb *req)
> __attribute__((nonnull))
> static void io_put_req_find_next(struct io_kiocb *req, struct io_kiocb **nxtptr)
> {
> - io_req_find_next(req, nxtptr);
> -
> - if (refcount_dec_and_test(&req->refs))
> + if (refcount_dec_and_test(&req->refs)) {
> + io_req_find_next(req, nxtptr);
> __io_free_req(req);
> + }
> }
>
> static void io_put_req(struct io_kiocb *req)
> @@ -4749,7 +4749,7 @@ static void __io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>
> err:
> /* drop submission reference */
> - io_put_req(req);
> + io_put_req_find_next(req, &nxt);
>
> if (linked_timeout) {
> if (!ret)
>
--
Pavel Begunkov
Attachment:
signature.asc
Description: OpenPGP digital signature
