On Fri, 2 Dec 2011, Sarah Sharp wrote: > When a UAS device stops responding to commands, it may be necessary to > issue a USB device reset (which will reset all device state, including > the device configuration and alternate interface settings). All pending > URBs for the device must be canceled and any bulk streams rings that > have been allocated for the device must be freed before the reset is > issued. > > The UAS driver needs to flush any pending URBs in the pre_reset method, > which means it needs to do four things: > > 1. It needs to get the function that queues SCSI commands > (uas_queuecommand_lck) to stop submitting URBs. > > 2. It needs to get the completion handler for the status URB > (uas_stat_cmplt) to stop submitting data URBs in the USB 2.0 UAS case. > > 3. It also needs to make sure that the function that aborts the SCSI > commands (uas_eh_abort_handler) isn't in the middle of aborting the same > command that the pre_reset method is trying to flush. Bear in mind that pre_reset isn't necessarily trying to flush any commands. It just gets called from time to time. Maybe because the device is stuck and needs to be reset; maybe because some other driver for a different interface on the same device needs to reset it. You do need to decide what to do about commands that are in flight. Ideally you would wait until they all complete by themselves (or are aborted by the error handler, which amounts to the same thing). Actively cancelling their URBs in preparation for the reset is not as safe, although it is necessary when things time out or aborts fail. > 4. It needs to ensure that the work queue that handles partially > submitted SCSI commands (uas_work) isn't trying to resubmit URBs or > reschedule itself. > > The solution is to introduce a new atomic_t, resetting. When the Why atomic_t? Is the flag going to be written from multiple threads or contexts? > resetting flag is set, uas_queuecommand_lck() should tell the SCSI layer > that the device is busy for any new commands, uas_stat_cmplt() will tell > the SCSI layer the command aborted without submitting the data URBs, and This is confusing. What you want to do is plug your queue. That is, stop sending out new commands, while waiting for the existing ones to complete. Is that what you meant? (I'm not sure exactly what uas_stat_cmplt's job is.) > uas_eh_abort_handler() should tell the SCSI layer that the abort failed. This should happen only for new abort requests arriving after you know you are going to do a reset. Aborts that are already in progress should be handled normally. One thing to watch out for is what you do when an abort fails. If you're not already waiting to do a reset, you might want to start one. > They can continue normally once pre_reset has re-setup the alternate > interface setting, reallocated the stream rings, cleared the resetting > flag. > > We use SRCU to synchronize between the readers of the resetting flag > (uas_queuecommand_lck, uas_xfer_data, and uas_eh_abort_handler) and the > writers of the flag (uas_pre_reset and uas_post_reset). We have to use > "sleepable" RCU instead of RCU because the readers must be able to sleep > while waiting for URBs to be killed, while holding the reader lock. It's not clear that this is the best approach. You already own the host lock in uas_queuecommand_lck; why not use that for synchronization? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
