On Tue, Dec 21, 2021 at 11:08:11AM +0100, Christoph Hellwig wrote:
> On Fri, Dec 17, 2021 at 07:37:43PM +0900, Tetsuo Handa wrote:
> > Well, I don't think that we can remove this blk_free_ext_minor() call, for
> > this call is releasing disk->first_minor rather than MINOR(bdev->bd_dev).
> >
> > Since bdev_add(disk->part0, MKDEV(disk->major, disk->first_minor)) is not
> > called when reaching the out_free_ext_minor label,
> >
> > if (MAJOR(bdev->bd_dev) == BLOCK_EXT_MAJOR)
> > blk_free_ext_minor(MINOR(bdev->bd_dev));
> >
> > in bdev_free_inode() will not be called because MAJOR(bdev->bd_dev) == 0
> > because bdev->bd_dev == 0.
> >
> > I think we can apply this patch as-is...
>
> With the patch as-is we'll still leak disk->ev if device_add fails.
> Something like the patch below should solve that by moving the disk->ev
> allocation later and always cleaning it up through disk->release:
Sorry, this still had the extra return.
diff --git a/block/genhd.c b/block/genhd.c
index 3c139a1b6f049..603db5d6f10c0 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -442,10 +442,6 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
disk->first_minor = ret;
}
- ret = disk_alloc_events(disk);
- if (ret)
- goto out_free_ext_minor;
-
/* delay uevents, until we scanned partition table */
dev_set_uevent_suppress(ddev, 1);
@@ -456,7 +452,12 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
ddev->devt = MKDEV(disk->major, disk->first_minor);
ret = device_add(ddev);
if (ret)
- goto out_disk_release_events;
+ goto out_free_ext_minor;
+
+ ret = disk_alloc_events(disk);
+ if (ret)
+ goto out_device_del;
+
if (!sysfs_deprecated) {
ret = sysfs_create_link(block_depr, &ddev->kobj,
kobject_name(&ddev->kobj));
@@ -538,8 +539,6 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
sysfs_remove_link(block_depr, dev_name(ddev));
out_device_del:
device_del(ddev);
-out_disk_release_events:
- disk_release_events(disk);
out_free_ext_minor:
if (disk->major == BLOCK_EXT_MAJOR)
blk_free_ext_minor(disk->first_minor);
