Re: [PATCH for-next] io_uring: add async data clear/free helpers

[Caleb Sander Mateos <csander@purestorage.com>]

On Tue, Aug 26, 2025 at 9:04 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 8/26/25 9:27 AM, Caleb Sander Mateos wrote:
> >> diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
> >> index 2e4f7223a767..86613b8224bd 100644
> >> --- a/io_uring/io_uring.h
> >> +++ b/io_uring/io_uring.h
> >> @@ -281,6 +281,19 @@ static inline bool req_has_async_data(struct io_kiocb *req)
> >>         return req->flags & REQ_F_ASYNC_DATA;
> >>  }
> >>
> >> +static inline void io_req_async_data_clear(struct io_kiocb *req,
> >> +                                          io_req_flags_t extra_flags)
> >> +{
> >> +       req->flags &= ~(REQ_F_ASYNC_DATA|extra_flags);
> >> +       req->async_data = NULL;
> >> +}
> >> +
> >> +static inline void io_req_async_data_free(struct io_kiocb *req)
> >> +{
> >> +       kfree(req->async_data);
> >> +       io_req_async_data_clear(req, 0);
> >> +}
> >
> > Would it make sense to also add a helper for assigning async_data that
> > would also make sure to set REQ_F_ASYNC_DATA?
>
> I did consider that, but it's only futex that'd use it and just in those
> two spots. So decided against it. At least for now.

Makes sense. I think it could also be used in
io_uring_alloc_async_data(), but it's true there are fewer sites
assigning to async_data than clearing it.

Best,
Caleb

>
> >> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> >> index ff1d029633b8..09f2a47a0020 100644
> >> --- a/io_uring/uring_cmd.c
> >> +++ b/io_uring/uring_cmd.c
> >> @@ -37,8 +37,7 @@ static void io_req_uring_cleanup(struct io_kiocb *req, unsigned int issue_flags)
> >>
> >>         if (io_alloc_cache_put(&req->ctx->cmd_cache, ac)) {
> >>                 ioucmd->sqe = NULL;
> >> -               req->async_data = NULL;
> >> -               req->flags &= ~(REQ_F_ASYNC_DATA|REQ_F_NEED_CLEANUP);
> >> +               io_req_async_data_clear(req, 0);
> >
> > Looks like the REQ_F_NEED_CLEANUP got lost here. Other than that,
>
> Oops indeed, added back.
>
> > Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
>
> Thanks for the review!
>
> --
> Jens Axboe