| Message ID | 20250205202641.646812-6-axboe@kernel.dk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Cancelation cleanups | expand |
Hi, > -----Original Message----- > From: Jens Axboe <axboe@kernel.dk> > Sent: Thursday, February 6, 2025 4:26 AM > To: io-uring@vger.kernel.org > Cc: Jens Axboe <axboe@kernel.dk> > Subject: [PATCH 5/6] io_uring/futex: use generic io_cancel_remove() helper > > Don't implement our own loop rolling and checking, just use the generic helper to > find and cancel requests. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> > --- > io_uring/futex.c | 24 +----------------------- > 1 file changed, 1 insertion(+), 23 deletions(-) > > diff --git a/io_uring/futex.c b/io_uring/futex.c index > 808eb57f1210..54b9760f2aa6 100644 > --- a/io_uring/futex.c > +++ b/io_uring/futex.c > @@ -116,29 +116,7 @@ static bool __io_futex_cancel(struct io_kiocb *req) int > io_futex_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd, > unsigned int issue_flags) > { > - struct hlist_node *tmp; > - struct io_kiocb *req; > - int nr = 0; > - > - if (cd->flags & > (IORING_ASYNC_CANCEL_FD|IORING_ASYNC_CANCEL_FD_FIXED)) > - return -ENOENT; Why remove this check? > - > - io_ring_submit_lock(ctx, issue_flags); > - hlist_for_each_entry_safe(req, tmp, &ctx->futex_list, hash_node) { > - if (req->cqe.user_data != cd->data && > - !(cd->flags & IORING_ASYNC_CANCEL_ANY)) > - continue; > - if (__io_futex_cancel(req)) > - nr++; > - if (!(cd->flags & IORING_ASYNC_CANCEL_ALL)) > - break; > - } > - io_ring_submit_unlock(ctx, issue_flags); > - > - if (nr) > - return nr; > - > - return -ENOENT; > + return io_cancel_remove(ctx, cd, issue_flags, &ctx->futex_list, > +__io_futex_cancel); > } > > bool io_futex_remove_all(struct io_ring_ctx *ctx, struct io_uring_task *tctx, > -- > 2.47.2 > > --- Li Zetao
On 2/6/25 5:56 AM, lizetao wrote: > Hi, > >> -----Original Message----- >> From: Jens Axboe <axboe@kernel.dk> >> Sent: Thursday, February 6, 2025 4:26 AM >> To: io-uring@vger.kernel.org >> Cc: Jens Axboe <axboe@kernel.dk> >> Subject: [PATCH 5/6] io_uring/futex: use generic io_cancel_remove() helper >> >> Don't implement our own loop rolling and checking, just use the generic helper to >> find and cancel requests. >> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> >> --- >> io_uring/futex.c | 24 +----------------------- >> 1 file changed, 1 insertion(+), 23 deletions(-) >> >> diff --git a/io_uring/futex.c b/io_uring/futex.c index >> 808eb57f1210..54b9760f2aa6 100644 >> --- a/io_uring/futex.c >> +++ b/io_uring/futex.c >> @@ -116,29 +116,7 @@ static bool __io_futex_cancel(struct io_kiocb *req) int >> io_futex_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd, >> unsigned int issue_flags) >> { >> - struct hlist_node *tmp; >> - struct io_kiocb *req; >> - int nr = 0; >> - >> - if (cd->flags & >> (IORING_ASYNC_CANCEL_FD|IORING_ASYNC_CANCEL_FD_FIXED)) >> - return -ENOENT; > > Why remove this check? It isn't really necessary. Yes we could loop pointlessly if they are set and not find anything, but it's really just a bad use case from the application. End result should be the same, that -ENOENT is returned on trying to lookup a futex operation based on the fd.
diff --git a/io_uring/futex.c b/io_uring/futex.c index 808eb57f1210..54b9760f2aa6 100644 --- a/io_uring/futex.c +++ b/io_uring/futex.c @@ -116,29 +116,7 @@ static bool __io_futex_cancel(struct io_kiocb *req) int io_futex_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd, unsigned int issue_flags) { - struct hlist_node *tmp; - struct io_kiocb *req; - int nr = 0; - - if (cd->flags & (IORING_ASYNC_CANCEL_FD|IORING_ASYNC_CANCEL_FD_FIXED)) - return -ENOENT; - - io_ring_submit_lock(ctx, issue_flags); - hlist_for_each_entry_safe(req, tmp, &ctx->futex_list, hash_node) { - if (req->cqe.user_data != cd->data && - !(cd->flags & IORING_ASYNC_CANCEL_ANY)) - continue; - if (__io_futex_cancel(req)) - nr++; - if (!(cd->flags & IORING_ASYNC_CANCEL_ALL)) - break; - } - io_ring_submit_unlock(ctx, issue_flags); - - if (nr) - return nr; - - return -ENOENT; + return io_cancel_remove(ctx, cd, issue_flags, &ctx->futex_list, __io_futex_cancel); } bool io_futex_remove_all(struct io_ring_ctx *ctx, struct io_uring_task *tctx,
Don't implement our own loop rolling and checking, just use the generic helper to find and cancel requests. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- io_uring/futex.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-)