* [PATCH 1/2] io_uring: make req from defer and link list not touch async list
@ 2019-07-13 3:58 Zhengyuan Liu
2019-07-13 3:58 ` [PATCH 2/2] io_uring: fix the judging condition in io_sequence_defer Zhengyuan Liu
2019-07-16 14:08 ` [PATCH 1/2] io_uring: make req from defer and link list not touch async list Jens Axboe
0 siblings, 2 replies; 5+ messages in thread
From: Zhengyuan Liu @ 2019-07-13 3:58 UTC (permalink / raw)
To: axboe; +Cc: linux-block, Zhengyuan Liu
We would queue a work for each req in defer and link list without
increasing async->cnt, so we shouldn't decrease it while exiting
from workqueue as well as shouldn't process the req in async list.
Signed-off-by: Zhengyuan Liu <liuzhengyuan@kylinos.cn>
---
fs/io_uring.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 7e932c572f26..3e48fd7cd08f 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -333,7 +333,8 @@ struct io_kiocb {
#define REQ_F_IO_DRAIN 16 /* drain existing IO first */
#define REQ_F_IO_DRAINED 32 /* drain done */
#define REQ_F_LINK 64 /* linked sqes */
-#define REQ_F_FAIL_LINK 128 /* fail rest of links */
+#define REQ_F_LINKED 128 /* linked sqes done */
+#define REQ_F_FAIL_LINK 256 /* fail rest of links */
u64 user_data;
u32 result;
u32 sequence;
@@ -630,6 +631,7 @@ static void io_req_link_next(struct io_kiocb *req)
nxt->flags |= REQ_F_LINK;
}
+ nxt->flags |= REQ_F_LINKED;
INIT_WORK(&nxt->work, io_sq_wq_submit_work);
queue_work(req->ctx->sqo_wq, &nxt->work);
}
@@ -1845,6 +1847,10 @@ static void io_sq_wq_submit_work(struct work_struct *work)
/* async context always use a copy of the sqe */
kfree(sqe);
+ /* req from defer and link list needn't dec async_list->cnt */
+ if (req->flags & (REQ_F_IO_DRAINED | REQ_F_LINKED))
+ goto out;
+
if (!async_list)
break;
if (!list_empty(&req_list)) {
@@ -1892,6 +1898,7 @@ static void io_sq_wq_submit_work(struct work_struct *work)
}
}
+out:
if (cur_mm) {
set_fs(old_fs);
unuse_mm(cur_mm);
--
2.19.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] io_uring: fix the judging condition in io_sequence_defer
2019-07-13 3:58 [PATCH 1/2] io_uring: make req from defer and link list not touch async list Zhengyuan Liu
@ 2019-07-13 3:58 ` Zhengyuan Liu
2019-07-13 21:43 ` Jens Axboe
2019-07-16 14:08 ` [PATCH 1/2] io_uring: make req from defer and link list not touch async list Jens Axboe
1 sibling, 1 reply; 5+ messages in thread
From: Zhengyuan Liu @ 2019-07-13 3:58 UTC (permalink / raw)
To: axboe; +Cc: linux-block, Zhengyuan Liu
sq->cached_sq_head and cq->cached_cq_tail are both unsigned int.
if cached_sq_head gets overflowed before cached_cq_tail, then we
may miss a barrier req. As cached_cq_tail moved always following
cached_sq_head, the NQ should be enough.
Signed-off-by: Zhengyuan Liu <liuzhengyuan@kylinos.cn>
---
fs/io_uring.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 3e48fd7cd08f..55294ef82102 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -429,7 +429,7 @@ static inline bool io_sequence_defer(struct io_ring_ctx *ctx,
if ((req->flags & (REQ_F_IO_DRAIN|REQ_F_IO_DRAINED)) != REQ_F_IO_DRAIN)
return false;
- return req->sequence > ctx->cached_cq_tail + ctx->sq_ring->dropped;
+ return req->sequence != ctx->cached_cq_tail + ctx->sq_ring->dropped;
}
static struct io_kiocb *io_get_deferred_req(struct io_ring_ctx *ctx)
--
2.19.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] io_uring: fix the judging condition in io_sequence_defer
2019-07-13 3:58 ` [PATCH 2/2] io_uring: fix the judging condition in io_sequence_defer Zhengyuan Liu
@ 2019-07-13 21:43 ` Jens Axboe
[not found] ` <5d2be620.1c69fb81.fff62.8eeeSMTPIN_ADDED_BROKEN@mx.google.com>
0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2019-07-13 21:43 UTC (permalink / raw)
To: Zhengyuan Liu; +Cc: linux-block
On 7/12/19 9:58 PM, Zhengyuan Liu wrote:
> sq->cached_sq_head and cq->cached_cq_tail are both unsigned int.
> if cached_sq_head gets overflowed before cached_cq_tail, then we
> may miss a barrier req. As cached_cq_tail moved always following
> cached_sq_head, the NQ should be enough.
This (and the previous patch) looks fine to me, just wondering if
you had a test case showing this issue?
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] io_uring: make req from defer and link list not touch async list
2019-07-13 3:58 [PATCH 1/2] io_uring: make req from defer and link list not touch async list Zhengyuan Liu
2019-07-13 3:58 ` [PATCH 2/2] io_uring: fix the judging condition in io_sequence_defer Zhengyuan Liu
@ 2019-07-16 14:08 ` Jens Axboe
1 sibling, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2019-07-16 14:08 UTC (permalink / raw)
To: Zhengyuan Liu; +Cc: linux-block
On 7/12/19 9:58 PM, Zhengyuan Liu wrote:
> We would queue a work for each req in defer and link list without
> increasing async->cnt, so we shouldn't decrease it while exiting
> from workqueue as well as shouldn't process the req in async list.
Took a closer look at this one. First comment, can you rewrite
the subject line? I honestly don't quite know what that is
trying to say.
Apart from that:
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 7e932c572f26..3e48fd7cd08f 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -333,7 +333,8 @@ struct io_kiocb {
> #define REQ_F_IO_DRAIN 16 /* drain existing IO first */
> #define REQ_F_IO_DRAINED 32 /* drain done */
> #define REQ_F_LINK 64 /* linked sqes */
> -#define REQ_F_FAIL_LINK 128 /* fail rest of links */
> +#define REQ_F_LINKED 128 /* linked sqes done */
> +#define REQ_F_FAIL_LINK 256 /* fail rest of links */
_LINKED is poorly chosen, hard to know what the difference between
_LINK and _LINKED is here, they seem identical. Please rename it to
_LINK_DONE or something like that.
Thanks!
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] io_uring: fix the judging condition in io_sequence_defer
[not found] ` <5d2be620.1c69fb81.fff62.8eeeSMTPIN_ADDED_BROKEN@mx.google.com>
@ 2019-07-16 14:10 ` Jens Axboe
0 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2019-07-16 14:10 UTC (permalink / raw)
To: Zhengyuan Liu; +Cc: linux-block
On 7/14/19 8:34 PM, Zhengyuan Liu wrote:
>
> On 7/14/19 5:43 AM, Jens Axboe wrote:
>> On 7/12/19 9:58 PM, Zhengyuan Liu wrote:
>>> sq->cached_sq_head and cq->cached_cq_tail are both unsigned int.
>>> if cached_sq_head gets overflowed before cached_cq_tail, then we
>>> may miss a barrier req. As cached_cq_tail moved always following
>>> cached_sq_head, the NQ should be enough.
>> This (and the previous patch) looks fine to me, just wondering if
>> you had a test case showing this issue?
>>
> Hi.
>
> Actually I don't have a real test case, but I have emulated a test case
> and showed the issue.
>
> Let's preset those count to close overflowed status at the begin:
>
> @@ -2979,6 +2987,10 @@ static int io_allocate_scq_urings(struct
> io_ring_ctx *ctx,
> cq_ring->ring_entries = p->cq_entries;
> ctx->cq_mask = cq_ring->ring_mask;
> ctx->cq_entries = cq_ring->ring_entries;
> +
> + ctx->cached_sq_head = ctx->sq_ring->r.head =
> ctx->sq_ring->r.tail = 0xfffffff8;
> + ctx->cached_cq_tail = ctx->cq_ring->r.head =
> ctx->cq_ring->r.tail = 0xfffffff8
> +
> return 0;
> }
>
> And queue serveral sqes following a io_uring_enter like this:
>
> write1 write2 ... write8 barrier write9 write10
>
> Then we can miss the barrier almost every time.
No problem, just curious if you had a test case, since then I'd add
it to the arsenal.
I've applied this one, thank you.
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-16 14:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-13 3:58 [PATCH 1/2] io_uring: make req from defer and link list not touch async list Zhengyuan Liu
2019-07-13 3:58 ` [PATCH 2/2] io_uring: fix the judging condition in io_sequence_defer Zhengyuan Liu
2019-07-13 21:43 ` Jens Axboe
[not found] ` <5d2be620.1c69fb81.fff62.8eeeSMTPIN_ADDED_BROKEN@mx.google.com>
2019-07-16 14:10 ` Jens Axboe
2019-07-16 14:08 ` [PATCH 1/2] io_uring: make req from defer and link list not touch async list Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).