On Sat, 2016-08-13 at 09:29 -0700, Dan Williams wrote: > On Sat, Aug 13, 2016 at 8:23 AM, James Bottomley > <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > > It does? The race is the fact that the parent can be removed > > before the child meaning if the parent name is re-registered before > > the child dies we get a duplicate name in bdi space. > > No, the race is that the *name* of the parent isn't released until > the child is both unregistered and put. The device core is already > ensuring that the parent is not released until all descendants have > been removed. We're both saying the same thing: the problem is that, with df08c32ce3be the bdi name lifetime is tied to the lifetime of the gendisk. However, the parent of the gendisk currently is only tied to the visibility lifetime of the gendisk, not the final put lifetime, so it doesn't see this. > > > > > So I tried the attached and it makes the libnvdimm unit tests > > > start crashing. > > > > Well, the attached is clearly buggy, isn't it? You're trying to do > > a get on the parent before the parent is actually set. > > Ah, yes, thank you. Fixed up v2 attached that passes my tests. > > > Why don't you just try the incremental patch I sent instead of > > trying to rework it? > > I reworked it because it is the bdi that holds this extra dependency > on the disk's parent, not the disk itself. Philosophically I don't like this approach. The dependency goes bdi->gendisk->parent Making the bdi manage the parent lifetime is an unusual pattern. Making the parent stay around until the last reference to gendisk is put is the usual one. > > > A couple crash logs attached. Not yet sure what assumption > > > is getting violated, but how about that conversion of scsi to use > > > dynamic devt? ;-) > > > > It's completely orthogonal. The problem is in hierarchy lifetimes: > > switching from static to dynamic allocation won't change that at > > all. You don't see this problem in nvme because the parent control > > device's lifetime belongs to the controller not the disk. In SCSI > > the parent is our representation of the SCSI device whose lifetime > > is governed at the SCSI level and effectively represents the disk. > > > > No, it's only the name. We could achieve the same by teaching the > block core to manage the "sd_index_ida" instead of the sd driver > itself, but the v2-patch attached works and does not introduce that > layering violation. Um, so this patch doesn't fix the problem. It merely makes the lifetime rules correct so the problem can then be fixed at the scsi level. James -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
