On Wed, 23 Oct 2002, Oliver Neukum wrote: > Am Mittwoch, 23. Oktober 2002 15:41 schrieb Alan Stern: > > The problem of immediately cancelling all URBs when a disconnect occurs > > exists throughout the usb-storage driver, not just in usb_stor_clear_halt > > and usb_stor_reset_common. It needs to be solved at a higher level. > > If you can come up with a workable scheme to do that, I'd like to see it.
For usb-storage, it's not hard. There's already a bit in the flags word (in struct us_data) that indicates whether or not the device is connected; it's called US_FL_DEV_ATTACHED. We can easily change the code so that this bit is set/cleared/tested using the atomic set_bit/clear_bit/test_bit routines. Then the disconnect handler can clear this bit, and the routines that submit URBs can test it before calling usb_submit_urb. I'm going to guess that you will object to the race condition that exists between the time the flag is tested and the URB is submitted. Yes, there's a race. But it is unavoidable, unless you want to surround every call to the usb core with a spinlock_irq or the equivalent. And in any case it doesn't matter, since we only want to guarantee that no URB will be submitted after disconnect() _returns_. We don't need to (and in fact cannot) guarantee that no URB will be submitted after disconnect() _is called_. Perhaps the real question is: Do we need to guarantee that no URBs will remain outstanding after disconnect() returns? This seems to be what you are getting at. Unfortunately, I haven't seen written down anywhere exactly what conditions the driver needs to satisfy. It would be nice if this sort of thing could be added to the documentation somewhere. > > Here's a suggestion that would help all existing usb drivers. Add an > > additional parameter to usb_control_msg (and the other synchronous message > > subroutines as well) that takes a struct urb **. The subroutines can use > > it to store a pointer to the dynamically allocated URB, so that the driver > > would have the ability to cancel the URB whenever it wanted. > > No, not whenever, only after the synchronous call has set the pointer. > Which you cannot know. I'm afraid you're right about that. Maybe the synchronous call routines should be eliminated entirely. > I think that it won't work. The correct sequence is: > ... > down() > if (device_not_disconnected) > synchronous_call() > up() > ... > > and in disconnect(): > > down() > mark_as_disconnected() > up() > > As you can see, the check for disconnection cannot be in the synchronous > helper. I'm not clear on how this helps. Yes, if you do it this way then you never have to cancel a synchronous message. But instead you force disconnect() to hang around waiting until the synchronous message completes. Is that really any better? And if it is, why not have disconnect() do the mark_as_disconnected() before doing the down()? > The solution you suggest only works if you can ensure that the synchronous > call has given you a handle to unlink the urb before disconnect() runs. > To do so requires the synchronous helper to be called with a lock against > disconnect() already held. Well no, I only need to insure that the synchronous call has given me a handle before disconnect() _returns_. But I agree that it's a problem. My feeling is that we need either to rethink the synchronous calls or get rid of them altogether. There's another aspect of usb-storage that runs into difficulties with regard to synchronous calls. The problem is that most of the things that usb-storage does come about because of calls from the scsi layer, and most of these calls are protected by a timeout and abort handler. But not all of them are, and also there are a few things usb-storage does on its own behalf (initialization, mostly) that also do not have the protection of an abort handler. As a result, we have to be able to abort some calls on demand but not others, and we have to use timeouts for some calls but not others. It's a real mess, and we haven't gotten it straightened out yet. My vote goes for making things as uniform as possible, but Matt Dharm doesn't agree. He has said that the driver should use different code paths for the different situations. Maybe this discussion about the problems with synchronous calls will make us rethink the whole question. Alan Stern ------------------------------------------------------- This sf.net email is sponsored by: Influence the future of Java(TM) technology. Join the Java Community Process(SM) (JCP(SM)) program now. http://ads.sourceforge.net/cgi-bin/redirect.pl?sunm0002en _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
