drbd: differentiate between normal and forced detach

Aborting local requests (not waiting for completion from the lower level
disk) is dangerous: if the master bio has been completed to upper
layers, data pages may be re-used for other things already.
If local IO is still pending and later completes,
this may cause crashes or corrupt unrelated data.

Only abort local IO if explicitly requested.
Intended use case is a lower level device that turned into a tarpit,
not completing io requests, not even doing error completion.

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_actlog.c b/drivers/block/drbd/drbd_actlog.c
index f500dc5cdf52cd35baeda5e27f9eb6c843938c0e..209b2e063b9230da32e0b73d5458d1fe0a6de0cd 100644 (file)
--- a/drivers/block/drbd/drbd_actlog.c
+++ b/drivers/block/drbd/drbd_actlog.c
@@ -445,7 +445,7 @@ _al_write_transaction(struct drbd_conf *mdev)
                /* drbd_chk_io_error done already */
        else if (drbd_md_sync_page_io(mdev, mdev->ldev, sector, WRITE)) {
                err = -EIO;
-               drbd_chk_io_error(mdev, 1, true);
+               drbd_chk_io_error(mdev, 1, DRBD_META_IO_ERROR);
        } else {
                /* advance ringbuffer position and transaction counter */
                mdev->al_tr_pos = (mdev->al_tr_pos + 1) % (MD_AL_SECTORS*512/MD_BLOCK_SIZE);

diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
index b3d55d4b6937fbe97295a2d12073ee12ad2e120d..33626e34c92aeaf80121b5b943a3e359fe94535d 100644 (file)
--- a/drivers/block/drbd/drbd_bitmap.c
+++ b/drivers/block/drbd/drbd_bitmap.c
@@ -1135,7 +1135,7 @@ static int bm_rw(struct drbd_conf *mdev, int rw, unsigned flags, unsigned lazy_w
 
        if (ctx->error) {
                dev_alert(DEV, "we had at least one MD IO ERROR during bitmap IO\n");
-               drbd_chk_io_error(mdev, 1, true);
+               drbd_chk_io_error(mdev, 1, DRBD_META_IO_ERROR);
                err = -EIO; /* ctx->error ? */
        }
 
@@ -1260,7 +1260,7 @@ int drbd_bm_write_page(struct drbd_conf *mdev, unsigned int idx) __must_hold(loc
        wait_until_done_or_disk_failure(mdev, mdev->ldev, &ctx->done);
 
        if (ctx->error)
-               drbd_chk_io_error(mdev, 1, true);
+               drbd_chk_io_error(mdev, 1, DRBD_META_IO_ERROR);
                /* that should force detach, so the in memory bitmap will be
                 * gone in a moment as well. */
 

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index b343875c9deee9645bef2551149ce535d9c36622..963766bafab491c467f8104d7854b267c59feb36 100644 (file)
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -689,6 +689,7 @@ enum {
        BITMAP_IO_QUEUED,       /* Started bitmap IO */
        GO_DISKLESS,            /* Disk is being detached, on io-error or admin request. */
        WAS_IO_ERROR,           /* Local disk failed returned IO error */
+       FORCE_DETACH,           /* Force-detach from local disk, aborting any pending local IO */
        RESYNC_AFTER_NEG,       /* Resync after online grow after the attach&negotiate finished. */
        RESIZE_PENDING,         /* Size change detected locally, waiting for the response from
                                 * the peer, if it changed there as well. */
@@ -1653,8 +1654,16 @@ static inline union drbd_state drbd_read_state(struct drbd_conf *mdev)
        return rv;
 }
 
+enum drbd_force_detach_flags {
+       DRBD_IO_ERROR,
+       DRBD_META_IO_ERROR,
+       DRBD_FORCE_DETACH,
+};
+
 #define __drbd_chk_io_error(m,f) __drbd_chk_io_error_(m,f, __func__)
-static inline void __drbd_chk_io_error_(struct drbd_conf *mdev, int forcedetach, const char *where)
+static inline void __drbd_chk_io_error_(struct drbd_conf *mdev,
+               enum drbd_force_detach_flags forcedetach,
+               const char *where)
 {
        enum drbd_io_error_p ep;
 
@@ -1663,7 +1672,7 @@ static inline void __drbd_chk_io_error_(struct drbd_conf *mdev, int forcedetach,
        rcu_read_unlock();
        switch (ep) {
        case EP_PASS_ON: /* FIXME would this be better named "Ignore"? */
-               if (!forcedetach) {
+               if (forcedetach == DRBD_IO_ERROR) {
                        if (__ratelimit(&drbd_ratelimit_state))
                                dev_err(DEV, "Local IO failed in %s.\n", where);
                        if (mdev->state.disk > D_INCONSISTENT)
@@ -1674,6 +1683,8 @@ static inline void __drbd_chk_io_error_(struct drbd_conf *mdev, int forcedetach,
        case EP_DETACH:
        case EP_CALL_HELPER:
                set_bit(WAS_IO_ERROR, &mdev->flags);
+               if (forcedetach == DRBD_FORCE_DETACH)
+                       set_bit(FORCE_DETACH, &mdev->flags);
                if (mdev->state.disk > D_FAILED) {
                        _drbd_set_state(_NS(mdev, disk, D_FAILED), CS_HARD, NULL);
                        dev_err(DEV,
@@ -1693,7 +1704,7 @@ static inline void __drbd_chk_io_error_(struct drbd_conf *mdev, int forcedetach,
  */
 #define drbd_chk_io_error(m,e,f) drbd_chk_io_error_(m,e,f, __func__)
 static inline void drbd_chk_io_error_(struct drbd_conf *mdev,
-       int error, int forcedetach, const char *where)
+       int error, enum drbd_force_detach_flags forcedetach, const char *where)
 {
        if (error) {
                unsigned long flags;

diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index c0acd86c84153185cfc8c18716a25dffe0928d65..849e5de9ea8fd6c4956b08da56008a7ecf44b284 100644 (file)
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -2866,7 +2866,7 @@ void drbd_md_sync(struct drbd_conf *mdev)
        if (drbd_md_sync_page_io(mdev, mdev->ldev, sector, WRITE)) {
                /* this was a try anyways ... */
                dev_err(DEV, "meta data update failed!\n");
-               drbd_chk_io_error(mdev, 1, true);
+               drbd_chk_io_error(mdev, 1, DRBD_META_IO_ERROR);
        }
 
        /* Update mdev->ldev->md.la_size_sect,

diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index 323293e88878876c3bf553b7505498c731ed59b6..d4c05e26a13a743a7bf1929cf961c8c645796054 100644 (file)
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -1299,6 +1299,9 @@ int drbd_adm_attach(struct sk_buff *skb, struct genl_info *info)
         * to realize a "hot spare" feature (not that I'd recommend that) */
        wait_event(mdev->misc_wait, !atomic_read(&mdev->local_cnt));
 
+       /* make sure there is no leftover from previous force-detach attempts */
+       clear_bit(FORCE_DETACH, &mdev->flags);
+
        /* allocation not in the IO path, drbdsetup context */
        nbc = kzalloc(sizeof(struct drbd_backing_dev), GFP_KERNEL);
        if (!nbc) {
@@ -1683,6 +1686,7 @@ static int adm_detach(struct drbd_conf *mdev, int force)
        int ret;
 
        if (force) {
+               set_bit(FORCE_DETACH, &mdev->flags);
                drbd_force_state(mdev, NS(disk, D_FAILED));
                retcode = SS_SUCCESS;
                goto out;

diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
index 891c3d41a277093ff44f8a967acf8e7a8400570d..e215dce4c6941241315abe02527c1dcb3798b08c 100644 (file)
--- a/drivers/block/drbd/drbd_req.c
+++ b/drivers/block/drbd/drbd_req.c
@@ -490,7 +490,7 @@ int __req_mod(struct drbd_request *req, enum drbd_req_event what,
                drbd_set_out_of_sync(mdev, req->i.sector, req->i.size);
                /* fall through. */
        case WRITE_COMPLETED_WITH_ERROR:
-               __drbd_chk_io_error(mdev, false);
+               __drbd_chk_io_error(mdev, DRBD_IO_ERROR);
                /* fall through. */
        case READ_AHEAD_COMPLETED_WITH_ERROR:
                /* it is legal to fail READA, no __drbd_chk_io_error in that case. */
@@ -1210,7 +1210,7 @@ void request_timer_fn(unsigned long data)
                 time_after(now, req->start_time + dt) &&
                !time_in_range(now, mdev->last_reattach_jif, mdev->last_reattach_jif + dt)) {
                dev_warn(DEV, "Local backing device failed to meet the disk-timeout\n");
-               __drbd_chk_io_error(mdev, 1);
+               __drbd_chk_io_error(mdev, DRBD_FORCE_DETACH);
        }
        nt = (time_after(now, req->start_time + et) ? now : req->start_time) + et;
        spin_unlock_irq(&tconn->req_lock);

diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index 84a5072d7370705235e7101c2160071f50893141..c9ec7d37632c8ab06480faed4a3e25d1ecad34b7 100644 (file)
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -1335,9 +1335,21 @@ static void after_state_ch(struct drbd_conf *mdev, union drbd_state os,
 
                        was_io_error = test_and_clear_bit(WAS_IO_ERROR, &mdev->flags);
 
-                       /* Immediately allow completion of all application IO, that waits
-                          for completion from the local disk. */
-                       tl_abort_disk_io(mdev);
+                       /* Immediately allow completion of all application IO,
+                        * that waits for completion from the local disk,
+                        * if this was a force-detach due to disk_timeout
+                        * or administrator request (drbdsetup detach --force).
+                        * Do NOT abort otherwise.
+                        * Aborting local requests may cause serious problems,
+                        * if requests are completed to upper layers already,
+                        * and then later the already submitted local bio completes.
+                        * This can cause DMA into former bio pages that meanwhile
+                        * have been re-used for other things.
+                        * So aborting local requests may cause crashes,
+                        * or even worse, silent data corruption.
+                        */
+                       if (test_and_clear_bit(FORCE_DETACH, &mdev->flags))
+                               tl_abort_disk_io(mdev);
 
                        /* current state still has to be D_FAILED,
                         * there is only one way out: to D_DISKLESS,

diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index 66be3910e8d2b7d67fe27f2687c631c7900752ea..07a4046dd8c34a55adcdfa97020ca13c2a9c9e10 100644 (file)
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -106,7 +106,7 @@ void drbd_endio_read_sec_final(struct drbd_peer_request *peer_req) __releases(lo
        if (list_empty(&mdev->read_ee))
                wake_up(&mdev->ee_wait);
        if (test_bit(__EE_WAS_ERROR, &peer_req->flags))
-               __drbd_chk_io_error(mdev, false);
+               __drbd_chk_io_error(mdev, DRBD_IO_ERROR);
        spin_unlock_irqrestore(&mdev->tconn->req_lock, flags);
 
        drbd_queue_work(&mdev->tconn->sender_work, &peer_req->w);
@@ -148,7 +148,7 @@ static void drbd_endio_write_sec_final(struct drbd_peer_request *peer_req) __rel
        do_wake = list_empty(block_id == ID_SYNCER ? &mdev->sync_ee : &mdev->active_ee);
 
        if (test_bit(__EE_WAS_ERROR, &peer_req->flags))
-               __drbd_chk_io_error(mdev, false);
+               __drbd_chk_io_error(mdev, DRBD_IO_ERROR);
        spin_unlock_irqrestore(&mdev->tconn->req_lock, flags);
 
        if (block_id == ID_SYNCER)