[RFC 00/10] drbd: part 13 of adding multiple volume support to drbd

[RFC 00/10] drbd: part 13 of adding multiple volume support to drbd

[Philipp Reisner]

This the first request for review of drbd-8.4. The complete set has 
464 patches. This is the thirteenth installment containing 10 patches.

The whole set is available here:
  git://git.drbd.org/linux-2.6-drbd.git for-jens

and is jens_for-3.2_drivers...for-jens
and this part is ab7e97f^...7afb8a0

The most interesting part of these patches is the introduction of 
reference counting for drbd_tconn objects.

[PATCH 01/10] drbd: allow status dump request all volumes of a specific resource

[Philipp Reisner]

From: Lars Ellenberg <lars.ellenberg@linbit.com>

We had drbd_adm_get_status (one single volume),
and drbd_adm_get_status_all (dump of all volumes of all resources).

This enhances the latter to be able to dump all volumes
of just one specific resource.

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
---
 drivers/block/drbd/drbd_nl.c |   70 +++++++++++++++++++++++++++++++++++++++--
 1 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index 77c0249..0b6b5d8 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -2647,7 +2647,7 @@ out:
 	return 0;
 }
 
-int drbd_adm_get_status_all(struct sk_buff *skb, struct netlink_callback *cb)
+int get_one_status(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct drbd_conf *mdev;
 	struct drbd_genlmsghdr *dh;
@@ -2665,6 +2665,9 @@ int drbd_adm_get_status_all(struct sk_buff *skb, struct netlink_callback *cb)
 	 * where tconn is cb->args[0];
 	 * and i is cb->args[1];
 	 *
+	 * cb->args[2] indicates if we shall loop over all resources,
+	 * or just dump all volumes of a single resource.
+	 *
 	 * This may miss entries inserted after this dump started,
 	 * or entries deleted before they are reached.
 	 *
@@ -2675,7 +2678,6 @@ int drbd_adm_get_status_all(struct sk_buff *skb, struct netlink_callback *cb)
 
 	/* synchronize with drbd_new_tconn/drbd_free_tconn */
 	down_read(&drbd_cfg_rwsem);
-next_tconn:
 	/* revalidate iterator position */
 	list_for_each_entry(tmp, &drbd_tconns, all_tconn) {
 		if (pos == NULL) {
@@ -2690,16 +2692,22 @@ next_tconn:
 		}
 	}
 	if (tconn) {
+next_tconn:
 		mdev = idr_get_next(&tconn->volumes, &volume);
 		if (!mdev) {
 			/* No more volumes to dump on this tconn.
 			 * Advance tconn iterator. */
 			pos = list_entry(tconn->all_tconn.next,
 					struct drbd_tconn, all_tconn);
-			/* But, did we dump any volume on this tconn yet? */
+			/* Did we dump any volume on this tconn yet? */
 			if (volume != 0) {
-				tconn = NULL;
+				/* If we reached the end of the list,
+				 * or only a single resource dump was requested,
+				 * we are done. */
+				if (&pos->all_tconn == &drbd_tconns || cb->args[2])
+					goto out;
 				volume = 0;
+				tconn = pos;
 				goto next_tconn;
 			}
 		}
@@ -2747,6 +2755,60 @@ out:
         return skb->len;
 }
 
+/*
+ * Request status of all resources, or of all volumes within a single resource.
+ *
+ * This is a dump, as the answer may not fit in a single reply skb otherwise.
+ * Which means we cannot use the family->attrbuf or other such members, because
+ * dump is NOT protected by the genl_lock().  During dump, we only have access
+ * to the incoming skb, and need to opencode "parsing" of the nlattr payload.
+ *
+ * Once things are setup properly, we call into get_one_status().
+ */
+int drbd_adm_get_status_all(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	const unsigned hdrlen = GENL_HDRLEN + GENL_MAGIC_FAMILY_HDRSZ;
+	struct nlattr *nla;
+	const char *conn_name;
+	struct drbd_tconn *tconn;
+
+	/* Is this a followup call? */
+	if (cb->args[0]) {
+		/* ... of a single resource dump,
+		 * and the resource iterator has been advanced already? */
+		if (cb->args[2] && cb->args[2] != cb->args[0])
+			return 0; /* DONE. */
+		goto dump;
+	}
+
+	/* First call (from netlink_dump_start).  We need to figure out
+	 * which resource(s) the user wants us to dump. */
+	nla = nla_find(nlmsg_attrdata(cb->nlh, hdrlen),
+			nlmsg_attrlen(cb->nlh, hdrlen),
+			DRBD_NLA_CFG_CONTEXT);
+
+	/* No explicit context given.  Dump all. */
+	if (!nla)
+		goto dump;
+	nla = nla_find_nested(nla, __nla_type(T_ctx_conn_name));
+	/* context given, but no name present? */
+	if (!nla)
+		return -EINVAL;
+	conn_name = nla_data(nla);
+	tconn = conn_by_name(conn_name);
+	if (!tconn)
+		return -ENODEV;
+
+	/* prime iterators, and set "filter" mode mark:
+	 * only dump this tconn. */
+	cb->args[0] = (long)tconn;
+	/* cb->args[1] = 0; passed in this way. */
+	cb->args[2] = (long)tconn;
+
+dump:
+	return get_one_status(skb, cb);
+}
+
 int drbd_adm_get_timeout_type(struct sk_buff *skb, struct genl_info *info)
 {
 	enum drbd_ret_code retcode;
-- 
1.7.4.1

[PATCH 02/10] drbd: fix setsockopt for user mode linux

[Philipp Reisner]

From: Lars Ellenberg <lars.ellenberg@linbit.com>

We use our own copy of kernel_setsockopt, and did not mess around with
get_fs/set_fs, since we thought we knew we would always be KERNEL_DS
anyways. Apparently not so for at least user mode linux, so put the
set_fs(KERNEL_DS) in there.

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
---
 drivers/block/drbd/drbd_int.h |   33 ++++++++++++++++++++-------------
 1 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index 3833d56..3080192 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -1484,46 +1484,53 @@ static inline void drbd_flush_workqueue(struct drbd_conf *mdev)
 	conn_flush_workqueue(mdev->tconn);
 }
 
-/* yes, there is kernel_setsockopt, but only since 2.6.18. we don't need to
- * mess with get_fs/set_fs, we know we are KERNEL_DS always. */
+/* Yes, there is kernel_setsockopt, but only since 2.6.18.
+ * So we have our own copy of it here. */
 static inline int drbd_setsockopt(struct socket *sock, int level, int optname,
-			char __user *optval, int optlen)
+				  char *optval, int optlen)
 {
+	mm_segment_t oldfs = get_fs();
+	char __user *uoptval;
 	int err;
+
+	uoptval = (char __user __force *)optval;
+
+	set_fs(KERNEL_DS);
 	if (level == SOL_SOCKET)
-		err = sock_setsockopt(sock, level, optname, optval, optlen);
+		err = sock_setsockopt(sock, level, optname, uoptval, optlen);
 	else
-		err = sock->ops->setsockopt(sock, level, optname, optval,
+		err = sock->ops->setsockopt(sock, level, optname, uoptval,
 					    optlen);
+	set_fs(oldfs);
 	return err;
 }
 
 static inline void drbd_tcp_cork(struct socket *sock)
 {
-	int __user val = 1;
+	int val = 1;
 	(void) drbd_setsockopt(sock, SOL_TCP, TCP_CORK,
-			(char __user *)&val, sizeof(val));
+			(char*)&val, sizeof(val));
 }
 
 static inline void drbd_tcp_uncork(struct socket *sock)
 {
-	int __user val = 0;
+	int val = 0;
 	(void) drbd_setsockopt(sock, SOL_TCP, TCP_CORK,
-			(char __user *)&val, sizeof(val));
+			(char*)&val, sizeof(val));
 }
 
 static inline void drbd_tcp_nodelay(struct socket *sock)
 {
-	int __user val = 1;
+	int val = 1;
 	(void) drbd_setsockopt(sock, SOL_TCP, TCP_NODELAY,
-			(char __user *)&val, sizeof(val));
+			(char*)&val, sizeof(val));
 }
 
 static inline void drbd_tcp_quickack(struct socket *sock)
 {
-	int __user val = 2;
+	int val = 2;
 	(void) drbd_setsockopt(sock, SOL_TCP, TCP_QUICKACK,
-			(char __user *)&val, sizeof(val));
+			(char*)&val, sizeof(val));
 }
 
 void drbd_bump_write_ordering(struct drbd_conf *mdev, enum write_ordering_e wo);
-- 
1.7.4.1

[PATCH 03/10] drbd: cmdname() enum to string convertion was missing a few constants

[Philipp Reisner]

From: Lars Ellenberg <lars.ellenberg@linbit.com>

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
---
 drivers/block/drbd/drbd_main.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index a0d81de..96add4e 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -3243,8 +3243,17 @@ const char *cmdname(enum drbd_packet cmd)
 		[P_DELAY_PROBE]         = "DelayProbe",
 		[P_OUT_OF_SYNC]		= "OutOfSync",
 		[P_RETRY_WRITE]		= "RetryWrite",
+		[P_RS_CANCEL]		= "RSCancel",
+		[P_CONN_ST_CHG_REQ]	= "conn_st_chg_req",
+		[P_CONN_ST_CHG_REPLY]	= "conn_st_chg_reply",
+
+		/* enum drbd_packet, but not commands - obsoleted flags:
+		 *	P_MAY_IGNORE
+		 *	P_MAX_OPT_CMD
+		 */
 	};
 
+	/* too big for the array: 0xfffX */
 	if (cmd == P_INITIAL_META)
 		return "InitialMeta";
 	if (cmd == P_INITIAL_DATA)
-- 
1.7.4.1

[PATCH 04/10] drbd: move comment about stopping the receiver thread to where it belongs

[Philipp Reisner]

From: Lars Ellenberg <lars.ellenberg@linbit.com>

When the last volume of a replication group is unconfigured,
the worker thread exits. To not interfere with cleanup
of other threads, before the the last cleanups run,
we need to make sure the receiver has already exited.

The commend explaining that clearly belongs above
drbd_thread_stop(&tconn->receiver), not in the cleanup loop below.

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
---
 drivers/block/drbd/drbd_worker.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index c54c191..4e14133 100644
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -1746,12 +1746,13 @@ int drbd_worker(struct drbd_thread *thi)
 	 */
 	spin_unlock_irq(&tconn->data.work.q_lock);
 
+	/* _drbd_set_state only uses stop_nowait.
+	 * wait here for the exiting receiver. */
 	drbd_thread_stop(&tconn->receiver);
+
 	down_read(&drbd_cfg_rwsem);
 	idr_for_each_entry(&tconn->volumes, mdev, vnr) {
 		D_ASSERT(mdev->state.disk == D_DISKLESS && mdev->state.conn == C_STANDALONE);
-		/* _drbd_set_state only uses stop_nowait.
-		 * wait here for the exiting receiver. */
 		drbd_mdev_cleanup(mdev);
 	}
 	up_read(&drbd_cfg_rwsem);
-- 
1.7.4.1

[PATCH 05/10] drbd: Eliminated drbd_free_resoruces() it is superseeded by conn_free_crypto()

[Philipp Reisner]

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
---
 drivers/block/drbd/drbd_int.h  |    1 -
 drivers/block/drbd/drbd_main.c |   35 ++++++++++++-----------------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index 3080192..0fb3fc3 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -1046,7 +1046,6 @@ extern void drbd_calc_cpu_mask(struct drbd_tconn *tconn);
 #define drbd_thread_current_set_cpu(A) ({})
 #define drbd_calc_cpu_mask(A) ({})
 #endif
-extern void drbd_free_resources(struct drbd_conf *mdev);
 extern void tl_release(struct drbd_tconn *, unsigned int barrier_nr,
 		       unsigned int set_size);
 extern void tl_clear(struct drbd_tconn *);
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 96add4e..28f1b74 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -2054,7 +2054,9 @@ void drbd_mdev_cleanup(struct drbd_conf *mdev)
 		drbd_bm_cleanup(mdev);
 	}
 
-	drbd_free_resources(mdev);
+	drbd_free_bc(mdev->ldev);
+	mdev->ldev = NULL;
+
 	clear_bit(AL_SUSPENDED, &mdev->flags);
 
 	D_ASSERT(list_empty(&mdev->active_ee));
@@ -2249,7 +2251,8 @@ void drbd_delete_device(struct drbd_conf *mdev)
 	if (mdev->this_bdev)
 		bdput(mdev->this_bdev);
 
-	drbd_free_resources(mdev);
+	drbd_free_bc(mdev->ldev);
+	mdev->ldev = NULL;
 
 	drbd_release_all_peer_reqs(mdev);
 
@@ -2384,11 +2387,18 @@ static void drbd_free_socket(struct drbd_socket *socket)
 
 void conn_free_crypto(struct drbd_tconn *tconn)
 {
+	drbd_free_sock(tconn);
+
+	crypto_free_hash(tconn->csums_tfm);
+	crypto_free_hash(tconn->verify_tfm);
 	crypto_free_hash(tconn->cram_hmac_tfm);
 	crypto_free_hash(tconn->integrity_w_tfm);
 	crypto_free_hash(tconn->integrity_r_tfm);
 	kfree(tconn->int_dig_in);
 	kfree(tconn->int_dig_vv);
+
+	tconn->csums_tfm = NULL;
+	tconn->verify_tfm = NULL;
 	tconn->cram_hmac_tfm = NULL;
 	tconn->integrity_w_tfm = NULL;
 	tconn->integrity_r_tfm = NULL;
@@ -2697,27 +2707,6 @@ void drbd_free_sock(struct drbd_tconn *tconn)
 	}
 }
 
-
-void drbd_free_resources(struct drbd_conf *mdev)
-{
-	crypto_free_hash(mdev->tconn->csums_tfm);
-	mdev->tconn->csums_tfm = NULL;
-	crypto_free_hash(mdev->tconn->verify_tfm);
-	mdev->tconn->verify_tfm = NULL;
-	crypto_free_hash(mdev->tconn->cram_hmac_tfm);
-	mdev->tconn->cram_hmac_tfm = NULL;
-	crypto_free_hash(mdev->tconn->integrity_w_tfm);
-	mdev->tconn->integrity_w_tfm = NULL;
-	crypto_free_hash(mdev->tconn->integrity_r_tfm);
-	mdev->tconn->integrity_r_tfm = NULL;
-
-	drbd_free_sock(mdev->tconn);
-
-	__no_warn(local,
-		  drbd_free_bc(mdev->ldev);
-		  mdev->ldev = NULL;);
-}
-
 /* meta data management */
 
 struct meta_data_on_disk {
-- 
1.7.4.1

[PATCH 06/10] drbd: Basic refcounting for drbd_tconn

[Philipp Reisner]

References hold by:
 * Each (running) drbd thread has a reference on tconn
 * Each mdev has a referenc on tconn
 * Beeing in the all_tconn list counts for one reference
 * Each after_conn_state_chg_work has a reference to tconn

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
---
 drivers/block/drbd/drbd_int.h   |    5 +++--
 drivers/block/drbd/drbd_main.c  |   20 +++++++++++++++++---
 drivers/block/drbd/drbd_nl.c    |   14 +++++++++-----
 drivers/block/drbd/drbd_state.c |    2 ++
 4 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index 0fb3fc3..3abf982 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -828,6 +828,7 @@ enum {
 struct drbd_tconn {			/* is a resource from the config file */
 	char *name;			/* Resource name */
 	struct list_head all_tconn;	/* linked on global drbd_tconns */
+	struct kref kref;
 	struct idr volumes;		/* <tconn, vnr> to mdev mapping */
 	enum drbd_conns cstate;		/* Only C_STANDALONE to C_WF_REPORT_PARAMS */
 	unsigned susp:1;		/* IO suspended by user */
@@ -1378,8 +1379,8 @@ extern int conn_lowest_minor(struct drbd_tconn *tconn);
 enum drbd_ret_code conn_new_minor(struct drbd_tconn *tconn, unsigned int minor, int vnr);
 extern void drbd_delete_device(struct drbd_conf *mdev);
 
-struct drbd_tconn *drbd_new_tconn(const char *name);
-extern void drbd_free_tconn(struct drbd_tconn *tconn);
+struct drbd_tconn *conn_create(const char *name);
+extern void conn_destroy(struct kref *kref);
 struct drbd_tconn *conn_by_name(const char *name);
 extern void conn_free_crypto(struct drbd_tconn *tconn);
 
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 28f1b74..1c94d56 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -509,6 +509,8 @@ restart:
 	conn_info(tconn, "Terminating %s\n", current->comm);
 
 	/* Release mod reference taken when thread was started */
+
+	kref_put(&tconn->kref, &conn_destroy);
 	module_put(THIS_MODULE);
 	return retval;
 }
@@ -546,6 +548,8 @@ int drbd_thread_start(struct drbd_thread *thi)
 			return false;
 		}
 
+		kref_get(&thi->tconn->kref);
+
 		init_completion(&thi->stop);
 		thi->reset_cpu_mask = 1;
 		thi->t_state = RUNNING;
@@ -558,6 +562,7 @@ int drbd_thread_start(struct drbd_thread *thi)
 		if (IS_ERR(nt)) {
 			conn_err(tconn, "Couldn't start thread\n");
 
+			kref_put(&tconn->kref, &conn_destroy);
 			module_put(THIS_MODULE);
 			return false;
 		}
@@ -2234,6 +2239,8 @@ static void drbd_release_all_peer_reqs(struct drbd_conf *mdev)
 /* caution. no locking. */
 void drbd_delete_device(struct drbd_conf *mdev)
 {
+	struct drbd_tconn *tconn = mdev->tconn;
+
 	idr_remove(&mdev->tconn->volumes, mdev->vnr);
 	idr_remove(&minors, mdev_to_minor(mdev));
 	synchronize_rcu();
@@ -2269,6 +2276,8 @@ void drbd_delete_device(struct drbd_conf *mdev)
 	put_disk(mdev->vdisk);
 	blk_cleanup_queue(mdev->rq_queue);
 	kfree(mdev);
+
+	kref_put(&tconn->kref, &conn_destroy);
 }
 
 static void drbd_cleanup(void)
@@ -2406,7 +2415,7 @@ void conn_free_crypto(struct drbd_tconn *tconn)
 	tconn->int_dig_vv = NULL;
 }
 
-struct drbd_tconn *drbd_new_tconn(const char *name)
+struct drbd_tconn *conn_create(const char *name)
 {
 	struct drbd_tconn *tconn;
 
@@ -2452,6 +2461,7 @@ struct drbd_tconn *drbd_new_tconn(const char *name)
 	};
 
 	down_write(&drbd_cfg_rwsem);
+	kref_init(&tconn->kref);
 	list_add_tail(&tconn->all_tconn, &drbd_tconns);
 	up_write(&drbd_cfg_rwsem);
 
@@ -2468,9 +2478,10 @@ fail:
 	return NULL;
 }
 
-void drbd_free_tconn(struct drbd_tconn *tconn)
+void conn_destroy(struct kref *kref)
 {
-	list_del(&tconn->all_tconn);
+	struct drbd_tconn *tconn = container_of(kref, struct drbd_tconn, kref);
+
 	idr_destroy(&tconn->volumes);
 
 	free_cpumask_var(tconn->cpu_mask);
@@ -2500,7 +2511,9 @@ enum drbd_ret_code conn_new_minor(struct drbd_tconn *tconn, unsigned int minor,
 	if (!mdev)
 		return ERR_NOMEM;
 
+	kref_get(&tconn->kref);
 	mdev->tconn = tconn;
+
 	mdev->minor = minor;
 	mdev->vnr = vnr;
 
@@ -2602,6 +2615,7 @@ out_no_disk:
 	blk_cleanup_queue(q);
 out_no_q:
 	kfree(mdev);
+	kref_put(&tconn->kref, &conn_destroy);
 	return err;
 }
 
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index 0b6b5d8..b854427 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -2676,7 +2676,7 @@ int get_one_status(struct sk_buff *skb, struct netlink_callback *cb)
 	 * on each iteration.
 	 */
 
-	/* synchronize with drbd_new_tconn/drbd_free_tconn */
+	/* synchronize with conn_create()/conn_destroy() */
 	down_read(&drbd_cfg_rwsem);
 	/* revalidate iterator position */
 	list_for_each_entry(tmp, &drbd_tconns, all_tconn) {
@@ -2983,7 +2983,7 @@ int drbd_adm_create_connection(struct sk_buff *skb, struct genl_info *info)
 		goto out;
 	}
 
-	if (!drbd_new_tconn(adm_ctx.conn_name))
+	if (!conn_create(adm_ctx.conn_name))
 		retcode = ERR_NOMEM;
 out:
 	drbd_adm_finish(info, retcode);
@@ -3058,7 +3058,7 @@ int drbd_adm_delete_minor(struct sk_buff *skb, struct genl_info *info)
 	/* if this was the last volume of this connection,
 	 * this will terminate all threads */
 	if (retcode == NO_ERROR)
-		conn_reconfig_done(adm_ctx.tconn);
+		conn_reconfig_done(adm_ctx.mdev->tconn);
 out:
 	drbd_adm_finish(info, retcode);
 	return 0;
@@ -3128,7 +3128,9 @@ int drbd_adm_down(struct sk_buff *skb, struct genl_info *info)
 
 	/* delete connection */
 	if (conn_lowest_minor(adm_ctx.tconn) < 0) {
-		drbd_free_tconn(adm_ctx.tconn);
+		list_del(&adm_ctx.tconn->all_tconn);
+		kref_put(&adm_ctx.tconn->kref, &conn_destroy);
+
 		retcode = NO_ERROR;
 	} else {
 		/* "can not happen" */
@@ -3157,7 +3159,9 @@ int drbd_adm_delete_connection(struct sk_buff *skb, struct genl_info *info)
 
 	down_write(&drbd_cfg_rwsem);
 	if (conn_lowest_minor(adm_ctx.tconn) < 0) {
-		drbd_free_tconn(adm_ctx.tconn);
+		list_del(&adm_ctx.tconn->all_tconn);
+		kref_put(&adm_ctx.tconn->kref, &conn_destroy);
+
 		retcode = NO_ERROR;
 	} else {
 		retcode = ERR_CONN_IN_USE;
diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index 2423283..054b698 100644
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -1442,6 +1442,7 @@ static int w_after_conn_state_ch(struct drbd_work *w, int unused)
 
 	//conn_err(tconn, STATE_FMT, STATE_ARGS("nms", nms));
 	after_all_state_ch(tconn);
+	kref_put(&tconn->kref, &conn_destroy);
 
 	return 0;
 }
@@ -1668,6 +1669,7 @@ _conn_request_state(struct drbd_tconn *tconn, union drbd_state mask, union drbd_
 		acscw->ns_max = ns_max;
 		acscw->flags = flags;
 		acscw->w.cb = w_after_conn_state_ch;
+		kref_get(&tconn->kref);
 		acscw->w.tconn = tconn;
 		drbd_queue_work(&tconn->data.work, &acscw->w);
 	} else {
-- 
1.7.4.1

[PATCH 07/10] drbd: Take a reference on tconn when finding a tconn by name

[Philipp Reisner]

Rule #3 of kref.txt

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
---
 drivers/block/drbd/drbd_int.h  |    2 +-
 drivers/block/drbd/drbd_main.c |    6 ++++--
 drivers/block/drbd/drbd_nl.c   |   15 +++++++++++----
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index 3abf982..7797879 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -1381,7 +1381,7 @@ extern void drbd_delete_device(struct drbd_conf *mdev);
 
 struct drbd_tconn *conn_create(const char *name);
 extern void conn_destroy(struct kref *kref);
-struct drbd_tconn *conn_by_name(const char *name);
+struct drbd_tconn *conn_get_by_name(const char *name);
 extern void conn_free_crypto(struct drbd_tconn *tconn);
 
 extern int proc_details;
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 1c94d56..02efec5 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -2359,7 +2359,7 @@ static void drbd_init_workqueue(struct drbd_work_queue* wq)
 	INIT_LIST_HEAD(&wq->q);
 }
 
-struct drbd_tconn *conn_by_name(const char *name)
+struct drbd_tconn *conn_get_by_name(const char *name)
 {
 	struct drbd_tconn *tconn;
 
@@ -2368,8 +2368,10 @@ struct drbd_tconn *conn_by_name(const char *name)
 
 	down_read(&drbd_cfg_rwsem);
 	list_for_each_entry(tconn, &drbd_tconns, all_tconn) {
-		if (!strcmp(tconn->name, name))
+		if (!strcmp(tconn->name, name)) {
+			kref_get(&tconn->kref);
 			goto found;
+		}
 	}
 	tconn = NULL;
 found:
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index b854427..3d03114 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -219,7 +219,7 @@ static int drbd_adm_prepare(struct sk_buff *skb, struct genl_info *info,
 
 	adm_ctx.minor = d_in->minor;
 	adm_ctx.mdev = minor_to_mdev(d_in->minor);
-	adm_ctx.tconn = conn_by_name(adm_ctx.conn_name);
+	adm_ctx.tconn = conn_get_by_name(adm_ctx.conn_name);
 
 	pr_info("adm request: cmd=%u[%s], flags=0x%x, minor=%d, conn=%s\n",
 		cmd, drbd_genl_cmd_to_str(cmd), d_in->flags,
@@ -251,8 +251,7 @@ static int drbd_adm_prepare(struct sk_buff *skb, struct genl_info *info,
 		drbd_msg_put_info("minor exists as different volume");
 		return ERR_INVALID_REQUEST;
 	}
-	if (adm_ctx.mdev && !adm_ctx.tconn)
-		adm_ctx.tconn = adm_ctx.mdev->tconn;
+
 	return NO_ERROR;
 
 fail:
@@ -267,6 +266,11 @@ static int drbd_adm_finish(struct genl_info *info, int retcode)
 	const char *conn_name = NULL;
 	const u8 cmd = info->genlhdr->cmd;
 
+	if (adm_ctx.tconn) {
+		kref_put(&adm_ctx.tconn->kref, &conn_destroy);
+		adm_ctx.tconn = NULL;
+	}
+
 	if (!adm_ctx.reply_skb)
 		return -ENOMEM;
 
@@ -2795,10 +2799,13 @@ int drbd_adm_get_status_all(struct sk_buff *skb, struct netlink_callback *cb)
 	if (!nla)
 		return -EINVAL;
 	conn_name = nla_data(nla);
-	tconn = conn_by_name(conn_name);
+	tconn = conn_get_by_name(conn_name);
+
 	if (!tconn)
 		return -ENODEV;
 
+	kref_put(&tconn->kref, &conn_destroy); /* get_one_status() (re)validates tconn by itself */
+
 	/* prime iterators, and set "filter" mode mark:
 	 * only dump this tconn. */
 	cb->args[0] = (long)tconn;
-- 
1.7.4.1

[PATCH 08/10] drbd: Removed the OBJECT_DYING and the CONFIG_PENDING bits

[Philipp Reisner]

superseded by refcounting

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
---
 drivers/block/drbd/drbd_int.h    |    6 ------
 drivers/block/drbd/drbd_nl.c     |   20 +++-----------------
 drivers/block/drbd/drbd_state.c  |   16 +---------------
 drivers/block/drbd/drbd_worker.c |    3 ---
 4 files changed, 4 insertions(+), 41 deletions(-)

diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index 7797879..28e7ecc 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -816,12 +816,6 @@ enum {
 	GOT_PING_ACK,		/* set when we receive a ping_ack packet, ping_wait gets woken */
 	CONN_WD_ST_CHG_OKAY,
 	CONN_WD_ST_CHG_FAIL,
-	CONFIG_PENDING,		/* serialization of (re)configuration requests.
-				 * if set, also prevents the device from dying */
-	OBJECT_DYING,		/* device became unconfigured,
-				 * but worker thread is still handling the cleanup.
-				 * reconfiguring (nl_disk_conf, nl_net_conf) is dissalowed,
-				 * while this is set. */
 	CONN_DRY_RUN,		/* Expect disconnect after resync handshake. */
 };
 
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index 3d03114..7926491 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -1062,34 +1062,20 @@ void drbd_reconsider_max_bio_size(struct drbd_conf *mdev)
 	drbd_setup_queue_param(mdev, new);
 }
 
-/* serialize deconfig (worker exiting, doing cleanup)
- * and reconfig (drbdsetup disk, drbdsetup net)
- *
- * Wait for a potentially exiting worker, then restart it,
- * or start a new one.  Flush any pending work, there may still be an
- * after_state_change queued.
- */
+/* Starts the worker thread */
 static void conn_reconfig_start(struct drbd_tconn *tconn)
 {
-	wait_event(tconn->ping_wait, !test_and_set_bit(CONFIG_PENDING, &tconn->flags));
-	wait_event(tconn->ping_wait, !test_bit(OBJECT_DYING, &tconn->flags));
 	drbd_thread_start(&tconn->worker);
 	conn_flush_workqueue(tconn);
 }
 
-/* if still unconfigured, stops worker again.
- * if configured now, clears CONFIG_PENDING.
- * wakes potential waiters */
+/* if still unconfigured, stops worker again. */
 static void conn_reconfig_done(struct drbd_tconn *tconn)
 {
 	spin_lock_irq(&tconn->req_lock);
-	if (conn_all_vols_unconf(tconn)) {
-		set_bit(OBJECT_DYING, &tconn->flags);
+	if (conn_all_vols_unconf(tconn))
 		drbd_thread_stop_nowait(&tconn->worker);
-	} else
-		clear_bit(CONFIG_PENDING, &tconn->flags);
 	spin_unlock_irq(&tconn->req_lock);
-	wake_up(&tconn->ping_wait);
 }
 
 /* Make sure IO is suspended before calling this function(). */
diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index 054b698..4465be8 100644
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -908,18 +908,6 @@ __drbd_set_state(struct drbd_conf *mdev, union drbd_state ns,
 	mdev->tconn->susp_nod = ns.susp_nod;
 	mdev->tconn->susp_fen = ns.susp_fen;
 
-	/* solve the race between becoming unconfigured,
-	 * worker doing the cleanup, and
-	 * admin reconfiguring us:
-	 * on (re)configure, first set CONFIG_PENDING,
-	 * then wait for a potentially exiting worker,
-	 * start the worker, and schedule one no_op.
-	 * then proceed with configuration.
-	 */
-	if(conn_all_vols_unconf(mdev->tconn) &&
-	   !test_and_set_bit(CONFIG_PENDING, &mdev->tconn->flags))
-		set_bit(OBJECT_DYING, &mdev->tconn->flags);
-
 	if (os.disk == D_ATTACHING && ns.disk >= D_NEGOTIATING)
 		drbd_print_uuids(mdev, "attached to UUIDs");
 
@@ -1383,10 +1371,8 @@ struct after_conn_state_chg_work {
 
 static void after_all_state_ch(struct drbd_tconn *tconn)
 {
-	if (conn_all_vols_unconf(tconn) &&
-	    test_bit(OBJECT_DYING, &tconn->flags)) {
+	if (conn_all_vols_unconf(tconn))
 		drbd_thread_stop_nowait(&tconn->worker);
-	}
 }
 
 static int w_after_conn_state_ch(struct drbd_work *w, int unused)
diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index 4e14133..a8aa667 100644
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -1756,9 +1756,6 @@ int drbd_worker(struct drbd_thread *thi)
 		drbd_mdev_cleanup(mdev);
 	}
 	up_read(&drbd_cfg_rwsem);
-	clear_bit(OBJECT_DYING, &tconn->flags);
-	clear_bit(CONFIG_PENDING, &tconn->flags);
-	wake_up(&tconn->ping_wait);
 
 	return 0;
 }
-- 
1.7.4.1

[PATCH 09/10] drbd: remove useless kobject_uevent from drbd_adm_connect

[Philipp Reisner]

From: Lars Ellenberg <lars.ellenberg@linbit.com>

Calling kobject_uevent, which may sleep, from within rcu_read_lock()
protected regions is not possible.
This particular kobject_uevent also is also wrong. It was supposed to
trigger a udev run, just in case something relevant to udev symlink
magic has changed, when adjusting runtime re-configurable settings while
we still had the "syncer conf".  It was improperly placed in connect
when we dropped the "syncer conf".  The right thing to do is probably to
call "udevadm trigger" directly in those cases where drbdadm thinks
there was a need to trigger extra udev runs.

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
---
 drivers/block/drbd/drbd_nl.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index 7926491..c475654 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -2088,15 +2088,15 @@ int drbd_adm_connect(struct sk_buff *skb, struct genl_info *info)
 
 	mutex_unlock(&tconn->net_conf_update);
 
-	retcode = conn_request_state(tconn, NS(conn, C_UNCONNECTED), CS_VERBOSE);
-
 	rcu_read_lock();
 	idr_for_each_entry(&tconn->volumes, mdev, i) {
 		mdev->send_cnt = 0;
 		mdev->recv_cnt = 0;
-		kobject_uevent(&disk_to_dev(mdev->vdisk)->kobj, KOBJ_CHANGE);
 	}
 	rcu_read_unlock();
+
+	retcode = conn_request_state(tconn, NS(conn, C_UNCONNECTED), CS_VERBOSE);
+
 	conn_reconfig_done(tconn);
 	drbd_adm_finish(info, retcode);
 	return 0;
-- 
1.7.4.1

[PATCH 10/10] drbd: fix various disconnecting races

[Philipp Reisner]

From: Lars Ellenberg <lars.ellenberg@linbit.com>

If an admin requests disconnect at a time when the state handling
already disconnects/reconnects, there have been some races.

Make sure to always really stop the network threads before
returning success for disconnect. Do not pretend successfull
forced disconnect, if the state handling returned an error.

Return success from drbd_adm_down() only after all threads are finished.

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
---
 drivers/block/drbd/drbd_nl.c       |   21 ++++++++++++---------
 drivers/block/drbd/drbd_receiver.c |    2 +-
 drivers/block/drbd/drbd_state.c    |   20 +++++++++++++-------
 3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index c475654..512410c 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -2120,10 +2120,9 @@ static enum drbd_state_rv conn_try_disconnect(struct drbd_tconn *tconn, bool for
 	enum drbd_state_rv rv;
 	if (force) {
 		spin_lock_irq(&tconn->req_lock);
-		if (tconn->cstate >= C_WF_CONNECTION)
-			_conn_request_state(tconn, NS(conn, C_DISCONNECTING), CS_HARD);
+		rv = _conn_request_state(tconn, NS(conn, C_DISCONNECTING), CS_HARD);
 		spin_unlock_irq(&tconn->req_lock);
-		return SS_SUCCESS;
+		return rv;
 	}
 
 	rv = conn_request_state(tconn, NS(conn, C_DISCONNECTING), 0);
@@ -2182,10 +2181,12 @@ int drbd_adm_disconnect(struct sk_buff *skb, struct genl_info *info)
 	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_DISCONNECTING)) {
-		/* Do not test for mdev->state.conn == C_STANDALONE, since
-		   someone else might connect us in the mean time! */
+				     tconn->cstate == C_STANDALONE)) {
 		retcode = ERR_INTR;
 		goto fail;
 	}
@@ -3093,6 +3094,10 @@ int drbd_adm_down(struct sk_buff *skb, struct genl_info *info)
 		goto out_unlock;
 	}
 
+	/* Make sure the network threads have actually stopped,
+	 * state handling only does drbd_thread_stop_nowait(). */
+	drbd_thread_stop(&adm_ctx.tconn->receiver);
+
 	/* detach */
 	idr_for_each_entry(&adm_ctx.tconn->volumes, mdev, i) {
 		rv = adm_detach(mdev);
@@ -3116,11 +3121,9 @@ int drbd_adm_down(struct sk_buff *skb, struct genl_info *info)
 		}
 	}
 
-	/* stop all threads */
-	conn_reconfig_done(adm_ctx.tconn);
-
 	/* delete connection */
 	if (conn_lowest_minor(adm_ctx.tconn) < 0) {
+		drbd_thread_stop(&adm_ctx.tconn->worker);
 		list_del(&adm_ctx.tconn->all_tconn);
 		kref_put(&adm_ctx.tconn->kref, &conn_destroy);
 
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 4665ad7..6da7aeb 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -4226,7 +4226,7 @@ static void drbd_disconnect(struct drbd_tconn *tconn)
 		synchronize_rcu();
 		kfree(old_conf);
 
-		conn_request_state(tconn, NS(conn, C_STANDALONE), CS_VERBOSE);
+		conn_request_state(tconn, NS(conn, C_STANDALONE), CS_VERBOSE | CS_HARD);
 	}
 }
 
diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index 4465be8..231e3fe 100644
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -586,21 +586,27 @@ is_valid_soft_transition(union drbd_state os, union drbd_state ns)
 static enum drbd_state_rv
 is_valid_conn_transition(enum drbd_conns oc, enum drbd_conns nc)
 {
-	enum drbd_state_rv rv = SS_SUCCESS;
+	/* no change -> nothing to do, at least for the connection part */
+	if (oc == nc)
+		return SS_NOTHING_TO_DO;
 
-	/* Disallow Network errors to configure a device's network part */
-	if ((nc >= C_TIMEOUT && nc <= C_TEAR_DOWN) && oc <= C_DISCONNECTING)
-		rv = SS_NEED_CONNECTION;
+	/* disconnect of an unconfigured connection does not make sense */
+	if (oc == C_STANDALONE && nc == C_DISCONNECTING)
+		return SS_ALREADY_STANDALONE;
+
+	/* from C_STANDALONE, we start with C_UNCONNECTED */
+	if (oc == C_STANDALONE && nc != C_UNCONNECTED)
+		return SS_NEED_CONNECTION;
 
 	/* After a network error only C_UNCONNECTED or C_DISCONNECTING may follow. */
 	if (oc >= C_TIMEOUT && oc <= C_TEAR_DOWN && nc != C_UNCONNECTED && nc != C_DISCONNECTING)
-		rv = SS_IN_TRANSIENT_STATE;
+		return SS_IN_TRANSIENT_STATE;
 
 	/* After C_DISCONNECTING only C_STANDALONE may follow */
 	if (oc == C_DISCONNECTING && nc != C_STANDALONE)
-		rv = SS_IN_TRANSIENT_STATE;
+		return SS_IN_TRANSIENT_STATE;
 
-	return rv;
+	return SS_SUCCESS;
 }
 
 
-- 
1.7.4.1