linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Sargun Dhillon <sargun@sargun.me>
Cc: Vivek Goyal <vgoyal@redhat.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-unionfs@vger.kernel.org, jlayton@kernel.org,
	amir73il@gmail.com, miklos@szeredi.hu, jack@suse.cz,
	neilb@suse.com, viro@zeniv.linux.org.uk, hch@lst.de
Subject: Re: [PATCH 3/3] overlayfs: Report writeback errors on upper
Date: Wed, 23 Dec 2020 20:07:46 +0000	[thread overview]
Message-ID: <20201223200746.GR874@casper.infradead.org> (raw)
In-Reply-To: <20201223192940.GA11012@ircssh-2.c.rugged-nimbus-611.internal>

On Wed, Dec 23, 2020 at 07:29:41PM +0000, Sargun Dhillon wrote:
> On Wed, Dec 23, 2020 at 06:50:44PM +0000, Matthew Wilcox wrote:
> > On Wed, Dec 23, 2020 at 06:20:27PM +0000, Sargun Dhillon wrote:
> > > I fail to see why this is neccessary if you incorporate error reporting into the 
> > > sync_fs callback. Why is this separate from that callback? If you pickup Jeff's
> > > patch that adds the 2nd flag to errseq for "observed", you should be able to
> > > stash the first errseq seen in the ovl_fs struct, and do the check-and-return
> > > in there instead instead of adding this new infrastructure.
> > 
> > You still haven't explained why you want to add the "observed" flag.
> 
> 
> In the overlayfs model, many users may be using the same filesystem (super block)
> for their upperdir. Let's say you have something like this:
> 
> /workdir [Mounted FS]
> /workdir/upperdir1 [overlayfs upperdir]
> /workdir/upperdir2 [overlayfs upperdir]
> /workdir/userscratchspace
> 
> The user needs to be able to do something like:
> sync -f ${overlayfs1}/file
> 
> which in turn will call sync on the the underlying filesystem (the one mounted 
> on /workdir), and can check if the errseq has changed since the overlayfs was
> mounted, and use that to return an error to the user.

OK, but I don't see why the current scheme doesn't work for this.  If
(each instance of) overlayfs samples the errseq at mount time and then
check_and_advances it at sync time, it will see any error that has occurred
since the mount happened (and possibly also an error which occurred before
the mount happened, but hadn't been reported to anybody before).

> If we do not advance the errseq on the upperdir to "mark it as seen", that means 
> future errors will not be reported if the user calls sync -f ${overlayfs1}/file,
> because errseq will not increment the value if the seen bit is unset.
> 
> On the other hand, if we mark it as seen, then if the user calls sync on 
> /workdir/userscratchspace/file, they wont see the error since we just set the 
> SEEN flag.

While we set the SEEN flag, if the file were opened before the error
occurred, we would still report the error because the sequence is higher
than it was when we sampled the error.


  reply	other threads:[~2020-12-23 20:08 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-21 19:50 [RFC PATCH 0/3][v3] vfs, overlayfs: Fix syncfs() to return correct errors Vivek Goyal
2020-12-21 19:50 ` [PATCH 1/3] vfs: Do not ignore return code from s_op->sync_fs Vivek Goyal
2020-12-22  1:23   ` NeilBrown
2020-12-22 15:17     ` Vivek Goyal
2020-12-21 19:50 ` [PATCH 2/3] vfs: Add a super block operation to check for writeback errors Vivek Goyal
2020-12-22 16:19   ` Matthew Wilcox
2020-12-22 16:25     ` Vivek Goyal
2020-12-23 12:44       ` Jeff Layton
2020-12-23 12:48   ` Jeff Layton
2021-01-04 19:41     ` Vivek Goyal
2020-12-21 19:50 ` [PATCH 3/3] overlayfs: Report writeback errors on upper Vivek Goyal
2020-12-22 16:20   ` Matthew Wilcox
2020-12-22 16:29     ` Vivek Goyal
2020-12-22 17:46       ` Matthew Wilcox
2020-12-22 17:55         ` Vivek Goyal
2020-12-23 12:53           ` Jeff Layton
2020-12-23 18:20   ` Sargun Dhillon
2020-12-23 18:50     ` Matthew Wilcox
2020-12-23 19:29       ` Sargun Dhillon
2020-12-23 20:07         ` Matthew Wilcox [this message]
2020-12-23 20:21           ` Sargun Dhillon
2020-12-23 20:44             ` Matthew Wilcox
2020-12-24  9:32               ` Amir Goldstein
2020-12-24 10:12                 ` Sargun Dhillon
2020-12-24 12:13                 ` Matthew Wilcox
2020-12-25  6:50                   ` Amir Goldstein
2020-12-28 13:25                     ` Jeff Layton
2020-12-28 15:51                       ` Amir Goldstein
2021-01-04 15:51                         ` Vivek Goyal
2020-12-28 15:56                       ` Matthew Wilcox
2020-12-28 17:26                         ` Jeff Layton
2020-12-28 19:25                           ` Sargun Dhillon
2020-12-28 19:37                           ` Amir Goldstein
2020-12-28 20:48                             ` Matthew Wilcox
2021-01-02 13:25                               ` Jeff Layton
2021-01-04 16:59                         ` Vivek Goyal
2021-01-04 15:14                 ` Vivek Goyal
2021-01-04 15:22                   ` Amir Goldstein
2021-01-04 15:40                     ` Vivek Goyal
2021-01-04 21:42                       ` Amir Goldstein
2021-01-04 22:44                         ` Vivek Goyal
2021-01-05  7:11                           ` Amir Goldstein
2021-01-05 16:26                             ` Vivek Goyal
2021-01-05 16:57                               ` Amir Goldstein
2020-12-23 19:00     ` Jeff Layton
2021-01-04 20:00     ` Vivek Goyal

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=20201223200746.GR874@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=amir73il@gmail.com \
    --cc=hch@lst.de \
    --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=vgoyal@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /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).