All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Josef Bacik <josef@toxicpanda.com>, Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org, nbd@other.debian.org,
	Xiubo Li <xiubli@redhat.com>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	syzbot+2c98885bcd769f56b6d6@syzkaller.appspotmail.com
Subject: [PATCH 6/6] nbd: remove nbd->destroy_complete
Date: Wed, 25 Aug 2021 18:31:08 +0200	[thread overview]
Message-ID: <20210825163108.50713-7-hch@lst.de> (raw)
In-Reply-To: <20210825163108.50713-1-hch@lst.de>

The nbd->destroy_complete pointer is not really needed.  For creating
a device without a specific index we now simplify skip devices marked
NBD_DESTROY_ON_DISCONNECT as there is not much point to reuse them.
For device creation with a specific index there is no real need to
treat the case of a requested but not finished disconnect different
than any other device that is being shutdown, i.e. we can just return
an error, as a slightly different race window would anyway.

Fixes: 6e4df4c64881 ("nbd: reduce the nbd_index_mutex scope")
Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reported-by: syzbot+2c98885bcd769f56b6d6@syzkaller.appspotmail.com
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/nbd.c | 52 ++++++++++++---------------------------------
 1 file changed, 14 insertions(+), 38 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 5c03f3eb3129..5170a630778d 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -120,7 +120,6 @@ struct nbd_device {
 	struct task_struct *task_recv;
 	struct task_struct *task_setup;
 
-	struct completion *destroy_complete;
 	unsigned long flags;
 
 	char *backend;
@@ -235,19 +234,6 @@ static const struct device_attribute backend_attr = {
 	.show = backend_show,
 };
 
-/*
- * Place this in the last just before the nbd is freed to
- * make sure that the disk and the related kobject are also
- * totally removed to avoid duplicate creation of the same
- * one.
- */
-static void nbd_notify_destroy_completion(struct nbd_device *nbd)
-{
-	if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) &&
-	    nbd->destroy_complete)
-		complete(nbd->destroy_complete);
-}
-
 static void nbd_dev_remove(struct nbd_device *nbd)
 {
 	struct gendisk *disk = nbd->disk;
@@ -262,7 +248,6 @@ static void nbd_dev_remove(struct nbd_device *nbd)
 	 */
 	mutex_lock(&nbd_index_mutex);
 	idr_remove(&nbd_index_idr, nbd->index);
-	nbd_notify_destroy_completion(nbd);
 	mutex_unlock(&nbd_index_mutex);
 
 	kfree(nbd);
@@ -1706,7 +1691,6 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs)
 		BLK_MQ_F_BLOCKING;
 	nbd->tag_set.driver_data = nbd;
 	INIT_WORK(&nbd->remove_work, nbd_dev_remove_work);
-	nbd->destroy_complete = NULL;
 	nbd->backend = NULL;
 
 	err = blk_mq_alloc_tag_set(&nbd->tag_set);
@@ -1858,7 +1842,6 @@ static int nbd_genl_size_set(struct genl_info *info, struct nbd_device *nbd)
 
 static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info)
 {
-	DECLARE_COMPLETION_ONSTACK(destroy_complete);
 	struct nbd_device *nbd;
 	struct nbd_config *config;
 	int index = -1;
@@ -1880,31 +1863,24 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info)
 	}
 again:
 	mutex_lock(&nbd_index_mutex);
-	if (index == -1)
+	if (index == -1) {
 		nbd = nbd_find_get_unused();
-	else
+	} else {
 		nbd = idr_find(&nbd_index_idr, index);
-	if (nbd && index != -1) {
-		if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) &&
-		    test_bit(NBD_DISCONNECT_REQUESTED, &nbd->flags)) {
-			nbd->destroy_complete = &destroy_complete;
-			mutex_unlock(&nbd_index_mutex);
-
-			/* wait until the nbd device is completely destroyed */
-			wait_for_completion(&destroy_complete);
-			goto again;
-		}
-
-		if (!refcount_inc_not_zero(&nbd->refs)) {
-			mutex_unlock(&nbd_index_mutex);
-			pr_err("nbd: device at index %d is going down\n",
-				index);
-			return -EINVAL;
+		if (nbd) {
+			if ((test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) &&
+			     test_bit(NBD_DISCONNECT_REQUESTED, &nbd->flags)) ||
+			    !refcount_inc_not_zero(&nbd->refs)) {
+				mutex_unlock(&nbd_index_mutex);
+				pr_err("nbd: device at index %d is going down\n",
+					index);
+				return -EINVAL;
+			}
 		}
-		mutex_unlock(&nbd_index_mutex);
-	} else {
-		mutex_unlock(&nbd_index_mutex);
+	}
+	mutex_unlock(&nbd_index_mutex);
 
+	if (!nbd) {
 		nbd = nbd_dev_add(index, 2);
 		if (IS_ERR(nbd)) {
 			pr_err("nbd: failed to add new device\n");
-- 
2.30.2


  parent reply	other threads:[~2021-08-25 16:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-25 16:31 nbd lifetimes fixes Christoph Hellwig
2021-08-25 16:31 ` [PATCH 1/6] nbd: add missing locking to the nbd_dev_add error path Christoph Hellwig
2021-08-25 16:31 ` [PATCH 2/6] nbd: reset NBD to NULL when restarting in nbd_genl_connect Christoph Hellwig
2021-08-25 16:31 ` [PATCH 3/6] nbd: prevent IDR lookups from finding partially initialized devices Christoph Hellwig
2021-08-25 16:31 ` [PATCH 4/6] nbd: set nbd->index before releasing nbd_index_mutex Christoph Hellwig
2021-08-25 16:31 ` [PATCH 5/6] nbd: only return usable devices from nbd_find_unused Christoph Hellwig
2021-08-25 16:31 ` Christoph Hellwig [this message]
2021-08-25 20:14 ` nbd lifetimes fixes Josef Bacik
2021-08-25 20:21 ` Jens Axboe
2021-10-08  9:17 ` Xiubo Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210825163108.50713-7-hch@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=josef@toxicpanda.com \
    --cc=linux-block@vger.kernel.org \
    --cc=nbd@other.debian.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=syzbot+2c98885bcd769f56b6d6@syzkaller.appspotmail.com \
    --cc=xiubli@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.