Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Vivek Goyal <vgoyal@redhat.com>
To:     linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-unionfs@vger.kernel.org
Cc:     jlayton@kernel.org, vgoyal@redhat.com, amir73il@gmail.com,
        sargun@sargun.me, miklos@szeredi.hu, willy@infradead.org,
        jack@suse.cz, neilb@suse.com, viro@zeniv.linux.org.uk
Subject: [PATCH 1/3] vfs: add new f_op->syncfs vector
Date:   Wed, 16 Dec 2020 18:31:47 -0500
Message-Id: <20201216233149.39025-2-vgoyal@redhat.com>
In-Reply-To: <20201216233149.39025-1-vgoyal@redhat.com>
References: <20201216233149.39025-1-vgoyal@redhat.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Current implementation of __sync_filesystem() ignores the return code
from ->sync_fs(). I am not sure why that's the case. There must have
been some historical reason for this.

Ignoring ->sync_fs() return code is problematic for overlayfs where
it can return error if sync_filesystem() on upper super block failed.
That error will simply be lost and sycnfs(overlay_fd), will get
success (despite the fact it failed).

If we modify existing implementation, there is a concern that it will
lead to user space visible behavior changes and break things. So
instead implement a new file_operations->syncfs() call which will
be called in syncfs() syscall path. Return code from this new
call will be captured. And all the writeback error detection
logic can go in there as well. Only filesystems which implement
this call get affected by this change. Others continue to fallback
to existing mechanism.

To be clear, I mean something like this (draft, untested) patch. You'd
also need to add a new ->syncfs op for overlayfs, and that could just do
a check_and_advance against the upper layer sb's errseq_t after calling
sync_filesystem.

Vivek, fixed couple of minor compile errors in original patch.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/sync.c          | 29 ++++++++++++++++++++---------
 include/linux/fs.h |  1 +
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/fs/sync.c b/fs/sync.c
index 1373a610dc78..06caa9758d93 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -155,27 +155,38 @@ void emergency_sync(void)
 	}
 }
 
+static int generic_syncfs(struct file *file)
+{
+	int ret, ret2;
+	struct super_block *sb = file->f_path.dentry->d_sb;
+
+	down_read(&sb->s_umount);
+	ret = sync_filesystem(sb);
+	up_read(&sb->s_umount);
+
+	ret2 = errseq_check_and_advance(&sb->s_wb_err, &file->f_sb_err);
+
+	return ret ? ret : ret2;
+}
+
 /*
  * sync a single super
  */
 SYSCALL_DEFINE1(syncfs, int, fd)
 {
 	struct fd f = fdget(fd);
-	struct super_block *sb;
-	int ret, ret2;
+	int ret;
 
 	if (!f.file)
 		return -EBADF;
-	sb = f.file->f_path.dentry->d_sb;
-
-	down_read(&sb->s_umount);
-	ret = sync_filesystem(sb);
-	up_read(&sb->s_umount);
 
-	ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
+	if (f.file->f_op->syncfs)
+		ret = f.file->f_op->syncfs(f.file);
+	else
+		ret = generic_syncfs(f.file);
 
 	fdput(f);
-	return ret ? ret : ret2;
+	return ret;
 }
 
 /**
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8667d0cdc71e..6710469b7e33 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1859,6 +1859,7 @@ struct file_operations {
 				   struct file *file_out, loff_t pos_out,
 				   loff_t len, unsigned int remap_flags);
 	int (*fadvise)(struct file *, loff_t, loff_t, int);
+	int (*syncfs)(struct file *);
 } __randomize_layout;
 
 struct inode_operations {
-- 
2.25.4