[PATCH] io_uring: make timeout sequence == 0 mean no sequence

[PATCH] io_uring: make timeout sequence == 0 mean no sequence

[Jens Axboe]

Currently we make sequence == 0 be the same as sequence == 1, but that's
not super useful if the intent is really to have a timeout that's just
a pure timeout.

If the user passes in sqe->off == 0, then don't apply any sequence logic
to the request, let it purely be driven by the timeout specified.

Reported-by: 李通洲 <carter.li@eoitek.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

---

Carter brings up a valid point that we should be able to support pure
timeouts, that don't factor in the sequence at all. Since timeouts were
introduced in this merge window, now's the time to make the correction.

diff --git a/fs/io_uring.c b/fs/io_uring.c
index f9a38998f2fc..87beca4377f7 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -326,6 +326,7 @@ struct io_kiocb {
 #define REQ_F_TIMEOUT		1024	/* timeout request */
 #define REQ_F_ISREG		2048	/* regular file */
 #define REQ_F_MUST_PUNT		4096	/* must be punted even for NONBLOCK */
+#define REQ_F_TIMEOUT_NOSEQ	8192	/* no timeout sequence */
 	u64			user_data;
 	u32			result;
 	u32			sequence;
@@ -453,9 +454,13 @@ static struct io_kiocb *io_get_timeout_req(struct io_ring_ctx *ctx)
 	struct io_kiocb *req;
 
 	req = list_first_entry_or_null(&ctx->timeout_list, struct io_kiocb, list);
-	if (req && !__io_sequence_defer(ctx, req)) {
-		list_del_init(&req->list);
-		return req;
+	if (req) {
+		if (req->flags & REQ_F_TIMEOUT_NOSEQ)
+			return NULL;
+		if (!__io_sequence_defer(ctx, req)) {
+			list_del_init(&req->list);
+			return req;
+		}
 	}
 
 	return NULL;
@@ -1941,18 +1946,24 @@ static int io_timeout(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (get_timespec64(&ts, u64_to_user_ptr(sqe->addr)))
 		return -EFAULT;
 
+	req->flags |= REQ_F_TIMEOUT;
+
 	/*
 	 * sqe->off holds how many events that need to occur for this
-	 * timeout event to be satisfied.
+	 * timeout event to be satisfied. If it isn't set, then this is
+	 * a pure timeout request, sequence isn't used.
 	 */
 	count = READ_ONCE(sqe->off);
-	if (!count)
-		count = 1;
+	if (!count) {
+		req->flags |= REQ_F_TIMEOUT_NOSEQ;
+		spin_lock_irq(&ctx->completion_lock);
+		entry = ctx->timeout_list.prev;
+		goto add;
+	}
 
 	req->sequence = ctx->cached_sq_head + count - 1;
 	/* reuse it to store the count */
 	req->submit.sequence = count;
-	req->flags |= REQ_F_TIMEOUT;
 
 	/*
 	 * Insertion sort, ensuring the first entry in the list is always
@@ -1964,6 +1975,9 @@ static int io_timeout(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		unsigned nxt_sq_head;
 		long long tmp, tmp_nxt;
 
+		if (nxt->flags & REQ_F_TIMEOUT_NOSEQ)
+			continue;
+
 		/*
 		 * Since cached_sq_head + count - 1 can overflow, use type long
 		 * long to store it.
@@ -1990,6 +2004,7 @@ static int io_timeout(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		nxt->sequence++;
 	}
 	req->sequence -= span;
+add:
 	list_add(&req->list, entry);
 	spin_unlock_irq(&ctx->completion_lock);
 
-- 
Jens Axboe

Re: [PATCH] io_uring: make timeout sequence == 0 mean no sequence

[Carter Li 李通洲]

Great patch! io_uring is becoming more and more user friendly.

Reviewed-by: 李通洲 <carter.li@eoitek.com>

--
Carter,

在 2019/11/12 下午2:38，“Jens Axboe”<axboe@kernel.dk> 写入:

    Currently we make sequence == 0 be the same as sequence == 1, but that's
    not super useful if the intent is really to have a timeout that's just
    a pure timeout.
    
    If the user passes in sqe->off == 0, then don't apply any sequence logic
    to the request, let it purely be driven by the timeout specified.
    
    Reported-by: 李通洲 <carter.li@eoitek.com>
    Signed-off-by: Jens Axboe <axboe@kernel.dk>
    
    ---
    
    Carter brings up a valid point that we should be able to support pure
    timeouts, that don't factor in the sequence at all. Since timeouts were
    introduced in this merge window, now's the time to make the correction.
    
    diff --git a/fs/io_uring.c b/fs/io_uring.c
    index f9a38998f2fc..87beca4377f7 100644
    --- a/fs/io_uring.c
    +++ b/fs/io_uring.c
    @@ -326,6 +326,7 @@ struct io_kiocb {
     #define REQ_F_TIMEOUT		1024	/* timeout request */
     #define REQ_F_ISREG		2048	/* regular file */
     #define REQ_F_MUST_PUNT		4096	/* must be punted even for NONBLOCK */
    +#define REQ_F_TIMEOUT_NOSEQ	8192	/* no timeout sequence */
     	u64			user_data;
     	u32			result;
     	u32			sequence;
    @@ -453,9 +454,13 @@ static struct io_kiocb *io_get_timeout_req(struct io_ring_ctx *ctx)
     	struct io_kiocb *req;
     
     	req = list_first_entry_or_null(&ctx->timeout_list, struct io_kiocb, list);
    -	if (req && !__io_sequence_defer(ctx, req)) {
    -		list_del_init(&req->list);
    -		return req;
    +	if (req) {
    +		if (req->flags & REQ_F_TIMEOUT_NOSEQ)
    +			return NULL;
    +		if (!__io_sequence_defer(ctx, req)) {
    +			list_del_init(&req->list);
    +			return req;
    +		}
     	}
     
     	return NULL;
    @@ -1941,18 +1946,24 @@ static int io_timeout(struct io_kiocb *req, const struct io_uring_sqe *sqe)
     	if (get_timespec64(&ts, u64_to_user_ptr(sqe->addr)))
     		return -EFAULT;
     
    +	req->flags |= REQ_F_TIMEOUT;
    +
     	/*
     	 * sqe->off holds how many events that need to occur for this
    -	 * timeout event to be satisfied.
    +	 * timeout event to be satisfied. If it isn't set, then this is
    +	 * a pure timeout request, sequence isn't used.
     	 */
     	count = READ_ONCE(sqe->off);
    -	if (!count)
    -		count = 1;
    +	if (!count) {
    +		req->flags |= REQ_F_TIMEOUT_NOSEQ;
    +		spin_lock_irq(&ctx->completion_lock);
    +		entry = ctx->timeout_list.prev;
    +		goto add;
    +	}
     
     	req->sequence = ctx->cached_sq_head + count - 1;
     	/* reuse it to store the count */
     	req->submit.sequence = count;
    -	req->flags |= REQ_F_TIMEOUT;
     
     	/*
     	 * Insertion sort, ensuring the first entry in the list is always
    @@ -1964,6 +1975,9 @@ static int io_timeout(struct io_kiocb *req, const struct io_uring_sqe *sqe)
     		unsigned nxt_sq_head;
     		long long tmp, tmp_nxt;
     
    +		if (nxt->flags & REQ_F_TIMEOUT_NOSEQ)
    +			continue;
    +
     		/*
     		 * Since cached_sq_head + count - 1 can overflow, use type long
     		 * long to store it.
    @@ -1990,6 +2004,7 @@ static int io_timeout(struct io_kiocb *req, const struct io_uring_sqe *sqe)
     		nxt->sequence++;
     	}
     	req->sequence -= span;
    +add:
     	list_add(&req->list, entry);
     	spin_unlock_irq(&ctx->completion_lock);
     
    -- 
    Jens Axboe