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>

diff --git a/src/test/librbd/fsx.cc b/src/test/librbd/fsx.cc
index 40a908f63087a3a71c68616eebb2ee90a5f3b3a2..9eea49cbfe3520dd1c5adfa48fb71d4d2e9fb1f7 100644 (file)
--- 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;