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

Doug Ledford
2003-01-25T00:05:29Z

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)

Yes.

> (3) call scsi_done() for all command in queue (max: 1)

Hmmm...only 1? USB limit or driver limit?

> (4) Call scsi_remove_host(), which should now work because no commands are
> outstanding

We may need to add code to scsi_remove_host() to allow it to clean out the
request queue of the device when the device is offline and this call is
made. Just because we returned the 1 command you had outstanding doesn't
mean that there weren't more in the request queue (especially true of hard
disks like my mp3 player). However, once the device is offline, cleaning
out the queue is a known non-blocking operation, it just takes non-0 time
as well. Once the queue is cleaned out, we need to shut it down so that
no more commands can come in to the block level.

> (5) Call scsi_unregister()
>
> And we're done, all structures can be freed. And, as I understand it, the
> following is true:
>
> (a) once (2) is done, no more commands will be queued

To your driver, yes. If Mike makes it clean out and disable the request
queue at the same time, then we could answer this question as yes at the
request queue level as well.

> (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.

> (c) there is nothing the user can do to make this sequence take a long time

True. We need time to do things in our very slightly async way, but the
user isn't able to keep us from completing.

> 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;

> (ii) Do I need to call scsi_set_device_offline() for each device? I
> presume 'yes'.

Yes. As people pointed out to me the reason a USB device is done as a
host is because it very well may *be* a host with several devices behind
it, so it must handle the multiple device scenario correctly and set all
devices offline and clean up after all of them that might be behind this
bridge.

> (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;

> Oh, and as for my being a 'prick'.... my big problem is that the documented
> interface is synchronous. 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. As to the sync vs. async, the scsi mid layer
quit being fully sync during the 2.4 timeframe. When the old error
handling code was dropped from 2.5+, all sync completion code was also
dropped.

> If someone will step forward with a 'yes' or 'no' on this sequence, then
> I'll get it done. If the answer is 'no', then what did I miss?

Just the tasklet completion issue.

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