mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Hemanth Selam <hemanth.selam@gmail.com>
To: djbw@kernel.org, vishal.l.verma@intel.com, dave.jiang@intel.com,
	alison.schofield@intel.com
Cc: iweiny@kernel.org, nvdimm@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH v2] nvdimm/pmem: Release gendisk on probe failure
Date: Mon, 31 Aug 2026 10:04:59 +0530	[thread overview]
Message-ID: <20260831043459.1298059-1-hemanth.selam@gmail.com> (raw)

pmem namespace probe allocates a gendisk early, fills it in, and only hands
it to devres once device_add_disk() has succeeded.  Until that handover the
probe itself owns the disk, and the error paths in between release it
through a common label.

Initializing the namespace's badblocks state sits between those two points
but returns directly on failure, so the gendisk, its queue and its bdev
inode are left allocated with nothing to free them.  The devres release
action has not been registered at that point, so unbinding or destroying
the namespace does not reclaim them either, and they stay allocated for the
rest of the boot.  Nothing is user visible, as the disk was never added, so
the only effect is the memory.

devm_init_badblocks() fails only if a one page GFP_KERNEL allocation fails,
which needs memory exhaustion, so the path is hard to reach in practice.

Release the gendisk through the existing cleanup path on this failure.

Fixes: 0caeef63e6d2 ("libnvdimm: Add a poison list and export badblocks")
Assisted-by: Cursor:claude-opus-5
Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com>
---
Changes in v2, all from Alison's review of v1:
 - Retitled, and the changelog rewritten as background, problem, impact and
   resolution rather than a walk through the error labels.
 - Fixes: re-derived.  b95f5f4391fa only replaced one failing call with
   another; the early return that leaks the disk was added a week earlier by
   0caeef63e6d2, when the disk was already allocated by alloc_disk_node()
   and the only put_disk() was in the detach path.
 - Says what the leak's fate is: the devres release action is registered
   only after device_add_disk(), so nothing reclaims the disk on unbind.
 - The object counts are gone, see below for why.

How this was found: reviewing pmem's probe error paths with an AI assistant,
hence the Assisted-by tag.  The finding was then confirmed against the code
and measured as follows.

How it was tested: this is a unit test of an error path that needs memory
exhaustion to reach, not a user visible scenario.  The badblocks init in
pmem_attach_disk() was forced to fail with a throwaway kernel change, a
namespace was bound 32 times in a guest with memmap=256M!1G, and
disk_release() calls were counted with ftrace's function profiler:

                          without the patch   with the patch
  disk_release() calls          0 of 32          32 of 32
  bdev_cache active         48 -> 72 (+24)     48 -> 48 (+0)

v1 quoted the bdev_cache delta, which is what drew the question about 64
attempts and 60 objects: slab accounting does not track this one for one,
as the +24 above shows for 32 probes.  disk_release() does, so that is the
number reported here and the slab figure is only context.

 drivers/nvdimm/pmem.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 30a51c365ce8..648fc7d66063 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -563,8 +563,10 @@ static int pmem_attach_disk(struct device *dev,
 	nvdimm_namespace_disk_name(ndns, disk->disk_name);
 	set_capacity(disk, (pmem->size - pmem->pfn_pad - pmem->data_offset)
 			/ 512);
-	if (devm_init_badblocks(dev, &pmem->bb))
-		return -ENOMEM;
+	if (devm_init_badblocks(dev, &pmem->bb)) {
+		rc = -ENOMEM;
+		goto out;
+	}
 	nvdimm_badblocks_populate(nd_region, &pmem->bb, &bb_range);
 	disk->bb = &pmem->bb;
 
-- 
2.43.7


             reply	other threads:[~2026-08-31  4:35 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31  4:34 Hemanth Selam [this message]
     [not found] ` <20260831044508.520351F000E9@smtp.kernel.org>
2026-08-31  5:02   ` Hemanth Selam

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=20260831043459.1298059-1-hemanth.selam@gmail.com \
    --to=hemanth.selam@gmail.com \
    --cc=alison.schofield@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=djbw@kernel.org \
    --cc=iweiny@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    --cc=vishal.l.verma@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®