linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: viro@ZenIV.linux.org.uk, willy@infradead.org, andres@anarazel.de
Subject: [RFC PATCH 05/11] fs: track per-sb writeback errors and report them to syncfs
Date: Fri, 18 May 2018 08:34:09 -0400	[thread overview]
Message-ID: <20180518123415.28181-6-jlayton@kernel.org> (raw)
In-Reply-To: <20180518123415.28181-1-jlayton@kernel.org>

From: Jeff Layton <jlayton@redhat.com>

Usually we suggest that applications call fsync when they want to
ensure that all data written to the file has made it to the backing
store, but that can be inefficient when there are a lot of open
files.

Calling syncfs on the filesystem may be more efficient, but the error
reporting doesn't currently work the way most people expect. If a single
inode on a filesystem reports a writeback error, syncfs won't return an
error. syncfs only returns an error if __sync_blockdev fails.

It would be better if it reported an error if there were any writeback
failures. Then applications could call syncfs to see if there are any
errors on any open files, and could then call fsync on all of the other
descriptors to figure out which one failed.

This patch implements a suggestion from Willy to remedy this. It adds a
new errseq_t to struct super_block, and has mapping_set_error also
record writeback errors there.

To report errors recorded there, we also need to keep an errseq_t for in
struct file to act as a cursor, but growing struct file for this purpose
is undesirable. We could just reuse f_wb_err, but someone could mix
calls to fsync and syncfs and that would break things.

As an alternative, this patch only has syncfs report errors recorded
in s_wb_err when the file has been opened with O_PATH. Any file opened
with O_PATH will not have its fsync field defined in its
file_operations so we can be sure that nothing else will be using its
f_wb_err field.

Note that calling syncfs on an O_PATH descriptor today will return
-EBADF, so this scheme gives userland a way to tell whether this
mechanism will work at runtime.

Cc: Andres Freund <andres@anarazel.de>
Cc: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/open.c               | 6 +++---
 fs/sync.c               | 9 +++++++--
 include/linux/fs.h      | 3 +++
 include/linux/pagemap.h | 5 ++++-
 4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index c5ee7cd60424..3e8c7b16abb8 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -739,15 +739,15 @@ static int do_dentry_open(struct file *f,
 	f->f_inode = inode;
 	f->f_mapping = inode->i_mapping;
 
-	/* Ensure that we skip any errors that predate opening of the file */
-	f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
-
 	if (unlikely(f->f_flags & O_PATH)) {
 		f->f_mode = FMODE_PATH;
 		f->f_op = &empty_fops;
+		f->f_wb_err = errseq_sample(&f->f_path.dentry->d_sb->s_wb_err);
 		goto done;
 	}
 
+	f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
+
 	if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
 		error = get_write_access(inode);
 		if (unlikely(error))
diff --git a/fs/sync.c b/fs/sync.c
index c876fa414cae..f2400d13a65f 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -162,16 +162,21 @@ void emergency_sync(void)
  */
 SYSCALL_DEFINE1(syncfs, int, fd)
 {
-	struct fd f = fdget(fd);
+	struct fd f = fdget_raw(fd);
 	struct super_block *sb;
+	errseq_t *wberr = NULL;
 	int ret;
 
 	if (!f.file)
 		return -EBADF;
+
 	sb = f.file->f_path.dentry->d_sb;
 
+	if (f.file->f_flags & O_PATH)
+		wberr = &f.file->f_wb_err;
+
 	down_read(&sb->s_umount);
-	ret = sync_filesystem(sb, NULL);
+	ret = sync_filesystem(sb, wberr);
 	up_read(&sb->s_umount);
 
 	fdput(f);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d40fe9ead5fb..fecd29325f36 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1416,6 +1416,9 @@ struct super_block {
 	/* Being remounted read-only */
 	int s_readonly_remount;
 
+	/* per-sb errseq_t for reporting writeback errors via syncfs */
+	errseq_t s_wb_err;
+
 	/* AIO completions deferred from interrupt context */
 	struct workqueue_struct *s_dio_done_wq;
 	struct hlist_head s_pins;
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index b1bd2186e6d2..2de87c5a2718 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -51,7 +51,10 @@ static inline void mapping_set_error(struct address_space *mapping, int error)
 		return;
 
 	/* Record in wb_err for checkers using errseq_t based tracking */
-	filemap_set_wb_err(mapping, error);
+	__filemap_set_wb_err(mapping, error);
+
+	/* Record it in superblock */
+	errseq_set(&mapping->host->i_sb->s_wb_err, error);
 
 	/* Record it in flags for now, for legacy callers */
 	if (error == -ENOSPC)
-- 
2.17.0

  parent reply	other threads:[~2018-05-18 12:34 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-18 12:34 [RFC PATCH 00/11] vfs: have syncfs return an error when inode writeback fails Jeff Layton
2018-05-18 12:34 ` [RFC PATCH 01/11] vfs: push __sync_blockdev calls down into sync_fs routines Jeff Layton
2018-05-18 15:56   ` Christoph Hellwig
2018-05-18 17:56     ` Jeff Layton
2018-05-18 12:34 ` [RFC PATCH 02/11] vfs: add a new errseq_t pointer to sync_fs prototype Jeff Layton
2018-05-18 12:34 ` [RFC PATCH 03/11] vfs: add an errseq_t pointer to sync_filesystem Jeff Layton
2018-05-18 12:34 ` [RFC PATCH 04/11] vfs: add errseq_t pointer to __sync_filesystem Jeff Layton
2018-05-18 12:34 ` Jeff Layton [this message]
2018-05-18 12:34 ` [RFC PATCH 06/11] buffer: record blockdev write errors in super_block that backs them Jeff Layton
2018-05-18 12:34 ` [RFC PATCH 07/11] ext4: have sync_fs op report writeback errors when passed a since pointer Jeff Layton
2018-05-18 15:22   ` Matthew Wilcox
2018-05-18 16:50     ` Jeff Layton
2018-05-18 12:34 ` [RFC PATCH 08/11] xfs: " Jeff Layton
2018-05-21 23:01   ` Dave Chinner
2018-05-21 23:23     ` Jeff Layton
2018-05-18 12:34 ` [RFC PATCH 09/11] btrfs: " Jeff Layton
2018-05-18 12:34 ` [RFC PATCH 10/11] ext2: " Jeff Layton
2018-05-18 12:34 ` [RFC PATCH 11/11] vfs: have call_sync_fs " Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180518123415.28181-6-jlayton@kernel.org \
    --to=jlayton@kernel.org \
    --cc=andres@anarazel.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).