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

[Matthew Dharm <mdharm-scsi@one-eyed-alien.net>]

[-- Attachment #1: Type: text/plain, Size: 3227 bytes --]

Ah... the sweet feeling of progress.

On Fri, Jan 24, 2003 at 07:05:29PM -0500, Doug Ledford wrote:
> On Fri, Jan 24, 2003 at 03:25:40PM -0800, Matthew Dharm wrote:
> > So, if I read this correctly, you're saying that the correct sequence is:
> > 
> > (1) get disconnect notification from USB
> > (2) Call scsi_set_device_offline() (must hold host lock for this)

> > (3) call scsi_done() for all command in queue (max: 1)
> 
> Hmmm...only 1?  USB limit or driver limit?

Driver limit.  I added support for queueing, but the queue is fixed at size
1.  It's an improvement for the future.

> > (4) Call scsi_remove_host(), which should now work because no commands are
> > outstanding
> 
> > (5) Call scsi_unregister()
> > 
> > And we're done, all structures can be freed.  And, as I understand it, the
> > following is true:
> > 
> > (b) once (3) is done, (4) is guaranteed to work
> 
> No!  Remember, command completion is delayed!  We have a tasklet that 
> processes your now complete command, and with that processing comes 
> marking the device unbusy, which is also required for 4 to work.  That's 
> why I was suggesting waking up the error handler thread and letting it 
> finish this process off.  The error handler thread has the luxury of being 
> able to wait for the command completion to happen, and in my opinion it's 
> a slightly better place to do the work of cleaning out the request queue.

Okay... so what do I do if it fails?  Sleep for a while and try again
later?  Wait on a flag somewhere?

> > Tho, this does leave me with a couple of questions:
> > 
> > (i) Doesn't scsi_set_device_offline() work on devices, not hosts?  How do I
> > map from my host to my device list?
> 
> Well, in hosts.c::scsi_remove_host() we do it thusly:
> 
>         list_for_each_entry(sdev, &shost->my_devices, siblings)
>                 if (scsi_check_device_busy(sdev))
>                         return 1;

Right, perfect example.

> > (iii) What should I shove into the status field of the scsi command before
> > I scsi_done() it?
> 
> Well, to force an error I always put DID_ERROR into the driver byte of 
> the result dword, aka:
> 
> cmd->result = DID_ERROR << 16;

Sounds reasonable.

> > Async is fine with me, but up until this e-mail,
> > all I've seen is people arguing over what the sequence is, and theoretical
> > issues of what users should and should not do.  And I also think that a
> > large number of hotplugable hosts are going to replicate a whole bunch of
> > code to do (2)+(3)+(4) in one, synchronous burst.
> 
> Which would be wrong BTW.  If you can support multiple devices behind a 
> bridge then you can't put (2)+(3)+(4) together in one burst.  That's why 
> they aren't that way now.

Hrm... I can see your point if we're talking about hotplugging an
individual device, but I don't see how (2)+(3)+(4) isn't what we want for
hotplugging an entire host.

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

You are needink to look more evil.  You likink very strong coffee?
					-- Pitr to Dust Puppy
User Friendly, 10/16/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]