usb-storage: revert DMA-alignment change for Wireless USB
Alan Stern [Mon, 30 Jun 2008 17:39:59 +0000 (13:39 -0400)]
This patch (as1110) reverts an earlier patch meant to help with
Wireless USB host controllers.  These controllers can have bulk
maxpacket values larger than 512, which puts unusual constraints on
the sizes of scatter-gather list elements.  However it turns out that
the block layer does not provide the support we need to enforce these
constraints; merely changing the DMA alignment mask doesn't help.
Hence there's no reason to keep the original patch.  The Wireless USB
problem will have to be solved a different way.

In addition, there is a reason to get rid of the earlier patch.  By
dereferencing a pointer stored in the ep_in array of struct
usb_device, the current code risks an invalid memory access when it
runs concurrently with device removal.  The members of that array are
cleared before the driver's disconnect method is called, so it should
not try to use them.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

drivers/usb/storage/scsiglue.c

index b4c9e0f..09779f6 100644 (file)
@@ -71,7 +71,6 @@ static const char* host_info(struct Scsi_Host *host)
 static int slave_alloc (struct scsi_device *sdev)
 {
        struct us_data *us = host_to_us(sdev->host);
-       struct usb_host_endpoint *bulk_in_ep;
 
        /*
         * Set the INQUIRY transfer length to 36.  We don't use any of
@@ -80,16 +79,22 @@ static int slave_alloc (struct scsi_device *sdev)
         */
        sdev->inquiry_len = 36;
 
-       /* Scatter-gather buffers (all but the last) must have a length
-        * divisible by the bulk maxpacket size.  Otherwise a data packet
-        * would end up being short, causing a premature end to the data
-        * transfer.  We'll use the maxpacket value of the bulk-IN pipe
-        * to set the SCSI device queue's DMA alignment mask.
+       /* USB has unusual DMA-alignment requirements: Although the
+        * starting address of each scatter-gather element doesn't matter,
+        * the length of each element except the last must be divisible
+        * by the Bulk maxpacket value.  There's currently no way to
+        * express this by block-layer constraints, so we'll cop out
+        * and simply require addresses to be aligned at 512-byte
+        * boundaries.  This is okay since most block I/O involves
+        * hardware sectors that are multiples of 512 bytes in length,
+        * and since host controllers up through USB 2.0 have maxpacket
+        * values no larger than 512.
+        *
+        * But it doesn't suffice for Wireless USB, where Bulk maxpacket
+        * values can be as large as 2048.  To make that work properly
+        * will require changes to the block layer.
         */
-       bulk_in_ep = us->pusb_dev->ep_in[usb_pipeendpoint(us->recv_bulk_pipe)];
-       blk_queue_update_dma_alignment(sdev->request_queue,
-                       le16_to_cpu(bulk_in_ep->desc.wMaxPacketSize) - 1);
-                       /* wMaxPacketSize must be a power of 2 */
+       blk_queue_update_dma_alignment(sdev->request_queue, (512 - 1));
 
        /*
         * The UFI spec treates the Peripheral Qualifier bits in an