test_librbd_fsx: invalidate before discard in krbd mode

Commit bd050240ceef ("test_librbd_fsx: flush before discard in krbd
mode") added an fsync() before BLKDISCARD.  Don't know what I was
thinking at the time, but I missed the invalidate part, for which we
need to use the BLKFLSBUF ioctl.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>

1 files changed, 40 insertions, 19 deletions

diff --git a/src/test/librbd/fsx.cc b/src/test/librbd/fsx.cc
index 40a908f6308..9eea49cbfe3 100644
--- a/src/test/librbd/fsx.cc
+++ b/src/test/librbd/fsx.cc
@@ -628,19 +628,34 @@ krbd_write(struct rbd_ctx *ctx, uint64_t off, size_t len, const char *buf)
 }
 
 int
-__krbd_flush(struct rbd_ctx *ctx)
+__krbd_flush(struct rbd_ctx *ctx, bool invalidate)
 {
+	int ret;
+
 	if (o_direct)
 		return 0;
 
 	/*
-	 * fsync(2) on the block device does not sync the filesystem
-	 * mounted on top of it, but that's OK - we control the entire
-	 * lifetime of the block device and write directly to it.
+	 * BLKFLSBUF will sync the filesystem on top of the device (we
+	 * don't care about that here, since we write directly to it),
+	 * write out any dirty buffers and invalidate the buffer cache.
+	 * It won't do a hardware cache flush.
+	 *
+	 * fsync() will write out any dirty buffers and do a hardware
+	 * cache flush (which we don't care about either, because for
+	 * krbd it's a noop).  It won't try to empty the buffer cache
+	 * nor poke the filesystem before writing out.
+	 *
+	 * Given that, for our purposes, fsync is a flush, while
+	 * BLKFLSBUF is a flush+invalidate.
 	 */
-	if (fsync(ctx->krbd_fd) < 0) {
-		int ret = -errno;
-		prt("fsync failed\n");
+        if (invalidate)
+		ret = ioctl(ctx->krbd_fd, BLKFLSBUF, NULL);
+	else
+		ret = fsync(ctx->krbd_fd);
+	if (ret < 0) {
+		ret = -errno;
+		prt("%s failed\n", invalidate ? "BLKFLSBUF" : "fsync");
 		return ret;
 	}
 
@@ -650,7 +665,7 @@ __krbd_flush(struct rbd_ctx *ctx)
 int
 krbd_flush(struct rbd_ctx *ctx)
 {
-	return __krbd_flush(ctx);
+	return __krbd_flush(ctx, false);
 }
 
 int
@@ -660,16 +675,22 @@ krbd_discard(struct rbd_ctx *ctx, uint64_t off, uint64_t len)
 	int ret;
 
 	/*
-	 * BLKDISCARD doesn't affect dirty pages.  This means we can't
-	 * rely on discarded sectors to match good_buf (i.e. contain
-	 * zeros) without a preceding cache flush:
+	 * BLKDISCARD goes straight to disk and doesn't do anything
+	 * about dirty buffers.  This means we need to flush so that
 	 *
 	 *   write 0..3M
 	 *   discard 1..2M
 	 *
-	 * results in "data data data" rather than "data 0000 data".
+	 * results in "data 0000 data" rather than "data data data" on
+	 * disk and invalidate so that
+	 *
+	 *   discard 1..2M
+	 *   read 0..3M
+	 *
+	 * returns "data 0000 data" rather than "data data data" in
+	 * case 1..2M was cached.
 	 */
-	ret = __krbd_flush(ctx);
+	ret = __krbd_flush(ctx, true);
 	if (ret < 0)
 		return ret;
 
@@ -715,16 +736,16 @@ krbd_resize(struct rbd_ctx *ctx, uint64_t size)
 	 * which ends up calling invalidate_bdev(), which invalidates
 	 * clean pages and does nothing about dirty pages beyond the
 	 * new size.  The preceding cache flush makes sure those pages
-	 * are invalidated, which is what we need on shrink:
+	 * are invalidated, which is what we need on shrink so that
 	 *
 	 *  write 0..1M
 	 *  resize 0
 	 *  resize 2M
-	 *  write 1..2M
+	 *  read 0..2M
 	 *
-	 * results in "data data" rather than "0000 data".
+	 * returns "0000 0000" rather than "data 0000".
 	 */
-	ret = __krbd_flush(ctx);
+	ret = __krbd_flush(ctx, false);
 	if (ret < 0)
 		return ret;
 
@@ -738,7 +759,7 @@ krbd_clone(struct rbd_ctx *ctx, const char *src_snapname,
 {
 	int ret;
 
-	ret = __krbd_flush(ctx);
+	ret = __krbd_flush(ctx, false);
 	if (ret < 0)
 		return ret;
 
@@ -751,7 +772,7 @@ krbd_flatten(struct rbd_ctx *ctx)
 {
 	int ret;
 
-	ret = __krbd_flush(ctx);
+	ret = __krbd_flush(ctx, false);
 	if (ret < 0)
 		return ret;