Re: BUG in: Driver core: convert block from raw kobjects to core devices (fwd)

[Kay Sievers <kay.sievers@vrfy.org>]

On Wed, 2007-10-31 at 11:31 -0500, James Bottomley wrote:
> On Wed, 2007-10-31 at 17:24 +0100, Kay Sievers wrote:
> > On Wed, 2007-10-31 at 11:13 -0500, James Bottomley wrote:
> > > On Wed, 2007-10-31 at 12:04 -0400, Alan Stern wrote:
> > > > On Wed, 31 Oct 2007, James Bottomley wrote:
> > > > 
> > > > > > Yes, the queue is a child of the disk.
> > > > > 
> > > > > Right, so this goes gendisk->queue (-> meaning parent of, or takes
> > > > > reference to)
> > > > 
> > > > No, no!  The _child_ takes an implicit reference to the _parent_, not 
> > > > the other way around.
> > > > 
> > > > > > > The scsi_device has a ref to the queue
> > > > > > 
> > > > > > Yeah, while the queue is a grandchild of the scsi_device with the
> > > > > > unified sysfs layout.
> > > > > 
> > > > > No, the scsi_device is a direct parent of the queue, so we have
> > > > > 
> > > > > scsi_device->queue
> > > > 
> > > > Wrong -- the gendisk is the direct parent of the queue.  The relevant 
> > > > line is in ll_rw_blk.c:blk_register_queue():
> > > > 
> > > > 	q->kobj.parent = kobject_get(&disk->dev.kobj);
> > > > 
> > > > > > Yes, sounds right. We need to break that deleted-but-wait-for-cleanup at
> > > > > > least at one of the devices involved.
> > > > > 
> > > > > But it's broken when the driver is unbound.  Diagrammatically it's:
> > > > > 
> > > > > scsi_disk -> scsi_device -> queue
> > > > >           -> gendisk     ->
> > > > > 
> > > > > It's not circular, it's released when scsi_disk is released.  It can
> > > > > become circular if there's some hidden dependency between any of the
> > > > > components ... but I don't think there is.
> > > > 
> > > > Forget about the scsi_disk.  It isn't part of the problem.  Just 
> > > > concentrate on the scsi_device, the gendisk, and the queue.  We have:
> > > > 
> > > > 	scsi_device <- gendisk <- queue <- scsi_device,
> > > 
> > > OK, so where does the gendisk get a reference to the scsi device?
> > 
> > In the unified sysfs layout where the silly and conceptual broken idea
> > of "class devices" gets removed.
> > Everything that has a "device" link today will just live below the
> > device the "device" link points to. The whole current kernel is already
> > converted to do this, besides the "raw kobject" gendisk's, and the SCSI
> > subsystem. The gendisk patch is queued in Greg's tree (see subject of
> > this mail), and the conversion from "struct class_device" to "struct
> > device" for the whole SCSI directory is coming soon.
> > 
> > With the gendisk pointing to "driverfs_dev" ("device" link) it will
> > become a child of the scsi_device.
> 
> OK, light beginning to go on now.
> 
> The problem is that you've fallen into the conceptual trap we tried very
> hard to avoid in the initial go around of joining SCSI upper layer
> drivers to gendisks.  That's why no gendisk references are held by the
> mid-layer, only by the entities that represent the objects created by
> upper layer drivers.

That will not change, only the disk will reference the device which it
points to. It's not a problem, we can "orphan" the disk on delete, or we
do the "orphaning" for all devices in the core, which is probably the
right fix anyway.

> Doesn't this circularity now exist for everything?  Every device that
> creates a queue has a reference to the queue, every queue has a
> reference to its attached gendisk and now every gendisk has a reference
> to the device creating the queue?  This doesn't look to be a SCSI
> specific problem.

It's only SCSI so far, everything else seems fine.

But, the real problem is that the core seems to deadlock if two devices
reference each other (or build a larger circle), even when they are
deleted, that's the problem we are running in.

Thanks,
Kay