vfs: track per-sb writeback errors and report them to syncfs
authorJeff Layton <jlayton@redhat.com>
Tue, 2 Jun 2020 04:45:36 +0000 (21:45 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Tue, 2 Jun 2020 17:59:05 +0000 (10:59 -0700)
Patch series "vfs: have syncfs() return error when there are writeback
errors", v6.

Currently, syncfs does not return errors when one of the inodes fails to
be written back.  It will return errors based on the legacy AS_EIO and
AS_ENOSPC flags when syncing out the block device fails, but that's not
particularly helpful for filesystems that aren't backed by a blockdev.
It's also possible for a stray sync to lose those errors.

The basic idea in this set is to track writeback errors at the
superblock level, so that we can quickly and easily check whether
something bad happened without having to fsync each file individually.
syncfs is then changed to reliably report writeback errors after they
occur, much in the same fashion as fsync does now.

This patch (of 2):

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 can be more efficient in some
situations, 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 necessarily return an error.  syncfs only returns an
error if __sync_blockdev fails, and on some filesystems that's a no-op.

It would be better if syncfs 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 adds a new errseq_t to struct super_block, and has
mapping_set_error also record writeback errors there.

To report those errors, we also need to keep an errseq_t in struct file
to act as a cursor.  This patch adds a dedicated field for that purpose,
which slots nicely into 4 bytes of padding at the end of struct file on
x86_64.

An earlier version of this patch used an O_PATH file descriptor to cue
the kernel that the open file should track the superblock error and not
the inode's writeback error.

I think that API is just too weird though.  This is simpler and should
make syncfs error reporting "just work" even if someone is multiplexing
fsync and syncfs on the same fds.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Jan Kara <jack@suse.cz>
Cc: Andres Freund <andres@anarazel.de>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dave Chinner <david@fromorbit.com>
Cc: David Howells <dhowells@redhat.com>
Link: http://lkml.kernel.org/r/20200428135155.19223-1-jlayton@kernel.org
Link: http://lkml.kernel.org/r/20200428135155.19223-2-jlayton@kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
drivers/dax/device.c
fs/file_table.c
fs/open.c
fs/sync.c
include/linux/fs.h
include/linux/pagemap.h

index 1af823b2fe6bdabea264d17cdbb49670edf61717..4c0af2eb7e19648aecf4489eebd08a60d7d2205d 100644 (file)
@@ -377,6 +377,7 @@ static int dax_open(struct inode *inode, struct file *filp)
        inode->i_mapping->a_ops = &dev_dax_aops;
        filp->f_mapping = inode->i_mapping;
        filp->f_wb_err = filemap_sample_wb_err(filp->f_mapping);
+       filp->f_sb_err = file_sample_sb_err(filp);
        filp->private_data = dev_dax;
        inode->i_flags = S_DAX;
 
index 30d55c9a1744a6244d50e8ae3497a1563e7e14a1..676e620948d24291b894b93f9edce8280bc21cd1 100644 (file)
@@ -198,6 +198,7 @@ static struct file *alloc_file(const struct path *path, int flags,
        file->f_inode = path->dentry->d_inode;
        file->f_mapping = path->dentry->d_inode->i_mapping;
        file->f_wb_err = filemap_sample_wb_err(file->f_mapping);
+       file->f_sb_err = file_sample_sb_err(file);
        if ((file->f_mode & FMODE_READ) &&
             likely(fop->read || fop->read_iter))
                file->f_mode |= FMODE_CAN_READ;
index 719b320ede52bb1482afd28fc65b8282e9d6fb94..d9467a8a7f6a9f31c85b893de4f9d785fe0a4760 100644 (file)
--- a/fs/open.c
+++ b/fs/open.c
@@ -743,9 +743,8 @@ static int do_dentry_open(struct file *f,
        path_get(&f->f_path);
        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);
+       f->f_sb_err = file_sample_sb_err(f);
 
        if (unlikely(f->f_flags & O_PATH)) {
                f->f_mode = FMODE_PATH | FMODE_OPENED;
index 4d1ff010bc5afcd6acee1e4dbd736e8bfbd9081a..c6f6f5be5682a97714d676eea9dae9507ae45d21 100644 (file)
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -161,7 +161,7 @@ SYSCALL_DEFINE1(syncfs, int, fd)
 {
        struct fd f = fdget(fd);
        struct super_block *sb;
-       int ret;
+       int ret, ret2;
 
        if (!f.file)
                return -EBADF;
@@ -171,8 +171,10 @@ SYSCALL_DEFINE1(syncfs, int, fd)
        ret = sync_filesystem(sb);
        up_read(&sb->s_umount);
 
+       ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
+
        fdput(f);
-       return ret;
+       return ret ? ret : ret2;
 }
 
 /**
index 45cc10cdf6ddd760aeadc92d255f65b132ed67cc..f2fb5b7406b9f4bbc324bffc22365261f1203c53 100644 (file)
@@ -976,6 +976,7 @@ struct file {
 #endif /* #ifdef CONFIG_EPOLL */
        struct address_space    *f_mapping;
        errseq_t                f_wb_err;
+       errseq_t                f_sb_err; /* for syncfs */
 } __randomize_layout
   __attribute__((aligned(4))); /* lest something weird decides that 2 is OK */
 
@@ -1520,6 +1521,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;
@@ -2827,6 +2831,18 @@ static inline errseq_t filemap_sample_wb_err(struct address_space *mapping)
        return errseq_sample(&mapping->wb_err);
 }
 
+/**
+ * file_sample_sb_err - sample the current errseq_t to test for later errors
+ * @mapping: mapping to be sampled
+ *
+ * Grab the most current superblock-level errseq_t value for the given
+ * struct file.
+ */
+static inline errseq_t file_sample_sb_err(struct file *file)
+{
+       return errseq_sample(&file->f_path.dentry->d_sb->s_wb_err);
+}
+
 static inline int filemap_nr_thps(struct address_space *mapping)
 {
 #ifdef CONFIG_READ_ONLY_THP_FOR_FS
index a8f7bd8ea1c62983088a7811595a94a66b15c5be..d4409b13747e8c7ea8a62d85476fde58603c8e98 100644 (file)
@@ -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)