On Fri, Aug 12, 2016 at 5:29 PM, Dan Williams [off-list ref] wrote:

On Fri, Aug 12, 2016 at 5:17 PM, James Bottomley
[off-list ref] wrote:

quoted

On Fri, 2016-08-12 at 14:29 -0700, Dan Williams wrote:

quoted

Before spending effort trying to flush the destruction of old bdi
instances before new ones are registered, is it rather time to
complete the conversion of sd to only use dynamically allocated devt?

Do we have to go that far?  Surely your fix is extensible: the only
reason it doesn't work for us is that the gendisk holds the parent
without a reference, so we can free the SCSI device before its child
gendisk (good job no-one actually uses gendisk->parent after we've
released it ...).  If we fix that it would mean SCSI can't release the
sdev until after the queue is dead and the bdi namespace released, so
isn't something like this the easy fix?

James

---

diff --git a/block/genhd.c b/block/genhd.c
index fcd6d4f..54ae4ae 100644
--- a/block/genhd.c
+++ b/block/genhd.c

@@ -514,7 +514,7 @@ static void register_disk(struct device *parent, struct gendisk *disk)
        struct hd_struct *part;
        int err;

-       ddev->parent = parent;
+       ddev->parent = get_device(parent);

        dev_set_name(ddev, "%s", disk->disk_name);

@@ -1144,6 +1144,7 @@ static void disk_release(struct device *dev)
        hd_free_part(&disk->part0);
        if (disk->queue)
                blk_put_queue(disk->queue);
+       put_device(dev->parent);
        kfree(disk);
 }
 struct class block_class = {

Looks ok at first glance to me.

We do hold a reference on the parent device, but it gets dropped at
device_unregister() time and this moves it out to the final put.
However, this does leave static devt block-device-drivers that
register a disk without a parent device susceptible to the race... I
think those exist given all the drivers still using add_disk() after
commit 52c44d93c26f "block: remove ->driverfs_dev".

So I tried the attached and it makes the libnvdimm unit tests start
crashing.  A couple crash logs attached.  Not yet sure what assumption
is getting violated, but how about that conversion of scsi to use
dynamic devt? ;-)