* [PATCHv2] block: fix leaking minors of hidden disks
@ 2022-10-07 19:38 Keith Busch
2022-10-10 6:43 ` Daniel Wagner
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Keith Busch @ 2022-10-07 19:38 UTC (permalink / raw)
To: linux-block, axboe; +Cc: Keith Busch, Christoph Hellwig, Daniel Wagner
From: Keith Busch <kbusch@kernel.org>
The major/minor of a hidden gendisk is not propagated to the block
device. This is required to suppress it from being visible. For these
disks, we need to handle freeing the dynamic minor directly when it's
released since bdev_free_inode() won't be able to.
Cc: Christoph Hellwig <hch@lst.de>
Reported-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
v1->v2:
Actually check that the disk is hidden before assuming the minor needs
to be freed.
block/genhd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/block/genhd.c b/block/genhd.c
index 514395361d7c..0afcdbb7575c 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1166,6 +1166,8 @@ static void disk_release(struct device *dev)
if (test_bit(GD_ADDED, &disk->state) && disk->fops->free_disk)
disk->fops->free_disk(disk);
+ if ((disk->flags & GENHD_FL_HIDDEN) && disk->major == BLOCK_EXT_MAJOR)
+ blk_free_ext_minor(disk->first_minor);
iput(disk->part0->bd_inode); /* frees the disk */
}
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCHv2] block: fix leaking minors of hidden disks 2022-10-07 19:38 [PATCHv2] block: fix leaking minors of hidden disks Keith Busch @ 2022-10-10 6:43 ` Daniel Wagner 2022-10-10 6:51 ` Chaitanya Kulkarni 2022-10-10 7:36 ` Christoph Hellwig 2 siblings, 0 replies; 7+ messages in thread From: Daniel Wagner @ 2022-10-10 6:43 UTC (permalink / raw) To: Keith Busch; +Cc: linux-block, axboe, Keith Busch, Christoph Hellwig On Fri, Oct 07, 2022 at 12:38:25PM -0700, Keith Busch wrote: > From: Keith Busch <kbusch@kernel.org> > > The major/minor of a hidden gendisk is not propagated to the block > device. This is required to suppress it from being visible. For these > disks, we need to handle freeing the dynamic minor directly when it's > released since bdev_free_inode() won't be able to. > > Cc: Christoph Hellwig <hch@lst.de> > Reported-by: Daniel Wagner <dwagner@suse.de> > Signed-off-by: Keith Busch <kbusch@kernel.org> Thanks for the quick fix! Tested-by: Daniel Wagner <dwagner@suse.de> # ls -l /dev/nvme* crw------- 1 root root 10, 124 Oct 10 08:39 /dev/nvme-fabrics crw------- 1 root root 243, 0 Oct 10 08:38 /dev/nvme0 brw-rw---- 1 root disk 259, 0 Oct 10 08:38 /dev/nvme0n1 crw------- 1 root root 243, 2 Oct 10 08:41 /dev/nvme2 brw-rw---- 1 root disk 259, 2 Oct 10 08:41 /dev/nvme2n1 brw-rw---- 1 root disk 259, 3 Oct 10 08:41 /dev/nvme2n1p1 brw-rw---- 1 root disk 259, 5 Oct 10 08:41 /dev/nvme2n2 brw-rw---- 1 root disk 259, 7 Oct 10 08:41 /dev/nvme2n3 brw-rw---- 1 root disk 259, 9 Oct 10 08:41 /dev/nvme2n4 crw------- 1 root root 243, 3 Oct 10 08:41 /dev/nvme3 crw------- 1 root root 243, 4 Oct 10 08:41 /dev/nvme4 crw------- 1 root root 243, 5 Oct 10 08:41 /dev/nvme5 # nvme disconnect-all # nvme connect-all # ls -l /dev/nvme* crw------- 1 root root 10, 124 Oct 10 08:39 /dev/nvme-fabrics crw------- 1 root root 243, 0 Oct 10 08:38 /dev/nvme0 brw-rw---- 1 root disk 259, 0 Oct 10 08:38 /dev/nvme0n1 crw------- 1 root root 243, 2 Oct 10 08:41 /dev/nvme2 brw-rw---- 1 root disk 259, 2 Oct 10 08:41 /dev/nvme2n1 brw-rw---- 1 root disk 259, 3 Oct 10 08:41 /dev/nvme2n1p1 brw-rw---- 1 root disk 259, 5 Oct 10 08:41 /dev/nvme2n2 brw-rw---- 1 root disk 259, 7 Oct 10 08:41 /dev/nvme2n3 brw-rw---- 1 root disk 259, 9 Oct 10 08:41 /dev/nvme2n4 crw------- 1 root root 243, 3 Oct 10 08:41 /dev/nvme3 crw------- 1 root root 243, 4 Oct 10 08:41 /dev/nvme4 crw------- 1 root root 243, 5 Oct 10 08:41 /dev/nvme5 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2] block: fix leaking minors of hidden disks 2022-10-07 19:38 [PATCHv2] block: fix leaking minors of hidden disks Keith Busch 2022-10-10 6:43 ` Daniel Wagner @ 2022-10-10 6:51 ` Chaitanya Kulkarni 2022-10-10 7:36 ` Christoph Hellwig 2 siblings, 0 replies; 7+ messages in thread From: Chaitanya Kulkarni @ 2022-10-10 6:51 UTC (permalink / raw) To: Keith Busch, linux-block@vger.kernel.org, axboe@kernel.dk Cc: Keith Busch, Christoph Hellwig, Daniel Wagner On 10/7/22 12:38, Keith Busch wrote: > From: Keith Busch <kbusch@kernel.org> > > The major/minor of a hidden gendisk is not propagated to the block > device. This is required to suppress it from being visible. For these > disks, we need to handle freeing the dynamic minor directly when it's > released since bdev_free_inode() won't be able to. > > Cc: Christoph Hellwig <hch@lst.de> > Reported-by: Daniel Wagner <dwagner@suse.de> > Signed-off-by: Keith Busch <kbusch@kernel.org> > --- > v1->v2: > > Actually check that the disk is hidden before assuming the minor needs > to be freed. > > block/genhd.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/block/genhd.c b/block/genhd.c > index 514395361d7c..0afcdbb7575c 100644 > --- a/block/genhd.c > +++ b/block/genhd.c > @@ -1166,6 +1166,8 @@ static void disk_release(struct device *dev) > if (test_bit(GD_ADDED, &disk->state) && disk->fops->free_disk) > disk->fops->free_disk(disk); > > + if ((disk->flags & GENHD_FL_HIDDEN) && disk->major == BLOCK_EXT_MAJOR) > + blk_free_ext_minor(disk->first_minor); > iput(disk->part0->bd_inode); /* frees the disk */ > } > Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -ck ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2] block: fix leaking minors of hidden disks 2022-10-07 19:38 [PATCHv2] block: fix leaking minors of hidden disks Keith Busch 2022-10-10 6:43 ` Daniel Wagner 2022-10-10 6:51 ` Chaitanya Kulkarni @ 2022-10-10 7:36 ` Christoph Hellwig 2022-10-10 8:02 ` Daniel Wagner 2 siblings, 1 reply; 7+ messages in thread From: Christoph Hellwig @ 2022-10-10 7:36 UTC (permalink / raw) To: Keith Busch Cc: linux-block, axboe, Keith Busch, Christoph Hellwig, Daniel Wagner On Fri, Oct 07, 2022 at 12:38:25PM -0700, Keith Busch wrote: > From: Keith Busch <kbusch@kernel.org> > > The major/minor of a hidden gendisk is not propagated to the block > device. This is required to suppress it from being visible. For these > disks, we need to handle freeing the dynamic minor directly when it's > released since bdev_free_inode() won't be able to. This now leads to a different lifetime. I think the proper fix is the one below (untested): diff --git a/block/genhd.c b/block/genhd.c index d36fabf0abc1f..1752ce356484e 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -507,6 +507,8 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk, */ dev_set_uevent_suppress(ddev, 0); disk_uevent(disk, KOBJ_ADD); + } else { + disk->part0->bd_dev = ddev->devt; } disk_update_readahead(disk); ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHv2] block: fix leaking minors of hidden disks 2022-10-10 7:36 ` Christoph Hellwig @ 2022-10-10 8:02 ` Daniel Wagner 2022-10-10 8:19 ` Christoph Hellwig 0 siblings, 1 reply; 7+ messages in thread From: Daniel Wagner @ 2022-10-10 8:02 UTC (permalink / raw) To: Christoph Hellwig Cc: Keith Busch, linux-block, axboe, Keith Busch, Christoph Hellwig On Mon, Oct 10, 2022 at 12:36:26AM -0700, Christoph Hellwig wrote: > On Fri, Oct 07, 2022 at 12:38:25PM -0700, Keith Busch wrote: > > From: Keith Busch <kbusch@kernel.org> > > > > The major/minor of a hidden gendisk is not propagated to the block > > device. This is required to suppress it from being visible. For these > > disks, we need to handle freeing the dynamic minor directly when it's > > released since bdev_free_inode() won't be able to. > > This now leads to a different lifetime. I think the proper fix is > the one below (untested): > > diff --git a/block/genhd.c b/block/genhd.c > index d36fabf0abc1f..1752ce356484e 100644 > --- a/block/genhd.c > +++ b/block/genhd.c > @@ -507,6 +507,8 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk, > */ > dev_set_uevent_suppress(ddev, 0); > disk_uevent(disk, KOBJ_ADD); > + } else { > + disk->part0->bd_dev = ddev->devt; > } > > disk_update_readahead(disk); Doesn't give me the same consistent result as with Keith's version: # ls -l /dev/nvme* crw------- 1 root root 10, 124 Oct 10 10:00 /dev/nvme-fabrics crw------- 1 root root 243, 0 Oct 10 09:58 /dev/nvme0 brw-rw---- 1 root disk 259, 0 Oct 10 09:58 /dev/nvme0n1 crw------- 1 root root 243, 2 Oct 10 10:00 /dev/nvme2 brw-rw---- 1 root disk 259, 2 Oct 10 10:00 /dev/nvme2n1 brw-rw---- 1 root disk 259, 3 Oct 10 10:00 /dev/nvme2n1p1 brw-rw---- 1 root disk 259, 5 Oct 10 10:00 /dev/nvme2n2 brw-rw---- 1 root disk 259, 7 Oct 10 10:00 /dev/nvme2n3 brw-rw---- 1 root disk 259, 9 Oct 10 10:00 /dev/nvme2n4 crw------- 1 root root 243, 3 Oct 10 10:00 /dev/nvme3 crw------- 1 root root 243, 4 Oct 10 10:00 /dev/nvme4 crw------- 1 root root 243, 5 Oct 10 10:00 /dev/nvme5 # nvme disconnect-all # nvme connect-all # ls -l /dev/nvme* crw------- 1 root root 10, 124 Oct 10 10:00 /dev/nvme-fabrics crw------- 1 root root 243, 0 Oct 10 09:58 /dev/nvme0 brw-rw---- 1 root disk 259, 0 Oct 10 09:58 /dev/nvme0n1 crw------- 1 root root 243, 2 Oct 10 10:00 /dev/nvme2 brw-rw---- 1 root disk 259, 3 Oct 10 10:00 /dev/nvme2n1 brw-rw---- 1 root disk 259, 5 Oct 10 10:00 /dev/nvme2n1p1 brw-rw---- 1 root disk 259, 9 Oct 10 10:00 /dev/nvme2n2 brw-rw---- 1 root disk 259, 23 Oct 10 10:00 /dev/nvme2n3 brw-rw---- 1 root disk 259, 25 Oct 10 10:00 /dev/nvme2n4 crw------- 1 root root 243, 3 Oct 10 10:00 /dev/nvme3 crw------- 1 root root 243, 4 Oct 10 10:00 /dev/nvme4 crw------- 1 root root 243, 5 Oct 10 10:00 /dev/nvme5 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2] block: fix leaking minors of hidden disks 2022-10-10 8:02 ` Daniel Wagner @ 2022-10-10 8:19 ` Christoph Hellwig 2022-10-10 8:49 ` Daniel Wagner 0 siblings, 1 reply; 7+ messages in thread From: Christoph Hellwig @ 2022-10-10 8:19 UTC (permalink / raw) To: Daniel Wagner; +Cc: Keith Busch, linux-block, axboe, Keith Busch On Mon, Oct 10, 2022 at 10:02:18AM +0200, Daniel Wagner wrote: > Doesn't give me the same consistent result as with Keith's version: That's because it is broken.. Try this version: diff --git a/block/genhd.c b/block/genhd.c index 514395361d7c5..2296ad422523a 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -507,6 +507,8 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk, */ dev_set_uevent_suppress(ddev, 0); disk_uevent(disk, KOBJ_ADD); + } else { + disk->part0->bd_dev = MKDEV(disk->major, disk->first_minor); } disk_update_readahead(disk); ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHv2] block: fix leaking minors of hidden disks 2022-10-10 8:19 ` Christoph Hellwig @ 2022-10-10 8:49 ` Daniel Wagner 0 siblings, 0 replies; 7+ messages in thread From: Daniel Wagner @ 2022-10-10 8:49 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Keith Busch, linux-block, axboe, Keith Busch On Mon, Oct 10, 2022 at 10:19:02AM +0200, Christoph Hellwig wrote: > On Mon, Oct 10, 2022 at 10:02:18AM +0200, Daniel Wagner wrote: > > Doesn't give me the same consistent result as with Keith's version: > > That's because it is broken.. Try this version: This version works. Tested-by: Daniel Wagner <dwagner@suse.de> # ls -l /dev/nvme* crw------- 1 root root 10, 124 Oct 10 10:47 /dev/nvme-fabrics crw------- 1 root root 243, 0 Oct 10 10:46 /dev/nvme0 brw-rw---- 1 root disk 259, 0 Oct 10 10:46 /dev/nvme0n1 crw------- 1 root root 243, 2 Oct 10 10:48 /dev/nvme2 brw-rw---- 1 root disk 259, 2 Oct 10 10:48 /dev/nvme2n1 brw-rw---- 1 root disk 259, 3 Oct 10 10:48 /dev/nvme2n1p1 brw-rw---- 1 root disk 259, 5 Oct 10 10:48 /dev/nvme2n2 brw-rw---- 1 root disk 259, 7 Oct 10 10:48 /dev/nvme2n3 brw-rw---- 1 root disk 259, 9 Oct 10 10:48 /dev/nvme2n4 crw------- 1 root root 243, 3 Oct 10 10:48 /dev/nvme3 crw------- 1 root root 243, 4 Oct 10 10:48 /dev/nvme4 crw------- 1 root root 243, 5 Oct 10 10:48 /dev/nvme5 # nvme disconnect-all # nvme connect-all # ls -l /dev/nvme* crw------- 1 root root 10, 124 Oct 10 10:47 /dev/nvme-fabrics crw------- 1 root root 243, 0 Oct 10 10:46 /dev/nvme0 brw-rw---- 1 root disk 259, 0 Oct 10 10:46 /dev/nvme0n1 crw------- 1 root root 243, 2 Oct 10 10:48 /dev/nvme2 brw-rw---- 1 root disk 259, 2 Oct 10 10:48 /dev/nvme2n1 brw-rw---- 1 root disk 259, 3 Oct 10 10:48 /dev/nvme2n1p1 brw-rw---- 1 root disk 259, 5 Oct 10 10:48 /dev/nvme2n2 brw-rw---- 1 root disk 259, 7 Oct 10 10:48 /dev/nvme2n3 brw-rw---- 1 root disk 259, 9 Oct 10 10:48 /dev/nvme2n4 crw------- 1 root root 243, 3 Oct 10 10:48 /dev/nvme3 crw------- 1 root root 243, 4 Oct 10 10:48 /dev/nvme4 crw------- 1 root root 243, 5 Oct 10 10:48 /dev/nvme5 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-10 8:49 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-07 19:38 [PATCHv2] block: fix leaking minors of hidden disks Keith Busch 2022-10-10 6:43 ` Daniel Wagner 2022-10-10 6:51 ` Chaitanya Kulkarni 2022-10-10 7:36 ` Christoph Hellwig 2022-10-10 8:02 ` Daniel Wagner 2022-10-10 8:19 ` Christoph Hellwig 2022-10-10 8:49 ` Daniel Wagner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox