| Message ID | Pine.LNX.4.44L0.1910231132470.1878-100000@iolanthe.rowland.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | UAS: Revert commit 3ae62a42090f ("UAS: fix alignment of scatter/gather segments") | expand |
On Wed, Oct 23, 2019 at 11:34:33AM -0400, Alan Stern wrote: > Commit 3ae62a42090f ("UAS: fix alignment of scatter/gather segments"), > copying a similar commit for usb-storage, attempted to solve a problem > involving scatter-gather I/O and USB/IP by setting the > virt_boundary_mask for mass-storage devices. > > However, it now turns out that the analogous change in usb-storage > interacted badly with commit 09324d32d2a0 ("block: force an unlimited > segment size on queues with a virt boundary"), which was added later. > A typical error message is: > > ehci-pci 0000:00:13.2: swiotlb buffer is full (sz: 327680 bytes), > total 32768 (slots), used 97 (slots) > > There is no longer any reason to keep the virt_boundary_mask setting > in the uas driver. It was needed in the first place only for > handling devices with a block size smaller than the maxpacket size and > where the host controller was not capable of fully general > scatter-gather operation (that is, able to merge two SG segments into > a single USB packet). But: > > High-speed or slower connections never use a bulk maxpacket > value larger than 512; > > The SCSI layer does not handle block devices with a block size > smaller than 512 bytes; > > All the host controllers capable of SuperSpeed operation can > handle fully general SG; > > Since commit ea44d190764b ("usbip: Implement SG support to > vhci-hcd and stub driver") was merged, the USB/IP driver can > also handle SG. > > Therefore all supported device/controller combinations should be okay > with no need for any special virt_boundary_mask. So in order to head > off potential problems similar to those affecting usb-storage, this > patch reverts commit 3ae62a42090f. > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > CC: Oliver Neukum <oneukum@suse.com> > CC: <stable@vger.kernel.org> Looks good, Acked-by: Christoph Hellwig <hch@lst.de>
Index: usb-devel/drivers/usb/storage/uas.c =================================================================== --- usb-devel.orig/drivers/usb/storage/uas.c +++ usb-devel/drivers/usb/storage/uas.c @@ -789,30 +789,10 @@ static int uas_slave_alloc(struct scsi_d { struct uas_dev_info *devinfo = (struct uas_dev_info *)sdev->host->hostdata; - int maxp; sdev->hostdata = devinfo; /* - * We have two requirements here. We must satisfy the requirements - * of the physical HC and the demands of the protocol, as we - * definitely want no additional memory allocation in this path - * ruling out using bounce buffers. - * - * For a transmission on USB to continue we must never send - * a package that is smaller than maxpacket. Hence the length of each - * scatterlist element except the last must be divisible by the - * Bulk maxpacket value. - * If the HC does not ensure that through SG, - * the upper layer must do that. We must assume nothing - * about the capabilities off the HC, so we use the most - * pessimistic requirement. - */ - - maxp = usb_maxpacket(devinfo->udev, devinfo->data_in_pipe, 0); - blk_queue_virt_boundary(sdev->request_queue, maxp - 1); - - /* * The protocol has no requirements on alignment in the strict sense. * Controllers may or may not have alignment restrictions. * As this is not exported, we use an extremely conservative guess.
Commit 3ae62a42090f ("UAS: fix alignment of scatter/gather segments"), copying a similar commit for usb-storage, attempted to solve a problem involving scatter-gather I/O and USB/IP by setting the virt_boundary_mask for mass-storage devices. However, it now turns out that the analogous change in usb-storage interacted badly with commit 09324d32d2a0 ("block: force an unlimited segment size on queues with a virt boundary"), which was added later. A typical error message is: ehci-pci 0000:00:13.2: swiotlb buffer is full (sz: 327680 bytes), total 32768 (slots), used 97 (slots) There is no longer any reason to keep the virt_boundary_mask setting in the uas driver. It was needed in the first place only for handling devices with a block size smaller than the maxpacket size and where the host controller was not capable of fully general scatter-gather operation (that is, able to merge two SG segments into a single USB packet). But: High-speed or slower connections never use a bulk maxpacket value larger than 512; The SCSI layer does not handle block devices with a block size smaller than 512 bytes; All the host controllers capable of SuperSpeed operation can handle fully general SG; Since commit ea44d190764b ("usbip: Implement SG support to vhci-hcd and stub driver") was merged, the USB/IP driver can also handle SG. Therefore all supported device/controller combinations should be okay with no need for any special virt_boundary_mask. So in order to head off potential problems similar to those affecting usb-storage, this patch reverts commit 3ae62a42090f. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> CC: Oliver Neukum <oneukum@suse.com> CC: <stable@vger.kernel.org> --- [as1923] drivers/usb/storage/uas.c | 20 -------------------- 1 file changed, 20 deletions(-)