On Mon, Feb 24, 2020 at 04:10:53PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Teach libxfs_umount to flush all dirty buffers when unmounting the
> filesystem, to log write verifier errors and IO errors, and to return an
> error code when things go wrong. Subsequent patches will teach critical
> utilities to exit with EXIT_FAILURE when this happens.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
Looks reasonable to me:
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> include/xfs_mount.h | 2 +
> libxfs/init.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++--
> libxfs/libxfs_io.h | 7 ++++
> libxfs/rdwr.c | 38 ++++++++++++++++++--
> 4 files changed, 135 insertions(+), 8 deletions(-)
>
>
> diff --git a/include/xfs_mount.h b/include/xfs_mount.h
> index 29b3cc1b..7bd23fbb 100644
> --- a/include/xfs_mount.h
> +++ b/include/xfs_mount.h
> @@ -184,7 +184,7 @@ xfs_perag_resv(
>
> extern xfs_mount_t *libxfs_mount (xfs_mount_t *, xfs_sb_t *,
> dev_t, dev_t, dev_t, int);
> -extern void libxfs_umount (xfs_mount_t *);
> +int libxfs_umount(struct xfs_mount *mp);
> extern void libxfs_rtmount_destroy (xfs_mount_t *);
>
> #endif /* __XFS_MOUNT_H__ */
> diff --git a/libxfs/init.c b/libxfs/init.c
> index a0d4b7f4..d4804ead 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -569,6 +569,8 @@ libxfs_buftarg_alloc(
> }
> btp->bt_mount = mp;
> btp->dev = dev;
> + btp->flags = 0;
> +
> return btp;
> }
>
> @@ -791,17 +793,104 @@ libxfs_rtmount_destroy(xfs_mount_t *mp)
> mp->m_rsumip = mp->m_rbmip = NULL;
> }
>
> +/* Flush a device and report on writes that didn't make it to stable storage. */
> +static inline int
> +libxfs_flush_buftarg(
> + struct xfs_buftarg *btp,
> + const char *buftarg_descr)
> +{
> + int error = 0;
> + int err2;
> +
> + /*
> + * Write verifier failures are evidence of a buggy program. Make sure
> + * that this state is always reported to the caller.
> + */
> + if (btp->flags & XFS_BUFTARG_CORRUPT_WRITE) {
> + fprintf(stderr,
> +_("%s: Refusing to write a corrupt buffer to the %s!\n"),
> + progname, buftarg_descr);
> + error = -EFSCORRUPTED;
> + }
> +
> + if (btp->flags & XFS_BUFTARG_LOST_WRITE) {
> + fprintf(stderr,
> +_("%s: Lost a write to the %s!\n"),
> + progname, buftarg_descr);
> + if (!error)
> + error = -EIO;
> + }
> +
> + err2 = libxfs_blkdev_issue_flush(btp);
> + if (err2) {
> + fprintf(stderr,
> +_("%s: Flushing the %s failed, err=%d!\n"),
> + progname, buftarg_descr, -err2);
> + }
> + if (!error)
> + error = err2;
> +
> + return error;
> +}
> +
> +/*
> + * Flush all dirty buffers to stable storage and report on writes that didn't
> + * make it to stable storage.
> + */
> +static int
> +libxfs_flush_mount(
> + struct xfs_mount *mp)
> +{
> + int error = 0;
> + int err2;
> +
> + /*
> + * Purge the buffer cache to write all dirty buffers to disk and free
> + * all incore buffers. Buffers that fail write verification will cause
> + * the CORRUPT_WRITE flag to be set in the buftarg. Buffers that
> + * cannot be written will cause the LOST_WRITE flag to be set in the
> + * buftarg.
> + */
> + libxfs_bcache_purge();
> +
> + /* Flush all kernel and disk write caches, and report failures. */
> + if (mp->m_ddev_targp) {
> + err2 = libxfs_flush_buftarg(mp->m_ddev_targp, _("data device"));
> + if (!error)
> + error = err2;
> + }
> +
> + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp) {
> + err2 = libxfs_flush_buftarg(mp->m_logdev_targp,
> + _("log device"));
> + if (!error)
> + error = err2;
> + }
> +
> + if (mp->m_rtdev_targp) {
> + err2 = libxfs_flush_buftarg(mp->m_rtdev_targp,
> + _("realtime device"));
> + if (!error)
> + error = err2;
> + }
> +
> + return error;
> +}
> +
> /*
> * Release any resource obtained during a mount.
> */
> -void
> -libxfs_umount(xfs_mount_t *mp)
> +int
> +libxfs_umount(
> + struct xfs_mount *mp)
> {
> struct xfs_perag *pag;
> int agno;
> + int error;
>
> libxfs_rtmount_destroy(mp);
> - libxfs_bcache_purge();
> +
> + error = libxfs_flush_mount(mp);
>
> for (agno = 0; agno < mp->m_maxagi; agno++) {
> pag = radix_tree_delete(&mp->m_perag_tree, agno);
> @@ -816,6 +905,7 @@ libxfs_umount(xfs_mount_t *mp)
> kmem_free(mp->m_logdev_targp);
> kmem_free(mp->m_ddev_targp);
>
> + return error;
> }
>
> /*
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 579df52b..6bb75a67 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -23,10 +23,17 @@ struct xfs_perag;
> struct xfs_buftarg {
> struct xfs_mount *bt_mount;
> dev_t dev;
> + unsigned int flags;
> };
>
> +/* We purged a dirty buffer and lost a write. */
> +#define XFS_BUFTARG_LOST_WRITE (1 << 0)
> +/* A dirty buffer failed the write verifier. */
> +#define XFS_BUFTARG_CORRUPT_WRITE (1 << 1)
> +
> extern void libxfs_buftarg_init(struct xfs_mount *mp, dev_t ddev,
> dev_t logdev, dev_t rtdev);
> +int libxfs_blkdev_issue_flush(struct xfs_buftarg *btp);
>
> #define LIBXFS_BBTOOFF64(bbs) (((xfs_off_t)(bbs)) << BBSHIFT)
>
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 8b47d438..4253b890 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -17,6 +17,7 @@
> #include "xfs_inode_fork.h"
> #include "xfs_inode.h"
> #include "xfs_trans.h"
> +#include "libfrog/platform.h"
>
> #include "libxfs.h" /* for LIBXFS_EXIT_ON_FAILURE */
>
> @@ -1219,6 +1220,19 @@ libxfs_iomove(xfs_buf_t *bp, uint boff, int len, void *data, int flags)
> }
> }
>
> +/* Complain about (and remember) dropping dirty buffers. */
> +static void
> +libxfs_whine_dirty_buf(
> + struct xfs_buf *bp)
> +{
> + fprintf(stderr, _("%s: Releasing dirty buffer to free list!\n"),
> + progname);
> +
> + if (bp->b_error == -EFSCORRUPTED)
> + bp->b_target->flags |= XFS_BUFTARG_CORRUPT_WRITE;
> + bp->b_target->flags |= XFS_BUFTARG_LOST_WRITE;
> +}
> +
> static void
> libxfs_brelse(
> struct cache_node *node)
> @@ -1228,8 +1242,7 @@ libxfs_brelse(
> if (!bp)
> return;
> if (bp->b_flags & LIBXFS_B_DIRTY)
> - fprintf(stderr,
> - "releasing dirty buffer to free list!\n");
> + libxfs_whine_dirty_buf(bp);
>
> pthread_mutex_lock(&xfs_buf_freelist.cm_mutex);
> list_add(&bp->b_node.cn_mru, &xfs_buf_freelist.cm_list);
> @@ -1249,8 +1262,7 @@ libxfs_bulkrelse(
>
> list_for_each_entry(bp, list, b_node.cn_mru) {
> if (bp->b_flags & LIBXFS_B_DIRTY)
> - fprintf(stderr,
> - "releasing dirty buffer (bulk) to free list!\n");
> + libxfs_whine_dirty_buf(bp);
> count++;
> }
>
> @@ -1479,6 +1491,24 @@ libxfs_irele(
> kmem_cache_free(xfs_inode_zone, ip);
> }
>
> +/*
> + * Flush everything dirty in the kernel and disk write caches to stable media.
> + * Returns 0 for success or a negative error code.
> + */
> +int
> +libxfs_blkdev_issue_flush(
> + struct xfs_buftarg *btp)
> +{
> + int fd, ret;
> +
> + if (btp->dev == 0)
> + return 0;
> +
> + fd = libxfs_device_to_fd(btp->dev);
> + ret = platform_flush_device(fd, btp->dev);
> + return ret ? -errno : 0;
> +}
> +
> /*
> * Write out a buffer list synchronously.
> *
>
