[RFC PATCH 0/3][v3] vfs, overlayfs: Fix syncfs() to return correct errors

From: Vivek Goyal
Date: Mon Dec 21 2020 - 14:53:31 EST


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@xxxxxxxxxx/
v1:
https://lore.kernel.org/linux-fsdevel/20201216143802.GA10550@xxxxxxxxxx/

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@xxxxxxxxxx/

I posted patches for option B in V2.

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

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