* [PATCH 0/2] implement timeout update @ 2020-11-29 17:12 Pavel Begunkov 2020-11-29 17:12 ` [PATCH 1/2] io_uring: restructure io_timeout_cancel() Pavel Begunkov 2020-11-29 17:12 ` [PATCH 2/2] io_uring: add timeout update Pavel Begunkov 0 siblings, 2 replies; 6+ messages in thread From: Pavel Begunkov @ 2020-11-29 17:12 UTC (permalink / raw) To: Jens Axboe, io-uring Timeout update is a IORING_OP_TIMEOUT_REMOVE request with timeout_flags containing a new IORING_TIMEOUT_UPDATE flag. Even though naming may be confusing, but update and remove are very similar both code and functionality wise, so doesn't seem necessary to add a new opcode. Updates don't support offsets, but I don't see a need either. Can be implemented in the future by passing it in sqe->len. Pavel Begunkov (2): io_uring: restructure io_timeout_cancel() io_uring: add timeout update fs/io_uring.c | 90 +++++++++++++++++++++++++++-------- include/uapi/linux/io_uring.h | 1 + 2 files changed, 71 insertions(+), 20 deletions(-) -- 2.24.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] io_uring: restructure io_timeout_cancel() 2020-11-29 17:12 [PATCH 0/2] implement timeout update Pavel Begunkov @ 2020-11-29 17:12 ` Pavel Begunkov 2020-11-29 17:12 ` [PATCH 2/2] io_uring: add timeout update Pavel Begunkov 1 sibling, 0 replies; 6+ messages in thread From: Pavel Begunkov @ 2020-11-29 17:12 UTC (permalink / raw) To: Jens Axboe, io-uring Add io_timeout_extract() helper, which searches and disarms timeouts, but doesn't complete them. No functional changes. Signed-off-by: Pavel Begunkov <[email protected]> --- fs/io_uring.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 12e641c61708..bffcbec6c9be 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5639,24 +5639,10 @@ static enum hrtimer_restart io_timeout_fn(struct hrtimer *timer) return HRTIMER_NORESTART; } -static int __io_timeout_cancel(struct io_kiocb *req) -{ - struct io_timeout_data *io = req->async_data; - int ret; - - ret = hrtimer_try_to_cancel(&io->timer); - if (ret == -1) - return -EALREADY; - list_del_init(&req->timeout.list); - - req_set_fail_links(req); - io_cqring_fill_event(req, -ECANCELED); - io_put_req_deferred(req, 1); - return 0; -} - -static int io_timeout_cancel(struct io_ring_ctx *ctx, __u64 user_data) +static struct io_kiocb *io_timeout_extract(struct io_ring_ctx *ctx, + __u64 user_data) { + struct io_timeout_data *io; struct io_kiocb *req; int ret = -ENOENT; @@ -5668,9 +5654,27 @@ static int io_timeout_cancel(struct io_ring_ctx *ctx, __u64 user_data) } if (ret == -ENOENT) - return ret; + return ERR_PTR(ret); + + io = req->async_data; + ret = hrtimer_try_to_cancel(&io->timer); + if (ret == -1) + return ERR_PTR(-EALREADY); + list_del_init(&req->timeout.list); + return req; +} - return __io_timeout_cancel(req); +static int io_timeout_cancel(struct io_ring_ctx *ctx, __u64 user_data) +{ + struct io_kiocb *req = io_timeout_extract(ctx, user_data); + + if (IS_ERR(req)) + return PTR_ERR(req); + + req_set_fail_links(req); + io_cqring_fill_event(req, -ECANCELED); + io_put_req_deferred(req, 1); + return 0; } static int io_timeout_remove_prep(struct io_kiocb *req, -- 2.24.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] io_uring: add timeout update 2020-11-29 17:12 [PATCH 0/2] implement timeout update Pavel Begunkov 2020-11-29 17:12 ` [PATCH 1/2] io_uring: restructure io_timeout_cancel() Pavel Begunkov @ 2020-11-29 17:12 ` Pavel Begunkov 2020-11-30 18:15 ` Jens Axboe 1 sibling, 1 reply; 6+ messages in thread From: Pavel Begunkov @ 2020-11-29 17:12 UTC (permalink / raw) To: Jens Axboe, io-uring Support timeout updates through IORING_OP_TIMEOUT_REMOVE with passed in IORING_TIMEOUT_UPDATE. Updates doesn't support offset timeout mode. Oirignal timeout.off will be ignored as well. Signed-off-by: Pavel Begunkov <[email protected]> --- fs/io_uring.c | 52 +++++++++++++++++++++++++++++++++-- include/uapi/linux/io_uring.h | 1 + 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index bffcbec6c9be..63d0d546e661 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -453,6 +453,10 @@ struct io_timeout { struct io_timeout_rem { struct file *file; u64 addr; + + /* timeout update */ + struct timespec64 ts; + u32 flags; }; struct io_rw { @@ -5677,17 +5681,51 @@ static int io_timeout_cancel(struct io_ring_ctx *ctx, __u64 user_data) return 0; } +static int io_timeout_update(struct io_ring_ctx *ctx, __u64 user_data, + struct timespec64 *ts, enum hrtimer_mode mode) +{ + struct io_kiocb *req = io_timeout_extract(ctx, user_data); + struct io_timeout_data *data; + + if (IS_ERR(req)) + return PTR_ERR(req); + + req->timeout.off = 0; /* noseq */ + data = req->async_data; + list_add_tail(&req->timeout.list, &ctx->timeout_list); + hrtimer_init(&data->timer, CLOCK_MONOTONIC, mode); + data->timer.function = io_timeout_fn; + hrtimer_start(&data->timer, timespec64_to_ktime(*ts), mode); + return 0; +} + static int io_timeout_remove_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { + struct io_timeout_rem *tr = &req->timeout_rem; + int ret; + if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL)) return -EINVAL; if (unlikely(req->flags & (REQ_F_FIXED_FILE | REQ_F_BUFFER_SELECT))) return -EINVAL; - if (sqe->ioprio || sqe->buf_index || sqe->len || sqe->timeout_flags) + if (sqe->ioprio || sqe->buf_index || sqe->len) return -EINVAL; - req->timeout_rem.addr = READ_ONCE(sqe->addr); + tr->addr = READ_ONCE(sqe->addr); + tr->flags = READ_ONCE(sqe->timeout_flags); + if (tr->flags) { + if (!(tr->flags & IORING_TIMEOUT_UPDATE)) + return -EINVAL; + if (tr->flags & ~(IORING_TIMEOUT_UPDATE|IORING_TIMEOUT_ABS)) + return -EINVAL; + + ret = __io_sq_thread_acquire_mm(req->ctx); + if (ret) + return ret; + if (get_timespec64(&tr->ts, u64_to_user_ptr(sqe->addr2))) + return -EFAULT; + } return 0; } @@ -5696,11 +5734,19 @@ static int io_timeout_remove_prep(struct io_kiocb *req, */ static int io_timeout_remove(struct io_kiocb *req) { + struct io_timeout_rem *tr = &req->timeout_rem; struct io_ring_ctx *ctx = req->ctx; int ret; spin_lock_irq(&ctx->completion_lock); - ret = io_timeout_cancel(ctx, req->timeout_rem.addr); + if (req->timeout_rem.flags & IORING_TIMEOUT_UPDATE) { + enum hrtimer_mode mode = (tr->flags & IORING_TIMEOUT_ABS) + ? HRTIMER_MODE_ABS : HRTIMER_MODE_REL; + + ret = io_timeout_update(ctx, tr->addr, &tr->ts, mode); + } else { + ret = io_timeout_cancel(ctx, tr->addr); + } io_cqring_fill_event(req, ret); io_commit_cqring(ctx); diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h index 6bb8229de892..12a6443ea60d 100644 --- a/include/uapi/linux/io_uring.h +++ b/include/uapi/linux/io_uring.h @@ -151,6 +151,7 @@ enum { * sqe->timeout_flags */ #define IORING_TIMEOUT_ABS (1U << 0) +#define IORING_TIMEOUT_UPDATE (1U << 31) /* * sqe->splice_flags -- 2.24.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] io_uring: add timeout update 2020-11-29 17:12 ` [PATCH 2/2] io_uring: add timeout update Pavel Begunkov @ 2020-11-30 18:15 ` Jens Axboe 2020-11-30 18:27 ` Pavel Begunkov 0 siblings, 1 reply; 6+ messages in thread From: Jens Axboe @ 2020-11-30 18:15 UTC (permalink / raw) To: Pavel Begunkov, io-uring On 11/29/20 10:12 AM, Pavel Begunkov wrote: > + tr->flags = READ_ONCE(sqe->timeout_flags); > + if (tr->flags) { > + if (!(tr->flags & IORING_TIMEOUT_UPDATE)) > + return -EINVAL; > + if (tr->flags & ~(IORING_TIMEOUT_UPDATE|IORING_TIMEOUT_ABS)) > + return -EINVAL; These flag comparisons are a bit obtuse - perhaps warrants a comment? > + ret = __io_sq_thread_acquire_mm(req->ctx); > + if (ret) > + return ret; Why is this done manually? > diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h > index 6bb8229de892..12a6443ea60d 100644 > --- a/include/uapi/linux/io_uring.h > +++ b/include/uapi/linux/io_uring.h > @@ -151,6 +151,7 @@ enum { > * sqe->timeout_flags > */ > #define IORING_TIMEOUT_ABS (1U << 0) > +#define IORING_TIMEOUT_UPDATE (1U << 31) Why bit 31? -- Jens Axboe ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] io_uring: add timeout update 2020-11-30 18:15 ` Jens Axboe @ 2020-11-30 18:27 ` Pavel Begunkov 2020-11-30 18:40 ` Jens Axboe 0 siblings, 1 reply; 6+ messages in thread From: Pavel Begunkov @ 2020-11-30 18:27 UTC (permalink / raw) To: Jens Axboe, io-uring On 30/11/2020 18:15, Jens Axboe wrote: > On 11/29/20 10:12 AM, Pavel Begunkov wrote: >> + tr->flags = READ_ONCE(sqe->timeout_flags); >> + if (tr->flags) { >> + if (!(tr->flags & IORING_TIMEOUT_UPDATE)) >> + return -EINVAL; >> + if (tr->flags & ~(IORING_TIMEOUT_UPDATE|IORING_TIMEOUT_ABS)) >> + return -EINVAL; > > These flag comparisons are a bit obtuse - perhaps warrants a comment? Ok, the one below should be more readable. if (tr->flags & IORING_TIMEOUT_UPDATE) { if (flags & ~ALLOWED_UPDATE_FLAGS) return -EINVAL; ... } else if (tr->flags) { /* timeout removal doesn't support flags */ return -EINVAL; } > >> + ret = __io_sq_thread_acquire_mm(req->ctx); >> + if (ret) >> + return ret; > > Why is this done manually? mm is only needed in *prep(), so don't want IO_WQ_WORK_MM to put it into req->work since it also affects timeout remove reqs. > >> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h >> index 6bb8229de892..12a6443ea60d 100644 >> --- a/include/uapi/linux/io_uring.h >> +++ b/include/uapi/linux/io_uring.h >> @@ -151,6 +151,7 @@ enum { >> * sqe->timeout_flags >> */ >> #define IORING_TIMEOUT_ABS (1U << 0) >> +#define IORING_TIMEOUT_UPDATE (1U << 31) > > Why bit 31? Left bits for other potential timeout modes, don't know which though. Can return it to bit 1. -- Pavel Begunkov ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] io_uring: add timeout update 2020-11-30 18:27 ` Pavel Begunkov @ 2020-11-30 18:40 ` Jens Axboe 0 siblings, 0 replies; 6+ messages in thread From: Jens Axboe @ 2020-11-30 18:40 UTC (permalink / raw) To: Pavel Begunkov, io-uring On 11/30/20 11:27 AM, Pavel Begunkov wrote: > On 30/11/2020 18:15, Jens Axboe wrote: >> On 11/29/20 10:12 AM, Pavel Begunkov wrote: >>> + tr->flags = READ_ONCE(sqe->timeout_flags); >>> + if (tr->flags) { >>> + if (!(tr->flags & IORING_TIMEOUT_UPDATE)) >>> + return -EINVAL; >>> + if (tr->flags & ~(IORING_TIMEOUT_UPDATE|IORING_TIMEOUT_ABS)) >>> + return -EINVAL; >> >> These flag comparisons are a bit obtuse - perhaps warrants a comment? > > Ok, the one below should be more readable. > > if (tr->flags & IORING_TIMEOUT_UPDATE) { > if (flags & ~ALLOWED_UPDATE_FLAGS) > return -EINVAL; > ... > } else if (tr->flags) { > /* timeout removal doesn't support flags */ > return -EINVAL; > } Yeah, that's actually readable. >>> + ret = __io_sq_thread_acquire_mm(req->ctx); >>> + if (ret) >>> + return ret; >> >> Why is this done manually? > > mm is only needed in *prep(), so don't want IO_WQ_WORK_MM to put it > into req->work since it also affects timeout remove reqs. Don't think it's worth it, I'd just mark it needing it uncondtionally instead. Reason being the obvious of not having stuff like this buried deep in an op hander, but also that for non SQPOLL, we'll do these inline always and it's a no-op. For SQPOLL we'd need to grab it, but I'd rather pay that cost than have this in there. >>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h >>> index 6bb8229de892..12a6443ea60d 100644 >>> --- a/include/uapi/linux/io_uring.h >>> +++ b/include/uapi/linux/io_uring.h >>> @@ -151,6 +151,7 @@ enum { >>> * sqe->timeout_flags >>> */ >>> #define IORING_TIMEOUT_ABS (1U << 0) >>> +#define IORING_TIMEOUT_UPDATE (1U << 31) >> >> Why bit 31? > > Left bits for other potential timeout modes, don't know which though. > Can return it to bit 1. Let's just use 1, there's no clear separation in there anyway. -- Jens Axboe ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-30 18:41 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-29 17:12 [PATCH 0/2] implement timeout update Pavel Begunkov 2020-11-29 17:12 ` [PATCH 1/2] io_uring: restructure io_timeout_cancel() Pavel Begunkov 2020-11-29 17:12 ` [PATCH 2/2] io_uring: add timeout update Pavel Begunkov 2020-11-30 18:15 ` Jens Axboe 2020-11-30 18:27 ` Pavel Begunkov 2020-11-30 18:40 ` Jens Axboe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox