From: Hemanth Selam <hemanth.selam@gmail.com>
To: Dan Williams <djbw@kernel.org>,
Vishal Verma <vishal.l.verma@intel.com>,
Dave Jiang <dave.jiang@intel.com>,
Alison Schofield <alison.schofield@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>,
nvdimm@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH v2] nvdimm/pmem: Release gendisk on probe failure
Date: Fri, 11 Sep 2026 13:55:43 +0530 [thread overview]
Message-ID: <20260911082543.473670-1-hemanth.selam@gmail.com> (raw)
pmem_attach_disk() allocates the gendisk with blk_alloc_disk() and hands
it to devres only once device_add_disk() has succeeded. Until that
point the probe path owns the disk itself, which is why every failure
after the allocation jumps to the out: label and puts it there.
The devm_init_badblocks() failure returns directly instead, so the disk
allocated a few lines earlier is never released. Nothing releases it
afterwards either: the devres action that would have done so has not
been registered yet, so unbinding the namespace or destroying it does
not reach the disk, and it stays allocated along with its queue and its
bdev inode until the machine is rebooted.
devm_init_badblocks() only fails when a single page allocation fails, so
reaching this at all needs memory exhaustion during namespace probe, and
because device_add_disk() has not run there is nothing user visible left
behind: no device node, no sysfs entry, only the leaked memory.
Release the gendisk through the existing cleanup path on this failure.
Fixes: 3dd60fb9d95d ("nvdimm/pmem: stop using q_usage_count as external pgmap refcount")
Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com>
---
v2, all of it from Alison's review of v1:
- retitled, and the changelog rewritten as background, problem, impact
and resolution rather than a walk through the call sequence
- says whether the disk is permanently leaked: it is, because the
devres action has not been registered at that point, so no later
unbind or destroy reaches it
- says when the failure can be reached at all, and that nothing user
visible is left behind
- the Fixes: tag re-derived. v1 blamed b95f5f4391fa, but the early
return after the disk was allocated already existed before it; that
commit only changed which call failed. The leak starts at
3dd60fb9d95d, which removed the pmem_release_queue devres action and
the fsdax_pagemap_ops .cleanup that had been freeing the disk on
these paths. accf58afb689 then converted the addr and dax_dev
returns to goto out, and this one was missed.
- the object counts kept, but measured across four batch sizes so that
the scaling is visible, and the shortfall you noticed explained
Found by an AI-assisted review of the error paths in pmem_attach_disk().
Tested on 7.3.0-rc2 in QEMU, with a legacy pmem region (memmap=1G!2G) and
a local debug patch forcing the devm_init_badblocks() branch, as it is
otherwise only reachable under memory exhaustion. namespace0.0 was bound
and unbound repeatedly with the branch forced, counting bdev_cache in
/proc/slabinfo after a drop_caches and a settle:
failed probes 32 64 128 256
growth, unfixed +24 +60 +120 +252
growth, fixed +12 +12 +12 +12
Without the patch the count tracks the number of failed probes, and the
unbind between attempts does not bring it back down, which is what makes
the leak permanent. With the patch it is flat. Both rows sit a little
under the probe count because SLUB's active_objs is an estimate, which is
the discrepancy you asked about in v1. Clearing the debug flag and
binding again still gives a working /dev/pmem0.
v1: https://lore.kernel.org/all/20260824060113.2330468-1-hemanth.selam@gmail.com/
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.48.1
next reply other threads:[~2026-09-11 8:25 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-11 8:25 Hemanth Selam [this message]
2026-09-18 17:04 ` Dave Jiang
-- strict thread matches above, loose matches on Subject: below --
2026-08-31 4:34 Hemanth Selam
[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=20260911082543.473670-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=ira.weiny@intel.com \
--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®