Date:   Mon, 21 Dec 2020 14:50:52 -0500
From:   Vivek Goyal <vgoyal@...hat.com>
To:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-unionfs@...r.kernel.org
Cc:     jlayton@...nel.org, vgoyal@...hat.com, amir73il@...il.com,
        sargun@...gun.me, miklos@...redi.hu, willy@...radead.org,
        jack@...e.cz, neilb@...e.com, viro@...iv.linux.org.uk, hch@....de
Subject: [RFC PATCH 0/3][v3] vfs, overlayfs: Fix syncfs() to return correct errors

Hi,

This is v3 of patches which try to fix syncfs() error handling issues
w.r.t overlayfs and other filesystems.

Previous version of patches are here.
v2: 
https://lore.kernel.org/linux-fsdevel/20201216233149.39025-1-vgoyal@redhat.com/
v1:
https://lore.kernel.org/linux-fsdevel/20201216143802.GA10550@redhat.com/

This series basically is trying to fix two problems.

- First problem is that we ignore error code returned by ->sync_fs().
  overlayfs file system can return error and there are other file
  systems which can return error in certain cases. So to fix this issue,
  first patch captures the return code from ->sync_fs and returns to
  user space.

- Second problem is that current syncfs(), writeback error detection
  logic does not work for overlayfs. current logic relies on all
  sb->s_wb_err being update when errors occur but that's not true for
  overlayfs. Real errors happen on underlyig filessytem and overlayfs
  has no clue about these. To fix this issue, it has been proposed
  that for filesystems like overlayfs, this check should be moved into
  filesystem and then filesystem can check for error w.r.t upper super
  block.

  There seem to be multiple ways of how this can be done.

  A. Add a "struct file" argument to ->sync_fs() and modify all helpers.
  B. Add a separate file operation say "f_op->syncfs()" and call that
     in syncfs().
  C. Add a separate super block operation to check and advance errors.

Option A involves lot of changes all across the code. Also it is little
problematic in the sense that for filesystems having a block device,
looks like we want to check for errors after ___sync_blockdev() has
returned. But ->sync_fs() is called before that. That means
__sync_blockdev() will have to be pushed in side filesystem code as
well. Jeff Layton gave something like this a try here.

https://lore.kernel.org/linux-fsdevel/20180518123415.28181-1-jlayton@kernel.org/

I posted patches for option B in V2. 

https://lore.kernel.org/linux-fsdevel/20201216233149.39025-1-vgoyal@redhat.com/

Now this is V3 of patches which implements option C. I think this is
simplest in terms of implementation atleast.

These patches are only compile tested. Will do more testing once I get
a sense which option has a chance to fly.

I think patch 1 should be applied irrespective of what option we end
up choosing for fixing the writeback error issue.

Thanks
Vivek

Vivek Goyal (3):
  vfs: Do not ignore return code from s_op->sync_fs
  vfs: Add a super block operation to check for writeback errors
  overlayfs: Report writeback errors on upper

 fs/overlayfs/file.c      |  1 +
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/readdir.c   |  1 +
 fs/overlayfs/super.c     | 23 +++++++++++++++++++++++
 fs/overlayfs/util.c      | 13 +++++++++++++
 fs/sync.c                | 13 ++++++++++---
 include/linux/fs.h       |  1 +
 7 files changed, 50 insertions(+), 3 deletions(-)

-- 
2.25.4

Powered by blists - more mailing lists