Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: xn62ZDYQImLB3xJ7ZgFDf8F4oqb10yny9IGGjzDefQWXxVT0XqfUoBjwb43Bqrbjxvyf9GaUui
 dl3tZlbSSBzzBn0CIZhhDcr3FsPpHVJyilmmY+ZgKusGW1FD+fnRdT6MzAszobK3AAAbU0QY0O
 a4lIi3S88VX8Xz4ro0H4jLBQNxsW4HIsKQ9ON84b1sXgEnGP7dEI9RstCo8h+gr134qpgIh0em
 U0dx+k/MUlxsoftVtpqM0rhLUJ+VRfsrpghxeIHiCeMHbFmgKf9TqaUcbRk9KZg27Dj7Dmp07C
 U8A=
IronPort-SDR: f8RV/cT1TcEwdFysI+RUwET5DQDd0s+Be04gTwDUmpKPpqjnr1/OfKgBn4Tn9sWghUozRGFXJz
 dojvgG4vvJn/4psKRII3ELvejQDMzTBkQ=
IronPort-SDR: oq9KIwqGk2lsQJ2F7UpRsBjmE8rHamV0JijZJGpj0a7tWtXi9jTvsO03148VIHsma6npPFRlf4
 Oe5++k68rUnQ==
WDCIronportException: Internal
From:   Niklas Cassel <niklas.cassel@wdc.com>
To:     Keith Busch <kbusch@kernel.org>, Jens Axboe <axboe@fb.com>,
        Christoph Hellwig <hch@lst.de>,
        Sagi Grimberg <sagi@grimberg.me>,
        James Smart <james.smart@broadcom.com>,
        Israel Rukshin <israelr@mellanox.com>,
        Max Gurtovoy <maxg@mellanox.com>
Cc:     Niklas Cassel <niklas.cassel@wdc.com>,
        linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: [PATCH] nvme: do not call del_gendisk() on a disk that was never added
Date:   Sun,  7 Jun 2020 13:45:20 +0200
Message-Id: <20200607114520.130756-1-niklas.cassel@wdc.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

device_add_disk() is negated by del_gendisk().
alloc_disk_node() is negated by put_disk().

In nvme_alloc_ns(), device_add_disk() is one of the last things being
called in the success case, and only void functions are being called
after this. Therefore this call should not be negated in the error path.

The superfluous call to del_gendisk() leads to the following prints:
[    7.839975] kobject: '(null)' (000000001ff73734): is not initialized, yet kobject_put() is being called.
[    7.840865] WARNING: CPU: 2 PID: 361 at lib/kobject.c:736 kobject_put+0x70/0x120

Fixes: 33cfdc2aa696 ("nvme: enforce extended LBA format for fabrics metadata")
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
An alternative would be to do like nvme_ns_remove(), i.e. in the error
path; check if ns->disk->flags & GENHD_FL_UP is set, and only then call
del_gendisk(). However, that seems unnecessary, since as nvme_alloc_ns()
is currently written, we know that device_add_disk() does not need to be
negated.

 drivers/nvme/host/core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 0585efa47d8f..c2c5bc4fb702 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3669,7 +3669,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 	ns->disk = disk;
 
 	if (__nvme_revalidate_disk(disk, id))
-		goto out_free_disk;
+		goto out_put_disk;
 
 	if ((ctrl->quirks & NVME_QUIRK_LIGHTNVM) && id->vs[0] == 0x1) {
 		ret = nvme_nvm_register(ns, disk_name, node);
@@ -3696,8 +3696,6 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 	/* prevent double queue cleanup */
 	ns->disk->queue = NULL;
 	put_disk(ns->disk);
- out_free_disk:
-	del_gendisk(ns->disk);
  out_unlink_ns:
 	mutex_lock(&ctrl->subsys->lock);
 	list_del_rcu(&ns->siblings);
-- 
2.26.2