Re: [RFC PATCH 08/11] xfs: have sync_fs op report writeback errors when passed a since pointer

From: Dave Chinner
Date: Mon May 21 2018 - 18:07:18 EST


On Fri, May 18, 2018 at 08:34:12AM -0400, Jeff Layton wrote:
> From: Jeff Layton <jlayton@xxxxxxxxxx>
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/xfs/xfs_super.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 9255de2767b4..7dc847f48f9f 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1092,6 +1092,7 @@ xfs_fs_sync_fs(
> int wait,
> errseq_t *since)
> {
> + int ret;
> struct xfs_mount *mp = XFS_M(sb);
>
> /*
> @@ -1110,7 +1111,13 @@ xfs_fs_sync_fs(
> flush_delayed_work(&mp->m_log->l_work);
> }
> out:
> - return __sync_blockdev(sb->s_bdev, wait);

Where did this come from? XFS doesn't use the underlying blockdev
address space, so this does nothing at all and should not be here.

> + ret = __sync_blockdev(sb->s_bdev, wait);
> + if (since) {
> + int ret2 = errseq_check_and_advance(&sb->s_wb_err, since);
> + if (ret == 0)
> + ret = ret2;
> + }
> + return ret;
> }

So to return errors correctly, xfs_fs_sync_fs() needs to capture
errors from the log force (i.e. metadata errors such as filesystem
shutdowns, journal IO errors, etc), then check for pending data IO
errors. i.e:


STATIC int
xfs_fs_sync_fs(
struct super_block *sb,
int wait)
{
struct xfs_mount *mp = XFS_M(sb);
+ int err;

/*
* Doing anything during the async pass would be counterproductive.
*/
if (!wait)
return 0;

- xfs_log_force(mp, XFS_LOG_SYNC);
+ err = xfs_log_force(mp, XFS_LOG_SYNC);
+ if (err)
+ return err;
+
if (laptop_mode) {
/*
* The disk must be active because we're syncing.
* We schedule log work now (now that the disk is
* active) instead of later (when it might not be).
*/
flush_delayed_work(&mp->m_log->l_work);
}

- return 0
+ return errseq_check_and_advance(&sb->s_wb_err, since);
}

Cheers,

Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx