($INBOX_DIR/description missing)
 help / color / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Sargun Dhillon <sargun@sargun.me>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	overlayfs <linux-unionfs@vger.kernel.org>,
	Jeff Layton <jlayton@redhat.com>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [PATCH] overlay: Implement volatile-specific fsync error behaviour
Date: Wed, 2 Dec 2020 12:25:12 +0200
Message-ID: <CAOQ4uxiUTsXEdQsE275qxTh61tZOB+-wqCp6gaNLkOw5ueUJgw@mail.gmail.com> (raw)
In-Reply-To: <20201202092720.41522-1-sargun@sargun.me>

On Wed, Dec 2, 2020 at 11:27 AM Sargun Dhillon <sargun@sargun.me> wrote:
>
> Overlayfs's volatile option allows the user to bypass all forced sync calls
> to the upperdir filesystem. This comes at the cost of safety. We can never
> ensure that the user's data is intact, but we can make a best effort to
> expose whether or not the data is likely to be in a bad state.
>
> We decided[1] that the best way to handle this in the time being is that if
> an overlayfs's upperdir experiences an error after a volatile mount occurs,
> that error will be returned on fsync, fdatasync, sync, and syncfs. This is
> contradictory to the traditional behaviour of VFS which fails the call
> once, and only raises an error if a subsequent fsync error has occured,
> and been raised by the filesystem.
>
> One awkward aspect of the patch is that we have to manually set the
> superblock's errseq_t after the sync_fs callback as opposed to just
> returning an error from syncfs. This is because the call chain looks
> something like this:
>
> sys_syncfs ->
>         sync_filesystem ->
>                 __sync_filesystem ->
>                         /* The return value is ignored here
>                         sb->s_op->sync_fs(sb)
>                         _sync_blockdev
>                 /* Where the VFS fetches the error to raise to userspace */
>                 errseq_check_and_advance
>
> Because of this we call errseq_set every time the sync_fs callback occurs.
>
> [1]: https://lore.kernel.org/linux-fsdevel/36d820394c3e7cd1faa1b28a8135136d5001dadd.camel@redhat.com/T/#u
>
> Signed-off-by: Sargun Dhillon <sargun@sargun.me>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Cc: linux-fsdevel@vger.kernel.org
> Cc: linux-unionfs@vger.kernel.org
> Cc: Jeff Layton <jlayton@redhat.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> ---

Looks safe :-)

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

We should consider sending this to stable, but maybe let's merge first
and let it
run in master for a while before because it is not a clear and immediate danger
and if anyone is using volatile already I hope they read all the
warnings on the box.

Thanks,
Amir.

  reply index

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-02  9:27 Sargun Dhillon
2020-12-02 10:25 ` Amir Goldstein [this message]
2020-12-02 15:07 ` Vivek Goyal
2020-12-02 17:02   ` Jeff Layton
2020-12-02 17:29     ` Vivek Goyal
2020-12-02 18:22       ` Jeff Layton
2020-12-02 18:56         ` Vivek Goyal
2020-12-02 19:03           ` Sargun Dhillon
2020-12-02 19:26           ` Jeff Layton
2020-12-02 21:34             ` Vivek Goyal
2020-12-02 21:52               ` Jeff Layton
2020-12-03 10:42                 ` Sargun Dhillon
2020-12-03 12:06                   ` Jeff Layton
2020-12-03 14:27                   ` Vivek Goyal
2020-12-03 15:20                     ` Jeff Layton
2020-12-03 17:08                       ` Sargun Dhillon
2020-12-03 17:50                         ` Jeff Layton
2020-12-03 20:43                           ` Vivek Goyal
2020-12-03 21:36                             ` Jeff Layton
2020-12-03 22:24                               ` Vivek Goyal
2020-12-03 23:36                                 ` Jeff Layton
2020-12-04  6:45                                   ` Amir Goldstein
2020-12-04 15:00                                     ` Vivek Goyal
2020-12-03 20:31                       ` Vivek Goyal
2020-12-02 18:49       ` Sargun Dhillon
2020-12-02 19:10         ` Jeff Layton
2020-12-03 10:36         ` Amir Goldstein
2020-12-02 17:41   ` Amir Goldstein

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=CAOQ4uxiUTsXEdQsE275qxTh61tZOB+-wqCp6gaNLkOw5ueUJgw@mail.gmail.com \
    --to=amir73il@gmail.com \
    --cc=jlayton@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=sargun@sargun.me \
    --cc=vgoyal@redhat.com \
    /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

($INBOX_DIR/description missing)

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-unionfs/0 linux-unionfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-unionfs linux-unionfs/ https://lore.kernel.org/linux-unionfs \
		linux-unionfs@vger.kernel.org
	public-inbox-index linux-unionfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-unionfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git