drbd: allow bitmap to change during writeout from resync_finished

Symptom: messages similar to
 "FIXME asender in bm_change_bits_to,
  bitmap locked for 'write from resync_finished' by worker"

If a resync or verify is finished (or aborted), a full bitmap writeout
is triggered.  If we have ongoing local IO, the bitmap may still change
during that writeout, pending and not yet processed acks may cause bits
to be cleared, while new writes may cause bits to be to be set.

To fix this, introduce the drbd_bm_write_copy_pages() variant.

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>

diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
index 49603bc67fe4fa78e60fb7b415b240bee29c13b1..9dab3700ca2d0e75915415e0278bea833a2fe7b4 100644 (file)
--- a/drivers/block/drbd/drbd_bitmap.c
+++ b/drivers/block/drbd/drbd_bitmap.c
@@ -1011,7 +1011,7 @@ static void bm_page_io_async(struct bm_aio_ctx *ctx, int page_nr, int rw) __must
 /*
  * bm_rw: read/write the whole bitmap from/to its on disk location.
  */
-static int bm_rw(struct drbd_conf *mdev, int rw, unsigned lazy_writeout_upper_idx) __must_hold(local)
+static int bm_rw(struct drbd_conf *mdev, int rw, unsigned flags, unsigned lazy_writeout_upper_idx) __must_hold(local)
 {
        struct bm_aio_ctx *ctx;
        struct drbd_bitmap *b = mdev->bitmap;
@@ -1037,7 +1037,7 @@ static int bm_rw(struct drbd_conf *mdev, int rw, unsigned lazy_writeout_upper_id
                .mdev = mdev,
                .in_flight = ATOMIC_INIT(1),
                .done = 0,
-               .flags = lazy_writeout_upper_idx ? BM_AIO_COPY_PAGES : 0,
+               .flags = flags,
                .error = 0,
                .kref = { ATOMIC_INIT(2) },
        };
@@ -1128,7 +1128,7 @@ static int bm_rw(struct drbd_conf *mdev, int rw, unsigned lazy_writeout_upper_id
  */
 int drbd_bm_read(struct drbd_conf *mdev) __must_hold(local)
 {
-       return bm_rw(mdev, READ, 0);
+       return bm_rw(mdev, READ, 0, 0);
 }
 
 /**
@@ -1139,7 +1139,7 @@ int drbd_bm_read(struct drbd_conf *mdev) __must_hold(local)
  */
 int drbd_bm_write(struct drbd_conf *mdev) __must_hold(local)
 {
-       return bm_rw(mdev, WRITE, 0);
+       return bm_rw(mdev, WRITE, 0, 0);
 }
 
 /**
@@ -1149,7 +1149,23 @@ int drbd_bm_write(struct drbd_conf *mdev) __must_hold(local)
  */
 int drbd_bm_write_lazy(struct drbd_conf *mdev, unsigned upper_idx) __must_hold(local)
 {
-       return bm_rw(mdev, WRITE, upper_idx);
+       return bm_rw(mdev, WRITE, BM_AIO_COPY_PAGES, upper_idx);
+}
+
+/**
+ * drbd_bm_write_copy_pages() - Write the whole bitmap to its on disk location.
+ * @mdev:      DRBD device.
+ *
+ * Will only write pages that have changed since last IO.
+ * In contrast to drbd_bm_write(), this will copy the bitmap pages
+ * to temporary writeout pages. It is intended to trigger a full write-out
+ * while still allowing the bitmap to change, for example if a resync or online
+ * verify is aborted due to a failed peer disk, while local IO continues, or
+ * pending resync acks are still being processed.
+ */
+int drbd_bm_write_copy_pages(struct drbd_conf *mdev) __must_hold(local)
+{
+       return bm_rw(mdev, WRITE, BM_AIO_COPY_PAGES, 0);
 }
 
 

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index 302a6e786f76906ceb2c48a3707254cb9418b566..6586053429b47e7f2c3480268629d7191131b97f 100644 (file)
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -862,22 +862,28 @@ enum bm_flag {
        BM_P_VMALLOCED = 0x10000, /* internal use only, will be masked out */
 
        /* currently locked for bulk operation */
-       BM_LOCKED_MASK = 0x7,
+       BM_LOCKED_MASK = 0xf,
 
        /* in detail, that is: */
        BM_DONT_CLEAR = 0x1,
        BM_DONT_SET   = 0x2,
        BM_DONT_TEST  = 0x4,
 
+       /* so we can mark it locked for bulk operation,
+        * and still allow all non-bulk operations */
+       BM_IS_LOCKED  = 0x8,
+
        /* (test bit, count bit) allowed (common case) */
-       BM_LOCKED_TEST_ALLOWED = 0x3,
+       BM_LOCKED_TEST_ALLOWED = BM_DONT_CLEAR | BM_DONT_SET | BM_IS_LOCKED,
 
        /* testing bits, as well as setting new bits allowed, but clearing bits
         * would be unexpected.  Used during bitmap receive.  Setting new bits
         * requires sending of "out-of-sync" information, though. */
-       BM_LOCKED_SET_ALLOWED = 0x1,
+       BM_LOCKED_SET_ALLOWED = BM_DONT_CLEAR | BM_IS_LOCKED,
 
-       /* clear is not expected while bitmap is locked for bulk operation */
+       /* for drbd_bm_write_copy_pages, everything is allowed,
+        * only concurrent bulk operations are locked out. */
+       BM_LOCKED_CHANGE_ALLOWED = BM_IS_LOCKED,
 };
 
 struct drbd_work_queue {
@@ -1457,6 +1463,7 @@ extern int  drbd_bm_e_weight(struct drbd_conf *mdev, unsigned long enr);
 extern int  drbd_bm_write_page(struct drbd_conf *mdev, unsigned int idx) __must_hold(local);
 extern int  drbd_bm_read(struct drbd_conf *mdev) __must_hold(local);
 extern int  drbd_bm_write(struct drbd_conf *mdev) __must_hold(local);
+extern int  drbd_bm_write_copy_pages(struct drbd_conf *mdev) __must_hold(local);
 extern unsigned long drbd_bm_ALe_set_all(struct drbd_conf *mdev,
                unsigned long al_enr);
 extern size_t       drbd_bm_words(struct drbd_conf *mdev);

diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index ab501b23b50e1cd16afc76c3fdad95c991f3797c..d830116781f9ba3f152b40755d3ea6b96b75bcf8 100644 (file)
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -1698,8 +1698,8 @@ static void after_state_ch(struct drbd_conf *mdev, union drbd_state os,
         * No harm done if some bits change during this phase.
         */
        if (os.conn > C_CONNECTED && ns.conn <= C_CONNECTED && get_ldev(mdev)) {
-               drbd_queue_bitmap_io(mdev, &drbd_bm_write, NULL,
-                       "write from resync_finished", BM_LOCKED_SET_ALLOWED);
+               drbd_queue_bitmap_io(mdev, &drbd_bm_write_copy_pages, NULL,
+                       "write from resync_finished", BM_LOCKED_CHANGE_ALLOWED);
                put_ldev(mdev);
        }