Re: [PATCH 25/20] sysfs: Only support removing emtpy sysfs directories.

[James Bottomley]

On Wed, 2009-05-27 at 16:40 -0400, Alan Stern wrote:
> On Wed, 27 May 2009, James Bottomley wrote:
> 
> > On Wed, 2009-05-27 at 14:07 -0400, Alan Stern wrote:
> > > On Wed, 27 May 2009, James Bottomley wrote:
> > > 
> > > > By refcount, I mean count of underlying devices.
> > > 
> > > Does that mean only registered devices, or does it include devices 
> > > which are unregistered but not yet released?
> > 
> > All devices ... scsi_device has to has a target parent before its
> > usable.
> 
> I can't tell whether you understood my point.  After a scsi_device is
> unregistered but before it is released -- i.e., when its state is
> SDEV_DEL -- it _is_ essentially unusable.  So why wait until it is
> released to decrement the target's device counter?  Why not do the
> decrement in __scsi_remove_device()?

because the use model of the device still requires a valid target.  Even
though it gets gated in several places in SDEV_DEL, we still have use of
the target parent.  This is fixable, but only by a long audit of all the
sdev uses plus the enforcement of no use of target in DEL state rule,
which adds complexity.

> > > > Um, well, that's roughly how I said we'd have to fix all of this in the
> > > > email to hannes ... it would be much easier if we could make a del'd
> > > > device visible,
> > > 
> > > I don't follow.  Why would you want to delete a target before the host
> > > is removed and then make it visible again later?  Because it doesn't
> > > have any underlying devices at the moment but may gain some later on?
> > 
> > Perhaps I haven't made the problem clear enough.  You only want early
> > del if the host is going away, otherwise the target might be reused and
> > it can't be if you've called del on it.  So there needs to be an
> > integration into the host lifecycle in some form.
> 
> Yes, granted.  That integration doesn't have to be complicated.  
> Basically, you just decrement the counters in all the targets when
> setting a host's state to SHOST_DEL or SHOST_DEL_RECOVERY.  At that 

And SHOST_CANCEL and SHOST_CANCEL_RECOVERY.

> point there's no reason to keep an unpopulated target around, right?

If the child list were empty, sure.  However, it's likely not going to
be at this point.

> Up until that point, the counter's value should be one more than the
> number of underlying sdevs.  So if the counter decrements to 0 then
> there were no underlying sdevs and the target is deleted immediately;
> otherwise it is deleted when the last remaining sdev is deleted.

No, that's the problem.  It can be removed from visibility if it has no
visible sdevs, but it can't be deleted until the last sdev is released.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/