* [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, axboe
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 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.