[PATCH 12/16] drbd: fix race when forcefully disconnecting

[Philipp Reisner]

From: Lars Ellenberg <lars.ellenberg@xxxxxxxxxx>

If a forced disconnect hits a restarting receiver right after it passed
its final "if (C_DISCONNECTING)" test in drbdd_init(), but before it was
actually restarted by drbd_thread_setup, we could be left with a
connection stuck in C_DISCONNECTING, never reaching C_STANDALONE,
which would be necessary to take it down or reconfigure it.

Move the last cleanup into w_after_conn_state_ch(), and do an additional
state change request in conn_try_disconnect(), just in case.

Signed-off-by: Philipp Reisner <philipp.reisner@xxxxxxxxxx>
Signed-off-by: Lars Ellenberg <lars.ellenberg@xxxxxxxxxx>
---
 drivers/block/drbd/drbd_nl.c       |   85 ++++++++++++++++++++----------------
 drivers/block/drbd/drbd_receiver.c |   14 +------
 drivers/block/drbd/drbd_state.c    |   13 ++++++
 3 files changed, 62 insertions(+), 50 deletions(-)

diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index 0bf74b1..455b9a9 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -2160,37 +2160,54 @@ out:
 static enum drbd_state_rv conn_try_disconnect(struct drbd_tconn *tconn, bool force)
 {
 	enum drbd_state_rv rv;
-	if (force) {
-		spin_lock_irq(&tconn->req_lock);
-		rv = _conn_request_state(tconn, NS(conn, C_DISCONNECTING), CS_HARD);
-		spin_unlock_irq(&tconn->req_lock);
-		return rv;
-	}
 
-	rv = conn_request_state(tconn, NS(conn, C_DISCONNECTING), 0);
+	rv = conn_request_state(tconn, NS(conn, C_DISCONNECTING),
+			force ? CS_HARD : 0);
 
 	switch (rv) {
 	case SS_NOTHING_TO_DO:
+		break;
 	case SS_ALREADY_STANDALONE:
 		return SS_SUCCESS;
 	case SS_PRIMARY_NOP:
 		/* Our state checking code wants to see the peer outdated. */
 		rv = conn_request_state(tconn, NS2(conn, C_DISCONNECTING,
-							pdsk, D_OUTDATED), CS_VERBOSE);
+						pdsk, D_OUTDATED), CS_VERBOSE);
 		break;
 	case SS_CW_FAILED_BY_PEER:
 		/* The peer probably wants to see us outdated. */
 		rv = conn_request_state(tconn, NS2(conn, C_DISCONNECTING,
 							disk, D_OUTDATED), 0);
 		if (rv == SS_IS_DISKLESS || rv == SS_LOWER_THAN_OUTDATED) {
-			conn_request_state(tconn, NS(conn, C_DISCONNECTING), CS_HARD);
-			rv = SS_SUCCESS;
+			rv = conn_request_state(tconn, NS(conn, C_DISCONNECTING),
+					CS_HARD);
 		}
 		break;
 	default:;
 		/* no special handling necessary */
 	}
 
+	if (rv >= SS_SUCCESS) {
+		enum drbd_state_rv rv2;
+		/* No one else can reconfigure the network while I am here.
+		 * The state handling only uses drbd_thread_stop_nowait(),
+		 * we want to really wait here until the receiver is no more.
+		 */
+		drbd_thread_stop(&adm_ctx.tconn->receiver);
+
+		/* Race breaker.  This additional state change request may be
+		 * necessary, if this was a forced disconnect during a receiver
+		 * restart.  We may have "killed" the receiver thread just
+		 * after drbdd_init() returned.  Typically, we should be
+		 * C_STANDALONE already, now, and this becomes a no-op.
+		 */
+		rv2 = conn_request_state(tconn, NS(conn, C_STANDALONE),
+				CS_VERBOSE | CS_HARD);
+		if (rv2 < SS_SUCCESS)
+			conn_err(tconn,
+				"unexpected rv2=%d in conn_try_disconnect()\n",
+				rv2);
+	}
 	return rv;
 }
 
@@ -2221,19 +2238,9 @@ int drbd_adm_disconnect(struct sk_buff *skb, struct genl_info *info)
 
 	rv = conn_try_disconnect(tconn, parms.force_disconnect);
 	if (rv < SS_SUCCESS)
-		goto fail;
-
-	/* No one else can reconfigure the network while I am here.
-	 * The state handling only uses drbd_thread_stop_nowait(),
-	 * we want to really wait here until the receiver is no more. */
-	drbd_thread_stop(&tconn->receiver);
-	if (wait_event_interruptible(tconn->ping_wait,
-				     tconn->cstate == C_STANDALONE)) {
-		retcode = ERR_INTR;
-		goto fail;
-	}
-
-	retcode = NO_ERROR;
+		retcode = rv;  /* FIXME: Type mismatch. */
+	else
+		retcode = NO_ERROR;
  fail:
 	drbd_adm_finish(info, retcode);
 	return 0;
@@ -3095,8 +3102,7 @@ out:
 
 int drbd_adm_down(struct sk_buff *skb, struct genl_info *info)
 {
-	enum drbd_ret_code retcode;
-	enum drbd_state_rv rv;
+	int retcode; /* enum drbd_ret_code rsp. enum drbd_state_rv */
 	struct drbd_conf *mdev;
 	unsigned i;
 
@@ -3120,30 +3126,35 @@ int drbd_adm_down(struct sk_buff *skb, struct genl_info *info)
 			goto out_unlock;
 		}
 	}
+	up_read(&drbd_cfg_rwsem);
 
-	/* disconnect */
-	rv = conn_try_disconnect(adm_ctx.tconn, 0);
-	if (rv < SS_SUCCESS) {
-		retcode = rv; /* enum type mismatch! */
+	/* disconnect; may stop the receiver;
+	 * must not hold the drbd_cfg_rwsem */
+	retcode = conn_try_disconnect(adm_ctx.tconn, 0);
+	if (retcode < SS_SUCCESS) {
 		drbd_msg_put_info("failed to disconnect");
-		goto out_unlock;
+		goto out;
 	}
 
-	/* Make sure the network threads have actually stopped,
-	 * state handling only does drbd_thread_stop_nowait(). */
-	drbd_thread_stop(&adm_ctx.tconn->receiver);
-
+	down_read(&drbd_cfg_rwsem);
 	/* detach */
 	idr_for_each_entry(&adm_ctx.tconn->volumes, mdev, i) {
-		rv = adm_detach(mdev);
-		if (rv < SS_SUCCESS) {
-			retcode = rv; /* enum type mismatch! */
+		retcode = adm_detach(mdev);
+		if (retcode < SS_SUCCESS) {
 			drbd_msg_put_info("failed to detach");
 			goto out_unlock;
 		}
 	}
 	up_read(&drbd_cfg_rwsem);
 
+	/* If we reach this, all volumes (of this tconn) are Secondary,
+	 * Disconnected, Diskless, aka Unconfigured. Make sure all threads have
+	 * actually stopped, state handling only does drbd_thread_stop_nowait().
+	 * This needs to be done without holding drbd_cfg_rwsem. */
+	drbd_thread_stop(&adm_ctx.tconn->worker);
+
+	/* Now, nothing can fail anymore */
+
 	/* delete volumes */
 	down_write(&drbd_cfg_rwsem);
 	idr_for_each_entry(&adm_ctx.tconn->volumes, mdev, i) {
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 9c8bcce..956cdda 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -4213,20 +4213,8 @@ static void drbd_disconnect(struct drbd_tconn *tconn)
 
 	spin_unlock_irq(&tconn->req_lock);
 
-	if (oc == C_DISCONNECTING) {
-		struct net_conf *old_conf;
-
-		mutex_lock(&tconn->net_conf_update);
-		old_conf = tconn->net_conf;
-		rcu_assign_pointer(tconn->net_conf, NULL);
-		conn_free_crypto(tconn);
-		mutex_unlock(&tconn->net_conf_update);
-
-		synchronize_rcu();
-		kfree(old_conf);
-
+	if (oc == C_DISCONNECTING)
 		conn_request_state(tconn, NS(conn, C_STANDALONE), CS_VERBOSE | CS_HARD);
-	}
 }
 
 static int drbd_disconnected(int vnr, void *p, void *data)
diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index 978dcef..f2e1535 100644
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -1398,6 +1398,19 @@ static int w_after_conn_state_ch(struct drbd_work *w, int unused)
 	if (oc == C_STANDALONE && ns_max.conn == C_UNCONNECTED)
 		drbd_thread_start(&tconn->receiver);
 
+	if (oc == C_DISCONNECTING && ns_max.conn == C_STANDALONE) {
+		struct net_conf *old_conf;
+
+		mutex_lock(&tconn->net_conf_update);
+		old_conf = tconn->net_conf;
+		rcu_assign_pointer(tconn->net_conf, NULL);
+		conn_free_crypto(tconn);
+		mutex_unlock(&tconn->net_conf_update);
+
+		synchronize_rcu();
+		kfree(old_conf);
+	}
+
 	if (ns_max.susp_fen) {
 		/* case1: The outdate peer handler is successful: */
 		if (ns_max.pdsk <= D_OUTDATED) {
-- 
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/