linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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 3/3] overlayfs: Check writeback errors w.r.t upper in ->syncfs()
Date: Wed, 16 Dec 2020 18:31:49 -0500	[thread overview]
Message-ID: <20201216233149.39025-4-vgoyal@redhat.com> (raw)
In-Reply-To: <20201216233149.39025-1-vgoyal@redhat.com>

Check for writeback error on overlay super block w.r.t "struct file"
passed in ->syncfs().

As of now real error happens on upper sb. So this patch first propagates
error from upper sb to overlay sb and then checks error w.r.t struct
file passed in.

Jeff, I know you prefer that I should rather file upper file and check
error directly on on upper sb w.r.t this real upper file.  While I was
implementing that I thought what if file is on lower (and has not been
copied up yet). In that case shall we not check writeback errors and
return back to user space? That does not sound right though because,
we are not checking for writeback errors on this file. Rather we
are checking for any error on superblock. Upper might have an error
and we should report it to user even if file in question is a lower
file. And that's why I fell back to this approach. But I am open to
change it if there are issues in this method.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/ovl_entry.h |  2 ++
 fs/overlayfs/super.c     | 15 ++++++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 1b5a2094df8e..a08fd719ee7b 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -79,6 +79,8 @@ struct ovl_fs {
 	atomic_long_t last_ino;
 	/* Whiteout dentry cache */
 	struct dentry *whiteout;
+	/* Protects multiple sb->s_wb_err update from upper_sb . */
+	spinlock_t errseq_lock;
 };
 
 static inline struct vfsmount *ovl_upper_mnt(struct ovl_fs *ofs)
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index b4d92e6fa5ce..e7bc4492205e 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -291,7 +291,7 @@ int ovl_syncfs(struct file *file)
 	struct super_block *sb = file->f_path.dentry->d_sb;
 	struct ovl_fs *ofs = sb->s_fs_info;
 	struct super_block *upper_sb;
-	int ret;
+	int ret, ret2;
 
 	ret = 0;
 	down_read(&sb->s_umount);
@@ -310,10 +310,18 @@ int ovl_syncfs(struct file *file)
 	ret = sync_filesystem(upper_sb);
 	up_read(&upper_sb->s_umount);
 
+	/* Update overlay sb->s_wb_err */
+	if (errseq_check(&upper_sb->s_wb_err, sb->s_wb_err)) {
+		/* Upper sb has errors since last time */
+		spin_lock(&ofs->errseq_lock);
+		errseq_check_and_advance(&upper_sb->s_wb_err, &sb->s_wb_err);
+		spin_unlock(&ofs->errseq_lock);
+	}
 
+	ret2 = errseq_check_and_advance(&sb->s_wb_err, &file->f_sb_err);
 out:
 	up_read(&sb->s_umount);
-	return ret;
+	return ret ? ret : ret2;
 }
 
 /**
@@ -1903,6 +1911,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	if (!cred)
 		goto out_err;
 
+	spin_lock_init(&ofs->errseq_lock);
 	/* Is there a reason anyone would want not to share whiteouts? */
 	ofs->share_whiteout = true;
 
@@ -1975,7 +1984,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 
 		sb->s_stack_depth = ovl_upper_mnt(ofs)->mnt_sb->s_stack_depth;
 		sb->s_time_gran = ovl_upper_mnt(ofs)->mnt_sb->s_time_gran;
-
+		sb->s_wb_err = errseq_sample(&ovl_upper_mnt(ofs)->mnt_sb->s_wb_err);
 	}
 	oe = ovl_get_lowerstack(sb, splitlower, numlower, ofs, layers);
 	err = PTR_ERR(oe);
-- 
2.25.4


  parent reply	other threads:[~2020-12-16 23:34 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16 23:31 [RFC PATCH 0/3] vfs, overlayfs: Fix syncfs() to return error Vivek Goyal
2020-12-16 23:31 ` [PATCH 1/3] vfs: add new f_op->syncfs vector Vivek Goyal
2020-12-17  0:49   ` Al Viro
2020-12-17  9:57     ` Jan Kara
2020-12-17 16:15       ` Vivek Goyal
2020-12-17 15:00     ` Vivek Goyal
2020-12-17 19:49     ` Jeff Layton
2020-12-16 23:31 ` [PATCH 2/3] overlayfs: Implement f_op->syncfs() call Vivek Goyal
2020-12-16 23:31 ` Vivek Goyal [this message]
2020-12-17 20:08   ` [PATCH 3/3] overlayfs: Check writeback errors w.r.t upper in ->syncfs() Jeffrey Layton
2020-12-18 14:44     ` Vivek Goyal
2020-12-18 15:02       ` Jeff Layton
2020-12-18 16:28         ` Vivek Goyal
2020-12-18 16:55           ` Jeffrey Layton
2020-12-18 20:25             ` NeilBrown

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=20201216233149.39025-4-vgoyal@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=amir73il@gmail.com \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=neilb@suse.com \
    --cc=sargun@sargun.me \
    --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).