Re: [regression, v6.0-rc0, io-uring?] filesystem freeze hangs on sb_wait_write()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/11/22 03:01, Jens Axboe wrote:
On 10/10/22 7:10 PM, Pavel Begunkov wrote:
On 10/11/22 01:40, Dave Chinner wrote:
[...]
I note that there are changes to the the io_uring IO path and write
IO end accounting in the io_uring stack that was merged, and there
was no doubt about the success/failure of the reproducer at each
step. Hence I think the bisect is good, and the problem is someone
in the io-uring changes.

Jens, over to you.

The reproducer - generic/068 - is 100% reliable here, io_uring is
being exercised by fsstress in the background whilst the filesystem
is being frozen and thawed repeatedly. Some path in the io-uring
code has an unbalanced sb_start_write()/sb_end_write() pair by the
look of it....

A quick guess, it's probably

b000145e99078 ("io_uring/rw: defer fsnotify calls to task context")

 From a quick look, it removes  kiocb_end_write() -> sb_end_write()
from kiocb_done(), which is a kind of buffered rw completion path.

Yeah, I'll take a look.
Didn't get the original email, only Pavel's reply?

Forwarded.

Not tested, but should be sth like below. Apart of obvious cases
like __io_complete_rw_common() we should also keep in mind
when we don't complete the request but ask for reissue with
REQ_F_REISSUE, that's for the first hunk


diff --git a/io_uring/rw.c b/io_uring/rw.c
index a25cd44cd415..f991aa78803e 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -239,6 +239,18 @@ static bool __io_complete_rw_common(struct io_kiocb *req, long res)
 	if (unlikely(res != req->cqe.res)) {
 		if ((res == -EAGAIN || res == -EOPNOTSUPP) &&
 		    io_rw_should_reissue(req)) {
+			struct io_rw *io = io_kiocb_to_cmd(req, struct io_rw);
+
+			/*
+			 * Need to do it for each rw retry, do it here instead
+			 * of handling it together with REQ_F_REISSUE
+			 */
+			if (io->kiocb.ki_flags & IOCB_WRITE) {
+				kiocb_end_write(req);
+				fsnotify_modify(req->file);
+			} else {
+				fsnotify_access(req->file);
+			}
 			req->flags |= REQ_F_REISSUE | REQ_F_PARTIAL_IO;
 			return true;
 		}
@@ -317,6 +329,12 @@ static int kiocb_done(struct io_kiocb *req, ssize_t ret,
 		req->file->f_pos = rw->kiocb.ki_pos;
 	if (ret >= 0 && (rw->kiocb.ki_complete == io_complete_rw)) {
 		if (!__io_complete_rw_common(req, ret)) {
+			if (rw->kiocb.ki_flags & IOCB_WRITE) {
+				kiocb_end_write(req);
+				fsnotify_modify(req->file);
+			} else {
+				fsnotify_access(req->file);
+			}
 			io_req_set_res(req, final_ret,
 				       io_put_kbuf(req, issue_flags));
 			return IOU_OK;




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux