[PATCH 0/4] libxfs: actually check that writes succeeded

[PATCH 0/4] libxfs: actually check that writes succeeded

[Darrick J. Wong]

Hi all,

A code audit demonstrated that many xfsprogs utilities do not check that
the buffers they write actually make it to disk.  While the userspace
buffer cache has a means to check that a buffer was written (which is to
reread the buffer after the write), most utilities mark a buffer dirty
and release it to the MRU and do not re-check the buffer.

Worse yet, the MRU will retain the buffers for all failed writes until
the buffer cache is torn down, but it in turn has no way to communicate
that writes were lost due to IO errors.  libxfs will flush the device
when it is unmounted, but as there is no return value, we again fail to
notice that writes have been lost.  Most likely this leads to a corrupt
filesystem, which makes it all the more surprising that xfs_repair can
lose writes yet still return 0!

Fix all this by making delwri_submit a synchronous write operation like
its kernel counterpart; teaching the buffer cache to mark the buftarg
when it knows it's losing writes; teaching the device flush functions to
return error codes; and adding a new "flush filesystem" API that user
programs can call to check for lost writes or IO errors.  Then teach all
the userspace programs to flush the fs at exit and report errors.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=buffer-write-fixes

[PATCH 1/4] libxfs: libxfs_buf_delwri_submit should write buffers immediately

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

The whole point of libxfs_buf_delwri_submit is to submit a bunch of
buffers for write and wait for the response.  Unfortunately, while it
does mark the buffers dirty, it doesn't actually flush them and lets the
cache mru flusher do it.  This is inconsistent with the kernel API,
which actually writes the buffers and returns any IO errors.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/rdwr.c   |    3 ++-
 mkfs/xfs_mkfs.c |   16 ++++++++++------
 2 files changed, 12 insertions(+), 7 deletions(-)


diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 0d9d7202..2e9f66cc 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1491,9 +1491,10 @@ xfs_buf_delwri_submit(
 
 	list_for_each_entry_safe(bp, n, buffer_list, b_list) {
 		list_del_init(&bp->b_list);
-		error2 = libxfs_writebuf(bp, 0);
+		error2 = libxfs_writebufr(bp);
 		if (!error)
 			error = error2;
+		libxfs_putbuf(bp);
 	}
 
 	return error;
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 5a042917..1f5d2105 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3685,6 +3685,7 @@ main(
 	};
 
 	struct list_head	buffer_list;
+	int			error;
 
 	platform_uuid_generate(&cli.uuid);
 	progname = basename(argv[0]);
@@ -3885,16 +3886,19 @@ main(
 		if (agno % 16)
 			continue;
 
-		if (libxfs_buf_delwri_submit(&buffer_list)) {
-			fprintf(stderr, _("%s: writing AG headers failed\n"),
-					progname);
+		error = -libxfs_buf_delwri_submit(&buffer_list);
+		if (error) {
+			fprintf(stderr,
+	_("%s: writing AG headers failed, err=%d\n"),
+					progname, error);
 			exit(1);
 		}
 	}
 
-	if (libxfs_buf_delwri_submit(&buffer_list)) {
-		fprintf(stderr, _("%s: writing AG headers failed\n"),
-				progname);
+	error = -libxfs_buf_delwri_submit(&buffer_list);
+	if (error) {
+		fprintf(stderr, _("%s: writing AG headers failed, err=%d\n"),
+				progname, error);
 		exit(1);
 	}
 

[PATCH 2/4] libxfs: complain when write IOs fail

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Complain whenever a metadata write fails.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/rdwr.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 2e9f66cc..8b47d438 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1149,7 +1149,12 @@ libxfs_writebufr(xfs_buf_t *bp)
 			(long long)LIBXFS_BBTOOFF64(bp->b_bn),
 			(long long)bp->b_bn, bp, bp->b_error);
 #endif
-	if (!bp->b_error) {
+	if (bp->b_error) {
+		fprintf(stderr,
+	_("%s: write failed on %s bno 0x%llx/0x%x, err=%d\n"),
+			__func__, bp->b_ops->name,
+			(long long)bp->b_bn, bp->b_bcount, -bp->b_error);
+	} else {
 		bp->b_flags |= LIBXFS_B_UPTODATE;
 		bp->b_flags &= ~(LIBXFS_B_DIRTY | LIBXFS_B_EXIT |
 				 LIBXFS_B_UNCHECKED);

[PATCH 3/4] libxfs: return flush failures

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Modify platform_flush_device so that we can return error status when
device flushes fail.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libfrog/linux.c    |   25 +++++++++++++++++--------
 libfrog/platform.h |    2 +-
 2 files changed, 18 insertions(+), 9 deletions(-)


diff --git a/libfrog/linux.c b/libfrog/linux.c
index 41a168b4..60bc1dc4 100644
--- a/libfrog/linux.c
+++ b/libfrog/linux.c
@@ -140,20 +140,29 @@ platform_set_blocksize(int fd, char *path, dev_t device, int blocksize, int fata
 	return error;
 }
 
-void
-platform_flush_device(int fd, dev_t device)
+/*
+ * Flush dirty pagecache and disk write cache to stable media.  Returns 0 for
+ * success or -1 (with errno set) for failure.
+ */
+int
+platform_flush_device(
+	int		fd,
+	dev_t		device)
 {
 	struct stat	st;
+	int		ret;
+
 	if (major(device) == RAMDISK_MAJOR)
-		return;
+		return 0;
 
-	if (fstat(fd, &st) < 0)
-		return;
+	ret = fstat(fd, &st);
+	if (ret)
+		return ret;
 
 	if (S_ISREG(st.st_mode))
-		fsync(fd);
-	else
-		ioctl(fd, BLKFLSBUF, 0);
+		return fsync(fd);
+
+	return ioctl(fd, BLKFLSBUF, 0);
 }
 
 void
diff --git a/libfrog/platform.h b/libfrog/platform.h
index 76887e5e..0aef318a 100644
--- a/libfrog/platform.h
+++ b/libfrog/platform.h
@@ -12,7 +12,7 @@ int platform_check_ismounted(char *path, char *block, struct stat *sptr,
 int platform_check_iswritable(char *path, char *block, struct stat *sptr);
 int platform_set_blocksize(int fd, char *path, dev_t device, int bsz,
 		int fatal);
-void platform_flush_device(int fd, dev_t device);
+int platform_flush_device(int fd, dev_t device);
 char *platform_findrawpath(char *path);
 char *platform_findblockpath(char *path);
 int platform_direct_blockdev(void);

[PATCH 4/4] misc: make all tools check that metadata updates have been committed

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Add a new function that will ensure that everything we changed has
landed on stable media, and report the results.  Teach the individual
programs to report when things go wrong.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/init.c           |   14 ++++++++++++++
 include/xfs_mount.h |    3 +++
 libxfs/init.c       |   43 +++++++++++++++++++++++++++++++++++++++++++
 libxfs/libxfs_io.h  |    2 ++
 libxfs/rdwr.c       |   27 +++++++++++++++++++++++++--
 mkfs/xfs_mkfs.c     |   15 +++++++++++++++
 repair/xfs_repair.c |   35 +++++++++++++++++++++++++++++++++++
 7 files changed, 137 insertions(+), 2 deletions(-)


diff --git a/db/init.c b/db/init.c
index 0ac37368..e92de232 100644
--- a/db/init.c
+++ b/db/init.c
@@ -184,6 +184,7 @@ main(
 	char	*input;
 	char	**v;
 	int	start_iocur_sp;
+	int	d, l, r;
 
 	init(argc, argv);
 	start_iocur_sp = iocur_sp;
@@ -216,6 +217,19 @@ main(
 	 */
 	while (iocur_sp > start_iocur_sp)
 		pop_cur();
+
+	libxfs_flush_devices(mp, &d, &l, &r);
+	if (d)
+		fprintf(stderr, _("%s: cannot flush data device (%d).\n"),
+				progname, d);
+	if (l)
+		fprintf(stderr, _("%s: cannot flush log device (%d).\n"),
+				progname, l);
+	if (r)
+		fprintf(stderr, _("%s: cannot flush realtime device (%d).\n"),
+				progname, r);
+
+
 	libxfs_umount(mp);
 	if (x.ddev)
 		libxfs_device_close(x.ddev);
diff --git a/include/xfs_mount.h b/include/xfs_mount.h
index 29b3cc1b..c80aaf69 100644
--- a/include/xfs_mount.h
+++ b/include/xfs_mount.h
@@ -187,4 +187,7 @@ extern xfs_mount_t	*libxfs_mount (xfs_mount_t *, xfs_sb_t *,
 extern void	libxfs_umount (xfs_mount_t *);
 extern void	libxfs_rtmount_destroy (xfs_mount_t *);
 
+void libxfs_flush_devices(struct xfs_mount *mp, int *datadev, int *logdev,
+		int *rtdev);
+
 #endif	/* __XFS_MOUNT_H__ */
diff --git a/libxfs/init.c b/libxfs/init.c
index a0d4b7f4..d1d3f4df 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -569,6 +569,8 @@ libxfs_buftarg_alloc(
 	}
 	btp->bt_mount = mp;
 	btp->dev = dev;
+	btp->lost_writes = false;
+
 	return btp;
 }
 
@@ -791,6 +793,47 @@ libxfs_rtmount_destroy(xfs_mount_t *mp)
 	mp->m_rsumip = mp->m_rbmip = NULL;
 }
 
+static inline int
+libxfs_flush_buftarg(
+	struct xfs_buftarg	*btp)
+{
+	if (btp->lost_writes)
+		return -ENOTRECOVERABLE;
+
+	return libxfs_blkdev_issue_flush(btp);
+}
+
+/*
+ * Purge the buffer cache to write all dirty buffers to disk and free all
+ * incore buffers.  Buffers that cannot be written will cause the lost_writes
+ * flag to be set in the buftarg.  If there were no lost writes, flush the
+ * device to make sure the writes made it to stable storage.
+ *
+ * For each device, the return code will be set to -ENOTRECOVERABLE if we
+ * couldn't write something to disk; or the results of the block device flush
+ * operation.
+ */
+void
+libxfs_flush_devices(
+	struct xfs_mount	*mp,
+	int			*datadev,
+	int			*logdev,
+	int			*rtdev)
+{
+	*datadev = *logdev = *rtdev = 0;
+
+	libxfs_bcache_purge();
+
+	if (mp->m_ddev_targp)
+		*datadev = libxfs_flush_buftarg(mp->m_ddev_targp);
+
+	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
+		*logdev = libxfs_flush_buftarg(mp->m_logdev_targp);
+
+	if (mp->m_rtdev_targp)
+		*rtdev = libxfs_flush_buftarg(mp->m_rtdev_targp);
+}
+
 /*
  * Release any resource obtained during a mount.
  */
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 579df52b..fc0fd060 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -23,10 +23,12 @@ struct xfs_perag;
 struct xfs_buftarg {
 	struct xfs_mount	*bt_mount;
 	dev_t			dev;
+	bool			lost_writes;
 };
 
 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..92e497f9 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 */
 
@@ -1227,9 +1228,11 @@ libxfs_brelse(
 
 	if (!bp)
 		return;
-	if (bp->b_flags & LIBXFS_B_DIRTY)
+	if (bp->b_flags & LIBXFS_B_DIRTY) {
 		fprintf(stderr,
 			"releasing dirty buffer to free list!\n");
+		bp->b_target->lost_writes = true;
+	}
 
 	pthread_mutex_lock(&xfs_buf_freelist.cm_mutex);
 	list_add(&bp->b_node.cn_mru, &xfs_buf_freelist.cm_list);
@@ -1248,9 +1251,11 @@ libxfs_bulkrelse(
 		return 0 ;
 
 	list_for_each_entry(bp, list, b_node.cn_mru) {
-		if (bp->b_flags & LIBXFS_B_DIRTY)
+		if (bp->b_flags & LIBXFS_B_DIRTY) {
 			fprintf(stderr,
 				"releasing dirty buffer (bulk) to free list!\n");
+			bp->b_target->lost_writes = true;
+		}
 		count++;
 	}
 
@@ -1479,6 +1484,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.
  *
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 1f5d2105..6b182264 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3644,6 +3644,7 @@ main(
 	char			*protofile = NULL;
 	char			*protostring = NULL;
 	int			worst_freelist = 0;
+	int			d, l, r;
 
 	struct libxfs_xinit	xi = {
 		.isdirect = LIBXFS_DIRECT,
@@ -3940,6 +3941,20 @@ main(
 	(XFS_BUF_TO_SBP(buf))->sb_inprogress = 0;
 	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
 
+	/* Make sure our new fs made it to stable storage. */
+	libxfs_flush_devices(mp, &d, &l, &r);
+	if (d)
+		fprintf(stderr, _("%s: cannot flush data device (%d).\n"),
+				progname, d);
+	if (l)
+		fprintf(stderr, _("%s: cannot flush log device (%d).\n"),
+				progname, l);
+	if (r)
+		fprintf(stderr, _("%s: cannot flush realtime device (%d).\n"),
+				progname, r);
+	if (d || l || r)
+		return 1;
+
 	libxfs_umount(mp);
 	if (xi.rtdev)
 		libxfs_device_close(xi.rtdev);
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
index eb1ce546..c0a77cad 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -690,6 +690,36 @@ check_fs_vs_host_sectsize(
 	}
 }
 
+/* Flush the devices and complain if anything bad happened. */
+static bool
+check_write_failed(
+	struct xfs_mount	*mp)
+{
+	int			d, l, r;
+
+	libxfs_flush_devices(mp, &d, &l, &r);
+
+	if (d == -ENOTRECOVERABLE)
+		do_warn(_("Lost writes to data device, please re-run.\n"));
+	else if (d)
+		do_warn(_("Error %d flushing data device, please re-run.\n"),
+				-d);
+
+	if (l == -ENOTRECOVERABLE)
+		do_warn(_("Lost writes to log device, please re-run.\n"));
+	else if (l)
+		do_warn(_("Error %d flushing log device, please re-run.\n"),
+				-l);
+
+	if (r == -ENOTRECOVERABLE)
+		do_warn(_("Lost writes to realtime device, please re-run.\n"));
+	else if (r)
+		do_warn(_("Error %d flushing realtime device, please re-run.\n"),
+				-r);
+
+	return d || l || r;
+}
+
 int
 main(int argc, char **argv)
 {
@@ -703,6 +733,7 @@ main(int argc, char **argv)
 	struct xfs_sb	psb;
 	int		rval;
 	struct xfs_ino_geometry	*igeo;
+	bool		writes_failed;
 
 	progname = basename(argv[0]);
 	setlocale(LC_ALL, "");
@@ -1106,6 +1137,8 @@ _("Note - stripe unit (%d) and width (%d) were copied from a backup superblock.\
 	format_log_max_lsn(mp);
 	libxfs_umount(mp);
 
+	writes_failed = check_write_failed(mp);
+
 	if (x.rtdev)
 		libxfs_device_close(x.rtdev);
 	if (x.logdev && x.logdev != x.ddev)
@@ -1125,6 +1158,8 @@ _("Repair of readonly mount complete.  Immediate reboot encouraged.\n"));
 
 	free(msgbuf);
 
+	if (writes_failed)
+		return 1;
 	if (fs_is_dirty && report_corrected)
 		return (4);
 	return (0);

Re: [PATCH 1/4] libxfs: libxfs_buf_delwri_submit should write buffers immediately

[Allison Collins]

On 2/4/20 5:47 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The whole point of libxfs_buf_delwri_submit is to submit a bunch of
> buffers for write and wait for the response.  Unfortunately, while it
> does mark the buffers dirty, it doesn't actually flush them and lets the
> cache mru flusher do it.  This is inconsistent with the kernel API,
> which actually writes the buffers and returns any IO errors.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks ok to me:
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   libxfs/rdwr.c   |    3 ++-
>   mkfs/xfs_mkfs.c |   16 ++++++++++------
>   2 files changed, 12 insertions(+), 7 deletions(-)
> 
> 
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 0d9d7202..2e9f66cc 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -1491,9 +1491,10 @@ xfs_buf_delwri_submit(
>   
>   	list_for_each_entry_safe(bp, n, buffer_list, b_list) {
>   		list_del_init(&bp->b_list);
> -		error2 = libxfs_writebuf(bp, 0);
> +		error2 = libxfs_writebufr(bp);
>   		if (!error)
>   			error = error2;
> +		libxfs_putbuf(bp);
>   	}
>   
>   	return error;
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 5a042917..1f5d2105 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3685,6 +3685,7 @@ main(
>   	};
>   
>   	struct list_head	buffer_list;
> +	int			error;
>   
>   	platform_uuid_generate(&cli.uuid);
>   	progname = basename(argv[0]);
> @@ -3885,16 +3886,19 @@ main(
>   		if (agno % 16)
>   			continue;
>   
> -		if (libxfs_buf_delwri_submit(&buffer_list)) {
> -			fprintf(stderr, _("%s: writing AG headers failed\n"),
> -					progname);
> +		error = -libxfs_buf_delwri_submit(&buffer_list);
> +		if (error) {
> +			fprintf(stderr,
> +	_("%s: writing AG headers failed, err=%d\n"),
> +					progname, error);
>   			exit(1);
>   		}
>   	}
>   
> -	if (libxfs_buf_delwri_submit(&buffer_list)) {
> -		fprintf(stderr, _("%s: writing AG headers failed\n"),
> -				progname);
> +	error = -libxfs_buf_delwri_submit(&buffer_list);
> +	if (error) {
> +		fprintf(stderr, _("%s: writing AG headers failed, err=%d\n"),
> +				progname, error);
>   		exit(1);
>   	}
>   
> 

Re: [PATCH 2/4] libxfs: complain when write IOs fail

[Allison Collins]

On 2/4/20 5:47 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Complain whenever a metadata write fails.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good:
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   libxfs/rdwr.c |    7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 2e9f66cc..8b47d438 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -1149,7 +1149,12 @@ libxfs_writebufr(xfs_buf_t *bp)
>   			(long long)LIBXFS_BBTOOFF64(bp->b_bn),
>   			(long long)bp->b_bn, bp, bp->b_error);
>   #endif
> -	if (!bp->b_error) {
> +	if (bp->b_error) {
> +		fprintf(stderr,
> +	_("%s: write failed on %s bno 0x%llx/0x%x, err=%d\n"),
> +			__func__, bp->b_ops->name,
> +			(long long)bp->b_bn, bp->b_bcount, -bp->b_error);
> +	} else {
>   		bp->b_flags |= LIBXFS_B_UPTODATE;
>   		bp->b_flags &= ~(LIBXFS_B_DIRTY | LIBXFS_B_EXIT |
>   				 LIBXFS_B_UNCHECKED);
> 

Re: [PATCH 3/4] libxfs: return flush failures

[Allison Collins]

On 2/4/20 5:47 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Modify platform_flush_device so that we can return error status when
> device flushes fail.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
I think there's one other place in init.c where platform_flush_device is 
called, but I suppose it didn't need the return code before?  Other than 
that it looks ok.

Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   libfrog/linux.c    |   25 +++++++++++++++++--------
>   libfrog/platform.h |    2 +-
>   2 files changed, 18 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/libfrog/linux.c b/libfrog/linux.c
> index 41a168b4..60bc1dc4 100644
> --- a/libfrog/linux.c
> +++ b/libfrog/linux.c
> @@ -140,20 +140,29 @@ platform_set_blocksize(int fd, char *path, dev_t device, int blocksize, int fata
>   	return error;
>   }
>   
> -void
> -platform_flush_device(int fd, dev_t device)
> +/*
> + * Flush dirty pagecache and disk write cache to stable media.  Returns 0 for
> + * success or -1 (with errno set) for failure.
> + */
> +int
> +platform_flush_device(
> +	int		fd,
> +	dev_t		device)
>   {
>   	struct stat	st;
> +	int		ret;
> +
>   	if (major(device) == RAMDISK_MAJOR)
> -		return;
> +		return 0;
>   
> -	if (fstat(fd, &st) < 0)
> -		return;
> +	ret = fstat(fd, &st);
> +	if (ret)
> +		return ret;
>   
>   	if (S_ISREG(st.st_mode))
> -		fsync(fd);
> -	else
> -		ioctl(fd, BLKFLSBUF, 0);
> +		return fsync(fd);
> +
> +	return ioctl(fd, BLKFLSBUF, 0);
>   }
>   
>   void
> diff --git a/libfrog/platform.h b/libfrog/platform.h
> index 76887e5e..0aef318a 100644
> --- a/libfrog/platform.h
> +++ b/libfrog/platform.h
> @@ -12,7 +12,7 @@ int platform_check_ismounted(char *path, char *block, struct stat *sptr,
>   int platform_check_iswritable(char *path, char *block, struct stat *sptr);
>   int platform_set_blocksize(int fd, char *path, dev_t device, int bsz,
>   		int fatal);
> -void platform_flush_device(int fd, dev_t device);
> +int platform_flush_device(int fd, dev_t device);
>   char *platform_findrawpath(char *path);
>   char *platform_findblockpath(char *path);
>   int platform_direct_blockdev(void);
> 

Re: [PATCH 4/4] misc: make all tools check that metadata updates have been committed

[Allison Collins]

On 2/4/20 5:47 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a new function that will ensure that everything we changed has
> landed on stable media, and report the results.  Teach the individual
> programs to report when things go wrong.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Ok, looks functionally sane to me:
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   db/init.c           |   14 ++++++++++++++
>   include/xfs_mount.h |    3 +++
>   libxfs/init.c       |   43 +++++++++++++++++++++++++++++++++++++++++++
>   libxfs/libxfs_io.h  |    2 ++
>   libxfs/rdwr.c       |   27 +++++++++++++++++++++++++--
>   mkfs/xfs_mkfs.c     |   15 +++++++++++++++
>   repair/xfs_repair.c |   35 +++++++++++++++++++++++++++++++++++
>   7 files changed, 137 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/db/init.c b/db/init.c
> index 0ac37368..e92de232 100644
> --- a/db/init.c
> +++ b/db/init.c
> @@ -184,6 +184,7 @@ main(
>   	char	*input;
>   	char	**v;
>   	int	start_iocur_sp;
> +	int	d, l, r;
>   
>   	init(argc, argv);
>   	start_iocur_sp = iocur_sp;
> @@ -216,6 +217,19 @@ main(
>   	 */
>   	while (iocur_sp > start_iocur_sp)
>   		pop_cur();
> +
> +	libxfs_flush_devices(mp, &d, &l, &r);
> +	if (d)
> +		fprintf(stderr, _("%s: cannot flush data device (%d).\n"),
> +				progname, d);
> +	if (l)
> +		fprintf(stderr, _("%s: cannot flush log device (%d).\n"),
> +				progname, l);
> +	if (r)
> +		fprintf(stderr, _("%s: cannot flush realtime device (%d).\n"),
> +				progname, r);
> +
> +
>   	libxfs_umount(mp);
>   	if (x.ddev)
>   		libxfs_device_close(x.ddev);
> diff --git a/include/xfs_mount.h b/include/xfs_mount.h
> index 29b3cc1b..c80aaf69 100644
> --- a/include/xfs_mount.h
> +++ b/include/xfs_mount.h
> @@ -187,4 +187,7 @@ extern xfs_mount_t	*libxfs_mount (xfs_mount_t *, xfs_sb_t *,
>   extern void	libxfs_umount (xfs_mount_t *);
>   extern void	libxfs_rtmount_destroy (xfs_mount_t *);
>   
> +void libxfs_flush_devices(struct xfs_mount *mp, int *datadev, int *logdev,
> +		int *rtdev);
> +
>   #endif	/* __XFS_MOUNT_H__ */
> diff --git a/libxfs/init.c b/libxfs/init.c
> index a0d4b7f4..d1d3f4df 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -569,6 +569,8 @@ libxfs_buftarg_alloc(
>   	}
>   	btp->bt_mount = mp;
>   	btp->dev = dev;
> +	btp->lost_writes = false;
> +
>   	return btp;
>   }
>   
> @@ -791,6 +793,47 @@ libxfs_rtmount_destroy(xfs_mount_t *mp)
>   	mp->m_rsumip = mp->m_rbmip = NULL;
>   }
>   
> +static inline int
> +libxfs_flush_buftarg(
> +	struct xfs_buftarg	*btp)
> +{
> +	if (btp->lost_writes)
> +		return -ENOTRECOVERABLE;
> +
> +	return libxfs_blkdev_issue_flush(btp);
> +}
> +
> +/*
> + * Purge the buffer cache to write all dirty buffers to disk and free all
> + * incore buffers.  Buffers that cannot be written will cause the lost_writes
> + * flag to be set in the buftarg.  If there were no lost writes, flush the
> + * device to make sure the writes made it to stable storage.
> + *
> + * For each device, the return code will be set to -ENOTRECOVERABLE if we
> + * couldn't write something to disk; or the results of the block device flush
> + * operation.
> + */
> +void
> +libxfs_flush_devices(
> +	struct xfs_mount	*mp,
> +	int			*datadev,
> +	int			*logdev,
> +	int			*rtdev)
> +{
> +	*datadev = *logdev = *rtdev = 0;
> +
> +	libxfs_bcache_purge();
> +
> +	if (mp->m_ddev_targp)
> +		*datadev = libxfs_flush_buftarg(mp->m_ddev_targp);
> +
> +	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
> +		*logdev = libxfs_flush_buftarg(mp->m_logdev_targp);
> +
> +	if (mp->m_rtdev_targp)
> +		*rtdev = libxfs_flush_buftarg(mp->m_rtdev_targp);
> +}
> +
>   /*
>    * Release any resource obtained during a mount.
>    */
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 579df52b..fc0fd060 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -23,10 +23,12 @@ struct xfs_perag;
>   struct xfs_buftarg {
>   	struct xfs_mount	*bt_mount;
>   	dev_t			dev;
> +	bool			lost_writes;
>   };
>   
>   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..92e497f9 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 */
>   
> @@ -1227,9 +1228,11 @@ libxfs_brelse(
>   
>   	if (!bp)
>   		return;
> -	if (bp->b_flags & LIBXFS_B_DIRTY)
> +	if (bp->b_flags & LIBXFS_B_DIRTY) {
>   		fprintf(stderr,
>   			"releasing dirty buffer to free list!\n");
> +		bp->b_target->lost_writes = true;
> +	}
>   
>   	pthread_mutex_lock(&xfs_buf_freelist.cm_mutex);
>   	list_add(&bp->b_node.cn_mru, &xfs_buf_freelist.cm_list);
> @@ -1248,9 +1251,11 @@ libxfs_bulkrelse(
>   		return 0 ;
>   
>   	list_for_each_entry(bp, list, b_node.cn_mru) {
> -		if (bp->b_flags & LIBXFS_B_DIRTY)
> +		if (bp->b_flags & LIBXFS_B_DIRTY) {
>   			fprintf(stderr,
>   				"releasing dirty buffer (bulk) to free list!\n");
> +			bp->b_target->lost_writes = true;
> +		}
>   		count++;
>   	}
>   
> @@ -1479,6 +1484,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.
>    *
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 1f5d2105..6b182264 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3644,6 +3644,7 @@ main(
>   	char			*protofile = NULL;
>   	char			*protostring = NULL;
>   	int			worst_freelist = 0;
> +	int			d, l, r;
>   
>   	struct libxfs_xinit	xi = {
>   		.isdirect = LIBXFS_DIRECT,
> @@ -3940,6 +3941,20 @@ main(
>   	(XFS_BUF_TO_SBP(buf))->sb_inprogress = 0;
>   	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
>   
> +	/* Make sure our new fs made it to stable storage. */
> +	libxfs_flush_devices(mp, &d, &l, &r);
> +	if (d)
> +		fprintf(stderr, _("%s: cannot flush data device (%d).\n"),
> +				progname, d);
> +	if (l)
> +		fprintf(stderr, _("%s: cannot flush log device (%d).\n"),
> +				progname, l);
> +	if (r)
> +		fprintf(stderr, _("%s: cannot flush realtime device (%d).\n"),
> +				progname, r);
> +	if (d || l || r)
> +		return 1;
> +
>   	libxfs_umount(mp);
>   	if (xi.rtdev)
>   		libxfs_device_close(xi.rtdev);
> diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> index eb1ce546..c0a77cad 100644
> --- a/repair/xfs_repair.c
> +++ b/repair/xfs_repair.c
> @@ -690,6 +690,36 @@ check_fs_vs_host_sectsize(
>   	}
>   }
>   
> +/* Flush the devices and complain if anything bad happened. */
> +static bool
> +check_write_failed(
> +	struct xfs_mount	*mp)
> +{
> +	int			d, l, r;
> +
> +	libxfs_flush_devices(mp, &d, &l, &r);
> +
> +	if (d == -ENOTRECOVERABLE)
> +		do_warn(_("Lost writes to data device, please re-run.\n"));
> +	else if (d)
> +		do_warn(_("Error %d flushing data device, please re-run.\n"),
> +				-d);
> +
> +	if (l == -ENOTRECOVERABLE)
> +		do_warn(_("Lost writes to log device, please re-run.\n"));
> +	else if (l)
> +		do_warn(_("Error %d flushing log device, please re-run.\n"),
> +				-l);
> +
> +	if (r == -ENOTRECOVERABLE)
> +		do_warn(_("Lost writes to realtime device, please re-run.\n"));
> +	else if (r)
> +		do_warn(_("Error %d flushing realtime device, please re-run.\n"),
> +				-r);
> +
> +	return d || l || r;
> +}
> +
>   int
>   main(int argc, char **argv)
>   {
> @@ -703,6 +733,7 @@ main(int argc, char **argv)
>   	struct xfs_sb	psb;
>   	int		rval;
>   	struct xfs_ino_geometry	*igeo;
> +	bool		writes_failed;
>   
>   	progname = basename(argv[0]);
>   	setlocale(LC_ALL, "");
> @@ -1106,6 +1137,8 @@ _("Note - stripe unit (%d) and width (%d) were copied from a backup superblock.\
>   	format_log_max_lsn(mp);
>   	libxfs_umount(mp);
>   
> +	writes_failed = check_write_failed(mp);
> +
>   	if (x.rtdev)
>   		libxfs_device_close(x.rtdev);
>   	if (x.logdev && x.logdev != x.ddev)
> @@ -1125,6 +1158,8 @@ _("Repair of readonly mount complete.  Immediate reboot encouraged.\n"));
>   
>   	free(msgbuf);
>   
> +	if (writes_failed)
> +		return 1;
>   	if (fs_is_dirty && report_corrected)
>   		return (4);
>   	return (0);
> 

Re: [PATCH 1/4] libxfs: libxfs_buf_delwri_submit should write buffers immediately

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

But can we come up with a better name and description for
libxfs_writebufr? and libxfs_writebuf?

Re: [PATCH 2/4] libxfs: complain when write IOs fail

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/4] libxfs: return flush failures

[Christoph Hellwig]

On Tue, Feb 04, 2020 at 04:47:43PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Modify platform_flush_device so that we can return error status when
> device flushes fail.

The change itself looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

But the existing logic in platform_flush_device looks suspicious.
Even on a block device fsync is usually the right thing, so we
should unconditionally do that first.  The BLKFLSBUF ioctl does some
rather weird things which I'm not sure we really want here, but if we
do I'd rather see if after we've flushed out the actual data..

Re: [PATCH 4/4] misc: make all tools check that metadata updates have been committed

[Christoph Hellwig]

On Tue, Feb 04, 2020 at 04:47:49PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a new function that will ensure that everything we changed has
> landed on stable media, and report the results.  Teach the individual
> programs to report when things go wrong.

This just seems to touch a few tools, so all seems a little
unspecific.  Best split this into the infrastructure in one patch,
and then one per tool to wire the call up.

Re: [PATCH 3/4] libxfs: return flush failures

[Darrick J. Wong]

On Thu, Feb 06, 2020 at 12:38:06PM -0700, Allison Collins wrote:
> 
> On 2/4/20 5:47 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Modify platform_flush_device so that we can return error status when
> > device flushes fail.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> I think there's one other place in init.c where platform_flush_device is
> called, but I suppose it didn't need the return code before?

I think the lack of error checking is/was just plain broken, seeing as
the current libxfs_close() code path just eats errors.

However, now that everyone calls libxfs_flush() to find out which
devices (if any) succeeded in flushing, I think there's less need to go
reworking that whole code path.

--D

> Other than that it looks ok.

> Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> 
> > ---
> >   libfrog/linux.c    |   25 +++++++++++++++++--------
> >   libfrog/platform.h |    2 +-
> >   2 files changed, 18 insertions(+), 9 deletions(-)
> > 
> > 
> > diff --git a/libfrog/linux.c b/libfrog/linux.c
> > index 41a168b4..60bc1dc4 100644
> > --- a/libfrog/linux.c
> > +++ b/libfrog/linux.c
> > @@ -140,20 +140,29 @@ platform_set_blocksize(int fd, char *path, dev_t device, int blocksize, int fata
> >   	return error;
> >   }
> > -void
> > -platform_flush_device(int fd, dev_t device)
> > +/*
> > + * Flush dirty pagecache and disk write cache to stable media.  Returns 0 for
> > + * success or -1 (with errno set) for failure.
> > + */
> > +int
> > +platform_flush_device(
> > +	int		fd,
> > +	dev_t		device)
> >   {
> >   	struct stat	st;
> > +	int		ret;
> > +
> >   	if (major(device) == RAMDISK_MAJOR)
> > -		return;
> > +		return 0;
> > -	if (fstat(fd, &st) < 0)
> > -		return;
> > +	ret = fstat(fd, &st);
> > +	if (ret)
> > +		return ret;
> >   	if (S_ISREG(st.st_mode))
> > -		fsync(fd);
> > -	else
> > -		ioctl(fd, BLKFLSBUF, 0);
> > +		return fsync(fd);
> > +
> > +	return ioctl(fd, BLKFLSBUF, 0);
> >   }
> >   void
> > diff --git a/libfrog/platform.h b/libfrog/platform.h
> > index 76887e5e..0aef318a 100644
> > --- a/libfrog/platform.h
> > +++ b/libfrog/platform.h
> > @@ -12,7 +12,7 @@ int platform_check_ismounted(char *path, char *block, struct stat *sptr,
> >   int platform_check_iswritable(char *path, char *block, struct stat *sptr);
> >   int platform_set_blocksize(int fd, char *path, dev_t device, int bsz,
> >   		int fatal);
> > -void platform_flush_device(int fd, dev_t device);
> > +int platform_flush_device(int fd, dev_t device);
> >   char *platform_findrawpath(char *path);
> >   char *platform_findblockpath(char *path);
> >   int platform_direct_blockdev(void);
> > 

Re: [PATCH 3/4] libxfs: return flush failures

[Darrick J. Wong]

On Mon, Feb 17, 2020 at 06:00:23AM -0800, Christoph Hellwig wrote:
> On Tue, Feb 04, 2020 at 04:47:43PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Modify platform_flush_device so that we can return error status when
> > device flushes fail.
> 
> The change itself looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> But the existing logic in platform_flush_device looks suspicious.
> Even on a block device fsync is usually the right thing, so we
> should unconditionally do that first.  The BLKFLSBUF ioctl does some
> rather weird things which I'm not sure we really want here, but if we
> do I'd rather see if after we've flushed out the actual data..

BLKFLSBUF flushes the data and then shoots down the page cache, right?
That's certainly odd, but I think (at least as far as old kernels go) we
aren't actively losing data.

However, I agree that we should fsync() files and block devices equally.

--D

Re: [PATCH 1/4] libxfs: libxfs_buf_delwri_submit should write buffers immediately

[Darrick J. Wong]

On Mon, Feb 17, 2020 at 05:57:11AM -0800, Christoph Hellwig wrote:
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> But can we come up with a better name and description for
> libxfs_writebufr? and libxfs_writebuf?

I'll fix both of those warts in the next cleanup series, for which I'm
running one more overnight fstests run.

In the meantime, thanks for reviewing these three series!

--D

Re: [PATCH 4/4] misc: make all tools check that metadata updates have been committed

[Darrick J. Wong]

On Thu, Feb 06, 2020 at 12:38:17PM -0700, Allison Collins wrote:
> 
> 
> On 2/4/20 5:47 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Add a new function that will ensure that everything we changed has
> > landed on stable media, and report the results.  Teach the individual
> > programs to report when things go wrong.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Ok, looks functionally sane to me:

Thanks for reviewing the three series!

--D

> Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> 
> > ---
> >   db/init.c           |   14 ++++++++++++++
> >   include/xfs_mount.h |    3 +++
> >   libxfs/init.c       |   43 +++++++++++++++++++++++++++++++++++++++++++
> >   libxfs/libxfs_io.h  |    2 ++
> >   libxfs/rdwr.c       |   27 +++++++++++++++++++++++++--
> >   mkfs/xfs_mkfs.c     |   15 +++++++++++++++
> >   repair/xfs_repair.c |   35 +++++++++++++++++++++++++++++++++++
> >   7 files changed, 137 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/db/init.c b/db/init.c
> > index 0ac37368..e92de232 100644
> > --- a/db/init.c
> > +++ b/db/init.c
> > @@ -184,6 +184,7 @@ main(
> >   	char	*input;
> >   	char	**v;
> >   	int	start_iocur_sp;
> > +	int	d, l, r;
> >   	init(argc, argv);
> >   	start_iocur_sp = iocur_sp;
> > @@ -216,6 +217,19 @@ main(
> >   	 */
> >   	while (iocur_sp > start_iocur_sp)
> >   		pop_cur();
> > +
> > +	libxfs_flush_devices(mp, &d, &l, &r);
> > +	if (d)
> > +		fprintf(stderr, _("%s: cannot flush data device (%d).\n"),
> > +				progname, d);
> > +	if (l)
> > +		fprintf(stderr, _("%s: cannot flush log device (%d).\n"),
> > +				progname, l);
> > +	if (r)
> > +		fprintf(stderr, _("%s: cannot flush realtime device (%d).\n"),
> > +				progname, r);
> > +
> > +
> >   	libxfs_umount(mp);
> >   	if (x.ddev)
> >   		libxfs_device_close(x.ddev);
> > diff --git a/include/xfs_mount.h b/include/xfs_mount.h
> > index 29b3cc1b..c80aaf69 100644
> > --- a/include/xfs_mount.h
> > +++ b/include/xfs_mount.h
> > @@ -187,4 +187,7 @@ extern xfs_mount_t	*libxfs_mount (xfs_mount_t *, xfs_sb_t *,
> >   extern void	libxfs_umount (xfs_mount_t *);
> >   extern void	libxfs_rtmount_destroy (xfs_mount_t *);
> > +void libxfs_flush_devices(struct xfs_mount *mp, int *datadev, int *logdev,
> > +		int *rtdev);
> > +
> >   #endif	/* __XFS_MOUNT_H__ */
> > diff --git a/libxfs/init.c b/libxfs/init.c
> > index a0d4b7f4..d1d3f4df 100644
> > --- a/libxfs/init.c
> > +++ b/libxfs/init.c
> > @@ -569,6 +569,8 @@ libxfs_buftarg_alloc(
> >   	}
> >   	btp->bt_mount = mp;
> >   	btp->dev = dev;
> > +	btp->lost_writes = false;
> > +
> >   	return btp;
> >   }
> > @@ -791,6 +793,47 @@ libxfs_rtmount_destroy(xfs_mount_t *mp)
> >   	mp->m_rsumip = mp->m_rbmip = NULL;
> >   }
> > +static inline int
> > +libxfs_flush_buftarg(
> > +	struct xfs_buftarg	*btp)
> > +{
> > +	if (btp->lost_writes)
> > +		return -ENOTRECOVERABLE;
> > +
> > +	return libxfs_blkdev_issue_flush(btp);
> > +}
> > +
> > +/*
> > + * Purge the buffer cache to write all dirty buffers to disk and free all
> > + * incore buffers.  Buffers that cannot be written will cause the lost_writes
> > + * flag to be set in the buftarg.  If there were no lost writes, flush the
> > + * device to make sure the writes made it to stable storage.
> > + *
> > + * For each device, the return code will be set to -ENOTRECOVERABLE if we
> > + * couldn't write something to disk; or the results of the block device flush
> > + * operation.
> > + */
> > +void
> > +libxfs_flush_devices(
> > +	struct xfs_mount	*mp,
> > +	int			*datadev,
> > +	int			*logdev,
> > +	int			*rtdev)
> > +{
> > +	*datadev = *logdev = *rtdev = 0;
> > +
> > +	libxfs_bcache_purge();
> > +
> > +	if (mp->m_ddev_targp)
> > +		*datadev = libxfs_flush_buftarg(mp->m_ddev_targp);
> > +
> > +	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
> > +		*logdev = libxfs_flush_buftarg(mp->m_logdev_targp);
> > +
> > +	if (mp->m_rtdev_targp)
> > +		*rtdev = libxfs_flush_buftarg(mp->m_rtdev_targp);
> > +}
> > +
> >   /*
> >    * Release any resource obtained during a mount.
> >    */
> > diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> > index 579df52b..fc0fd060 100644
> > --- a/libxfs/libxfs_io.h
> > +++ b/libxfs/libxfs_io.h
> > @@ -23,10 +23,12 @@ struct xfs_perag;
> >   struct xfs_buftarg {
> >   	struct xfs_mount	*bt_mount;
> >   	dev_t			dev;
> > +	bool			lost_writes;
> >   };
> >   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..92e497f9 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 */
> > @@ -1227,9 +1228,11 @@ libxfs_brelse(
> >   	if (!bp)
> >   		return;
> > -	if (bp->b_flags & LIBXFS_B_DIRTY)
> > +	if (bp->b_flags & LIBXFS_B_DIRTY) {
> >   		fprintf(stderr,
> >   			"releasing dirty buffer to free list!\n");
> > +		bp->b_target->lost_writes = true;
> > +	}
> >   	pthread_mutex_lock(&xfs_buf_freelist.cm_mutex);
> >   	list_add(&bp->b_node.cn_mru, &xfs_buf_freelist.cm_list);
> > @@ -1248,9 +1251,11 @@ libxfs_bulkrelse(
> >   		return 0 ;
> >   	list_for_each_entry(bp, list, b_node.cn_mru) {
> > -		if (bp->b_flags & LIBXFS_B_DIRTY)
> > +		if (bp->b_flags & LIBXFS_B_DIRTY) {
> >   			fprintf(stderr,
> >   				"releasing dirty buffer (bulk) to free list!\n");
> > +			bp->b_target->lost_writes = true;
> > +		}
> >   		count++;
> >   	}
> > @@ -1479,6 +1484,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.
> >    *
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index 1f5d2105..6b182264 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -3644,6 +3644,7 @@ main(
> >   	char			*protofile = NULL;
> >   	char			*protostring = NULL;
> >   	int			worst_freelist = 0;
> > +	int			d, l, r;
> >   	struct libxfs_xinit	xi = {
> >   		.isdirect = LIBXFS_DIRECT,
> > @@ -3940,6 +3941,20 @@ main(
> >   	(XFS_BUF_TO_SBP(buf))->sb_inprogress = 0;
> >   	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
> > +	/* Make sure our new fs made it to stable storage. */
> > +	libxfs_flush_devices(mp, &d, &l, &r);
> > +	if (d)
> > +		fprintf(stderr, _("%s: cannot flush data device (%d).\n"),
> > +				progname, d);
> > +	if (l)
> > +		fprintf(stderr, _("%s: cannot flush log device (%d).\n"),
> > +				progname, l);
> > +	if (r)
> > +		fprintf(stderr, _("%s: cannot flush realtime device (%d).\n"),
> > +				progname, r);
> > +	if (d || l || r)
> > +		return 1;
> > +
> >   	libxfs_umount(mp);
> >   	if (xi.rtdev)
> >   		libxfs_device_close(xi.rtdev);
> > diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> > index eb1ce546..c0a77cad 100644
> > --- a/repair/xfs_repair.c
> > +++ b/repair/xfs_repair.c
> > @@ -690,6 +690,36 @@ check_fs_vs_host_sectsize(
> >   	}
> >   }
> > +/* Flush the devices and complain if anything bad happened. */
> > +static bool
> > +check_write_failed(
> > +	struct xfs_mount	*mp)
> > +{
> > +	int			d, l, r;
> > +
> > +	libxfs_flush_devices(mp, &d, &l, &r);
> > +
> > +	if (d == -ENOTRECOVERABLE)
> > +		do_warn(_("Lost writes to data device, please re-run.\n"));
> > +	else if (d)
> > +		do_warn(_("Error %d flushing data device, please re-run.\n"),
> > +				-d);
> > +
> > +	if (l == -ENOTRECOVERABLE)
> > +		do_warn(_("Lost writes to log device, please re-run.\n"));
> > +	else if (l)
> > +		do_warn(_("Error %d flushing log device, please re-run.\n"),
> > +				-l);
> > +
> > +	if (r == -ENOTRECOVERABLE)
> > +		do_warn(_("Lost writes to realtime device, please re-run.\n"));
> > +	else if (r)
> > +		do_warn(_("Error %d flushing realtime device, please re-run.\n"),
> > +				-r);
> > +
> > +	return d || l || r;
> > +}
> > +
> >   int
> >   main(int argc, char **argv)
> >   {
> > @@ -703,6 +733,7 @@ main(int argc, char **argv)
> >   	struct xfs_sb	psb;
> >   	int		rval;
> >   	struct xfs_ino_geometry	*igeo;
> > +	bool		writes_failed;
> >   	progname = basename(argv[0]);
> >   	setlocale(LC_ALL, "");
> > @@ -1106,6 +1137,8 @@ _("Note - stripe unit (%d) and width (%d) were copied from a backup superblock.\
> >   	format_log_max_lsn(mp);
> >   	libxfs_umount(mp);
> > +	writes_failed = check_write_failed(mp);
> > +
> >   	if (x.rtdev)
> >   		libxfs_device_close(x.rtdev);
> >   	if (x.logdev && x.logdev != x.ddev)
> > @@ -1125,6 +1158,8 @@ _("Repair of readonly mount complete.  Immediate reboot encouraged.\n"));
> >   	free(msgbuf);
> > +	if (writes_failed)
> > +		return 1;
> >   	if (fs_is_dirty && report_corrected)
> >   		return (4);
> >   	return (0);
> >