Re: [linux-usb-devel] Re: [PATCH] USB changes for 2.5.58

[Doug Ledford <dledford@redhat.com>]

On Thu, Jan 23, 2003 at 09:59:28PM +0100, Oliver Neukum wrote:
> Hi Doug
> 
> > Actually, I would have both complicated and simple transports call
> > scsi_set_device_offline() and for two reasons.  1) you have to provide
> > that function for simple drivers so duplicating other detection code in
> > the scsi completion handler is a waste.  2) pretty much all transports
> > will learn of the device being offline while they are in their interrupt
> > handler and should already be holding the lock for the device, which means
> 
> This is not the case for USB and IEEE1394. I am not sure about PCMCIA.
> We are in context of a kernel thread while we learn about device removal.

No.  You might be in a kernel thread context when you decode an interrupt 
down to determining that a device was removed, but somewhere along the 
line you took an interrupt that told you the device was removed (or else 
the command simply timed out and you are in the error handler for the 
command already).  Are you saying that the USB subsystem queues up those 
interrupt packets and decodes them later (which is fine, I just want to be 
clear on the point)?

> > that calling scsi_set_device_offline() won't race with scsi_request_fn()
> > which also needs the device lock (which in reality is the host lock).
> > Saving this race is convenient enough IMHO to warrant saying that's the
> > way things need to be.
> >
> > > > scsi_set_device_offline(dev) calls a high-level kernel function to
> > > > start higher level things (block queue cut off, etc) which *may* need
> > > > to be done.
> >
> > No, scsi_set_device_offline() schedules the error handler thread for that
> > host to be woken up.
> >
> > > How do you differentiate between real failure and device removal?
> >
> > We don't, and we shouldn't.  Device removal *is* a real failure.
> 
> Well shouldn't a device removal remove the device as a logical
> entity and a failure should not?

No.  That's what the user space hot plug manager is for.  If you want this 
type of behaviour, you take an interrupt to tell you that the device is 
gone, you mark it gone, the error handler cleans up any outstanding 
commands, then once the device no longer has any commands outstanding 
*then* the hot plug manager can successfully umount/unattach/whatever the 
device and then tell the kernel to actually remove it.  Putting this into 
the scsi stack when it's already in place elsewhere makes no sense to me.

> > If the LLDD is the type such that it knows the device is gone (aka, in my
> > driver if I get a selection timeout then I know something is fishy and can
> > proceed from there, iSCSI may not be so lucky), then it has one of two
> > choices.  1) it may flush any commands that it can out of the hardware and
> > return them immediately with the same error condition as the one that it
> > is already returning.  2) it can sit and wait for the commands to timeout
> > one by one if that's what it wants.  Since the device has already been
> > marked offline by scsi_set_device_offline() and the error handler thread
> > is already scheduled to run for the device, 2 is probably the easiest
> > thing for the driver to do.  The error handler will call the abort/reset
> 
> Again not for USB and IEEE1394. We'd have to wait for the error handler
> to finish. Doing it ourselves is easier.

OK, are you reading my comments or not?  I said "since the error handler 
thread is already scheduled to run for the device, 2 is probably easiest".  
In other words, you don't have to wait for anything, it's gonna happen 
post-haste.  So since you should already have proper error handling 
functions in place (You do have proper error handler functions in place, 
don't you?), duplicating that code here won't really buy you anything.

> > Once all the commands are gone and no more are arriving, then if, and only
> > if, someone actually removes the device from the scsi subsystem (maybe
> > hotplug manager or something) then you will get the typical
> > slave_destroy() call to tell you that it is safe to release all resources
> > related to this device.  Otherwise, the device will hang around as an
> > offline device until someone does echo "scsi-remove-single-device a b c d"
> 
> Eek. That part I must strongly object to. The device is physically gone.
> Ever bothering the LLDD with it is very inconvinient.

OK, let's look at this realistically.  I'm saying you get an interrupt 
telling you that the device is gone and you tell the scsi core the same 
thing.  Immediately after that the scsi core calls your error handler 
routines to clean up any pending commands on the device.  Once all those 
pending commands are cleaned up, the hot plug manager is free to remove 
the device from the system.  Once the hot plug manager calls for the free 
to happen, you get a slave_destroy() call and you free the instances.  
This all happens in a span of a few milliseconds most likely.  Is that 
really so inconvenient for you?

> > > /proc/scsi/scsi to remove it.
> >
> > Basically, as I see it, we need a new function scsi_set_device_offline()
> > that marks the device offline, we need an offline check in
> 
> These functions are needed for a whole bus as well. USB needs it.
> 
> > As far as plugging back in, the answer is simple.  Until the old instance
> > is dead *and removed* a new one can't be added at the same ID, aka you
> > simply ignore the hot plug until the hot remove has completed.
> 
> What do you mean? It is dead because it is removed. How can a device be
> anything than dead if it has been unplugged? Please elaborate.

I said "old instance", aka the internal data structs (struct scsi_device 
for that device).  A device can be dead but not removed from the scsi 
subsys if no one has cleaned up after the removal by unmounting any 
filesystems that were on it and removing the scsi device itself.  That 
would be the job of the hotplug manager.

> And who should ignore a hot addition, the LLDD or SCSI core.
> If the former, again I must object.

The scsi core doesn't allow two devices with the same complete ID set.  
You would either have to attach the device at a different ID (aka khubd 
could set the reattached device to a higher SCSI ID or something) or wait 
for the hot plug manager to complete the old instance of the device's 
removal before adding the device back in again.

-- 
  Doug Ledford <dledford@redhat.com>     919-754-3700 x44233
         Red Hat, Inc. 
         1801 Varsity Dr.
         Raleigh, NC 27606