| Message ID | 20250124154344.6928-1-sidong.yang@furiosa.ai (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] io_uring/futex: Factor out common free logic into io_free_ifd() | expand |
Hi, > -----Original Message----- > From: Sidong Yang <sidong.yang@furiosa.ai> > Sent: Friday, January 24, 2025 11:44 PM > To: io-uring <io-uring@vger.kernel.org>; Jens Axboe <axboe@kernel.dk> > Cc: Sidong Yang <sidong.yang@furiosa.ai> > Subject: [PATCH v2] io_uring/futex: Factor out common free logic into > io_free_ifd() > > This patch introduces io_free_ifd() that try to cache or free io_futex_data. It > could be used for completion. It also could be used for error path in > io_futex_wait(). Old code just release the ifd but it could be cached. > > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai> > --- > v2: use io_free_ifd() for completion > --- > io_uring/futex.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/io_uring/futex.c b/io_uring/futex.c index > e29662f039e1..94a7159f9cff 100644 > --- a/io_uring/futex.c > +++ b/io_uring/futex.c > @@ -44,6 +44,13 @@ void io_futex_cache_free(struct io_ring_ctx *ctx) > io_alloc_cache_free(&ctx->futex_cache, kfree); } > > +static void io_free_ifd(struct io_ring_ctx *ctx, struct io_futex_data > +*ifd) { > + if (!io_alloc_cache_put(&ctx->futex_cache, ifd)) { > + kfree(ifd); > + } > +} inline static void io_free_ifd(struct io_ring_ctx *ctx, struct io_futex_data *ifd) { if (!io_alloc_cache_put(&ctx->futex_cache, ifd)) kfree(ifd); } Maybe inline function would be better here, and the code format needs to be fine-tuned. > + > static void __io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts) { > req->async_data = NULL; > @@ -57,8 +64,7 @@ static void io_futex_complete(struct io_kiocb *req, struct > io_tw_state *ts) > struct io_ring_ctx *ctx = req->ctx; > > io_tw_lock(ctx, ts); > - if (!io_alloc_cache_put(&ctx->futex_cache, ifd)) > - kfree(ifd); > + io_free_ifd(ctx, ifd); > __io_futex_complete(req, ts); > } > > @@ -353,13 +359,13 @@ int io_futex_wait(struct io_kiocb *req, unsigned int > issue_flags) > return IOU_ISSUE_SKIP_COMPLETE; > } > > + io_free_ifd(ctx, ifd); > done_unlock: > io_ring_submit_unlock(ctx, issue_flags); > done: > if (ret < 0) > req_set_fail(req); > io_req_set_res(req, ret, 0); > - kfree(ifd); Since kfree() is deleted, it is redundant to initialize ifd to NULL. You can consider modifying it like this: - struct io_futex_data *ifd = NULL; struct futex_hash_bucket *hb; + struct io_futex_data *ifd; > return IOU_OK; > } > > -- > 2.43.0 > -- Li Zetao
On Sat, Jan 25, 2025 at 07:17:06AM +0000, lizetao wrote: > Hi, > > > -----Original Message----- > > From: Sidong Yang <sidong.yang@furiosa.ai> > > Sent: Friday, January 24, 2025 11:44 PM > > To: io-uring <io-uring@vger.kernel.org>; Jens Axboe <axboe@kernel.dk> > > Cc: Sidong Yang <sidong.yang@furiosa.ai> > > Subject: [PATCH v2] io_uring/futex: Factor out common free logic into > > io_free_ifd() > > > > This patch introduces io_free_ifd() that try to cache or free io_futex_data. It > > could be used for completion. It also could be used for error path in > > io_futex_wait(). Old code just release the ifd but it could be cached. > > > > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai> > > --- > > v2: use io_free_ifd() for completion > > --- > > io_uring/futex.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/io_uring/futex.c b/io_uring/futex.c index > > e29662f039e1..94a7159f9cff 100644 > > --- a/io_uring/futex.c > > +++ b/io_uring/futex.c > > @@ -44,6 +44,13 @@ void io_futex_cache_free(struct io_ring_ctx *ctx) > > io_alloc_cache_free(&ctx->futex_cache, kfree); } > > > > +static void io_free_ifd(struct io_ring_ctx *ctx, struct io_futex_data > > +*ifd) { > > + if (!io_alloc_cache_put(&ctx->futex_cache, ifd)) { > > + kfree(ifd); > > + } > > +} > > inline static void io_free_ifd(struct io_ring_ctx *ctx, struct io_futex_data *ifd) > { > if (!io_alloc_cache_put(&ctx->futex_cache, ifd)) > kfree(ifd); > } > > Maybe inline function would be better here, and the code format needs to be fine-tuned. Thanks, I missed formatting. inline also good. > > > + > > static void __io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts) { > > req->async_data = NULL; > > @@ -57,8 +64,7 @@ static void io_futex_complete(struct io_kiocb *req, struct > > io_tw_state *ts) > > struct io_ring_ctx *ctx = req->ctx; > > > > io_tw_lock(ctx, ts); > > - if (!io_alloc_cache_put(&ctx->futex_cache, ifd)) > > - kfree(ifd); > > + io_free_ifd(ctx, ifd); > > __io_futex_complete(req, ts); > > } > > > > @@ -353,13 +359,13 @@ int io_futex_wait(struct io_kiocb *req, unsigned int > > issue_flags) > > return IOU_ISSUE_SKIP_COMPLETE; > > } > > > > + io_free_ifd(ctx, ifd); > > done_unlock: > > io_ring_submit_unlock(ctx, issue_flags); > > done: > > if (ret < 0) > > req_set_fail(req); > > io_req_set_res(req, ret, 0); > > - kfree(ifd); > > Since kfree() is deleted, it is redundant to initialize ifd to NULL. You can consider modifying it like this: > - struct io_futex_data *ifd = NULL; > struct futex_hash_bucket *hb; > + struct io_futex_data *ifd; This is also good. but is it actually orthogonal to this patch? ifd could be initialized with io_alloc_ifd(). Thanks, Sidong > > > return IOU_OK; > > } > > > > -- > > 2.43.0 > > > > -- > Li Zetao >
diff --git a/io_uring/futex.c b/io_uring/futex.c index e29662f039e1..94a7159f9cff 100644 --- a/io_uring/futex.c +++ b/io_uring/futex.c @@ -44,6 +44,13 @@ void io_futex_cache_free(struct io_ring_ctx *ctx) io_alloc_cache_free(&ctx->futex_cache, kfree); } +static void io_free_ifd(struct io_ring_ctx *ctx, struct io_futex_data *ifd) +{ + if (!io_alloc_cache_put(&ctx->futex_cache, ifd)) { + kfree(ifd); + } +} + static void __io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts) { req->async_data = NULL; @@ -57,8 +64,7 @@ static void io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts) struct io_ring_ctx *ctx = req->ctx; io_tw_lock(ctx, ts); - if (!io_alloc_cache_put(&ctx->futex_cache, ifd)) - kfree(ifd); + io_free_ifd(ctx, ifd); __io_futex_complete(req, ts); } @@ -353,13 +359,13 @@ int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags) return IOU_ISSUE_SKIP_COMPLETE; } + io_free_ifd(ctx, ifd); done_unlock: io_ring_submit_unlock(ctx, issue_flags); done: if (ret < 0) req_set_fail(req); io_req_set_res(req, ret, 0); - kfree(ifd); return IOU_OK; }
This patch introduces io_free_ifd() that try to cache or free io_futex_data. It could be used for completion. It also could be used for error path in io_futex_wait(). Old code just release the ifd but it could be cached. Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai> --- v2: use io_free_ifd() for completion --- io_uring/futex.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)