Message ID | 158258945354.451075.11223931828645692053.stgit@magnolia (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | xfsprogs: actually check that writes succeeded | expand |
On Mon, Feb 24, 2020 at 04:10:53PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > 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@oracle.com> > --- Looks reasonable to me: Reviewed-by: Brian Foster <bfoster@redhat.com> > 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. > * >
On Mon, Feb 24, 2020 at 04:10:53PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > 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@oracle.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de>
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. *