Re: [PATCH] scsi_device refcounting and list lockdown

[Christoph Hellwig <hch@lst.de>]

On Mon, Oct 27, 2003 at 06:32:35PM -0800, Mike Anderson wrote:
> > +	/*
> > +	 * Ok, this look a bit strange.  We always look for the first device
> > +	 * on the list as scsi_remove_device removes them from it - thus we
> > +	 * also have to release the lock.
> > +	 * We don't need to get another reference to the device before
> > +	 * releasing the lock as we already own the reference from
> > +	 * scsi_register_device that's release in scsi_remove_device.  And
> > +	 * after that we don't look at sdev anymore.
> > +	 */
> > +	spin_lock_irqsave(shost->host_lock, flags);
> > +	list_for_each_entry_safe(sdev, tmp, &shost->__devices, siblings) {
> > +		spin_unlock_irqrestore(shost->host_lock, flags);
> >  		scsi_remove_device(sdev);
> > +		spin_lock_irqsave(shost->host_lock, flags);
> >  	}
> > +	spin_unlock_irqrestore(shost->host_lock, flags);
> >  }
> >  
> >  /*
> 
> Do we still have issues with other callers of scsi_remove_device?

Well, the other callers aren't walking the list until it's empty but
just remove a single device which is fine with the existing constructs.

> >  void scsi_remove_device(struct scsi_device *sdev)
> >  {
> > -	struct class *class = class_get(&sdev_class);
> > -
> >  	class_device_unregister(&sdev->sdev_classdev);
> > -
> > -	if (class) {
> > -		down_write(&class->subsys.rwsem);
> > -		set_bit(SDEV_DEL, &sdev->sdev_state);
> > -		if (sdev->host->hostt->slave_destroy)
> > -			sdev->host->hostt->slave_destroy(sdev);
> > -		device_del(&sdev->sdev_gendev);
> > -		up_write(&class->subsys.rwsem);
> > -	}
> > -
> > +	if (sdev->host->hostt->slave_destroy)
> > +		sdev->host->hostt->slave_destroy(sdev);
> > +	device_del(&sdev->sdev_gendev);
> >  	put_device(&sdev->sdev_gendev);
> > -
> > -	class_put(&sdev_class);
> >  }
> >  
> 
> Don't we still want to set SDEV_DEL to keep callers to scsi_device_get
> from do more gets.

Indeed!  James, can you add this back when applying?