On 4/16/24 5:38 AM, Jens Axboe wrote: > On 4/16/24 4:00 AM, Ming Lei wrote: >> On Tue, Apr 16, 2024 at 10:26:16AM +0800, Changhui Zhong wrote: >>>>> >>>>> I can't reproduce this here, fwiw. Ming, something you've seen? >>>> >>>> I just test against the latest for-next/block(-rc4 based), and still can't >>>> reproduce it. There was such RH internal report before, and maybe not >>>> ublk related. >>>> >>>> Changhui, if the issue can be reproduced in your machine, care to share >>>> your machine for me to investigate a bit? >>>> >>>> Thanks, >>>> Ming >>>> >>> >>> I still can reproduce this issue on my machine? >>> and I shared machine to Ming?he can do more investigation for this issue? >>> >>> [ 1244.207092] running generic/006 >>> [ 1246.456896] blk_print_req_error: 77 callbacks suppressed >>> [ 1246.456907] I/O error, dev ublkb1, sector 2395864 op 0x1:(WRITE) >>> flags 0x8800 phys_seg 1 prio class 0 >> >> The failure is actually triggered in recovering qcow2 target in generic/005, >> since ublkb0 isn't removed successfully in generic/005. >> >> git-bisect shows that the 1st bad commit is cca6571381a0 ("io_uring/rw: >> cleanup retry path"). >> >> And not see any issue in uring command side, so the trouble seems >> in normal io_uring rw side over XFS file, and not related with block >> device. > > Indeed, I can reproduce it on XFS as well. I'll take a look. Can you try this one? diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c index 3c9087f37c43..c67ae6e36c4f 100644 --- a/io_uring/io_uring.c +++ b/io_uring/io_uring.c @@ -527,6 +527,19 @@ static void io_queue_iowq(struct io_kiocb *req) io_queue_linked_timeout(link); } +static void io_tw_requeue_iowq(struct io_kiocb *req, struct io_tw_state *ts) +{ + req->flags &= ~REQ_F_REISSUE; + io_queue_iowq(req); +} + +void io_tw_queue_iowq(struct io_kiocb *req) +{ + req->flags |= REQ_F_REISSUE | REQ_F_BL_NO_RECYCLE; + req->io_task_work.func = io_tw_requeue_iowq; + io_req_task_work_add(req); +} + static __cold void io_queue_deferred(struct io_ring_ctx *ctx) { while (!list_empty(&ctx->defer_list)) { diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h index 624ca9076a50..b83a719c5443 100644 --- a/io_uring/io_uring.h +++ b/io_uring/io_uring.h @@ -75,6 +75,7 @@ struct file *io_file_get_fixed(struct io_kiocb *req, int fd, void __io_req_task_work_add(struct io_kiocb *req, unsigned flags); bool io_alloc_async_data(struct io_kiocb *req); void io_req_task_queue(struct io_kiocb *req); +void io_tw_queue_iowq(struct io_kiocb *req); void io_req_task_complete(struct io_kiocb *req, struct io_tw_state *ts); void io_req_task_queue_fail(struct io_kiocb *req, int ret); void io_req_task_submit(struct io_kiocb *req, struct io_tw_state *ts); diff --git a/io_uring/rw.c b/io_uring/rw.c index 3134a6ece1be..4fed829fe97c 100644 --- a/io_uring/rw.c +++ b/io_uring/rw.c @@ -455,7 +455,7 @@ static bool __io_complete_rw_common(struct io_kiocb *req, long res) * current cycle. */ io_req_io_end(req); - req->flags |= REQ_F_REISSUE | REQ_F_BL_NO_RECYCLE; + io_tw_queue_iowq(req); return true; } req_set_fail(req); @@ -521,7 +521,7 @@ static void io_complete_rw_iopoll(struct kiocb *kiocb, long res) io_req_end_write(req); if (unlikely(res != req->cqe.res)) { if (res == -EAGAIN && io_rw_should_reissue(req)) { - req->flags |= REQ_F_REISSUE | REQ_F_BL_NO_RECYCLE; + io_tw_queue_iowq(req); return; } req->cqe.res = res; @@ -839,7 +839,8 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags) ret = io_iter_do_read(rw, &io->iter); if (ret == -EAGAIN || (req->flags & REQ_F_REISSUE)) { - req->flags &= ~REQ_F_REISSUE; + if (req->flags & REQ_F_REISSUE) + return IOU_ISSUE_SKIP_COMPLETE; /* If we can poll, just do that. */ if (io_file_can_poll(req)) return -EAGAIN; @@ -1034,10 +1035,8 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags) else ret2 = -EINVAL; - if (req->flags & REQ_F_REISSUE) { - req->flags &= ~REQ_F_REISSUE; - ret2 = -EAGAIN; - } + if (req->flags & REQ_F_REISSUE) + return IOU_ISSUE_SKIP_COMPLETE; /* * Raw bdev writes will return -EOPNOTSUPP for IOCB_NOWAIT. Just -- Jens Axboe