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

Matthew Dharm
2003-01-25T00:45:53Z

[-- 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 --]