On Tue, 1 Sep 2020 at 10:07, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, Sep 01, 2020 at 08:55:07AM +0800, Tom Yan wrote: > > Hi, > > > > So a recent commit that disable 64 bit dma addressing for ASM1142: > > https://github.com/torvalds/linux/commit/ec37198acca7b4c17b96247697406e47aafe0605 > > > > I notice that it also causes (hw) max_sectors of uas drives connected > > to the controller to drop from 1024 blocks (SCSI_DEFAULT_MAX_SECTORS) > > to 512 blocks, because we clamp it against > > dma_max_mapping_size($some_device). > > > > For uas drives, $some_device is `sdev->host->dma_dev`: > > https://github.com/torvalds/linux/blob/v5.8/drivers/scsi/scsi_lib.c#L1778 > > (`shost` is `sdev->host`: > > https://github.com/torvalds/linux/blob/v5.8/drivers/scsi/scsi_lib.c#L1873) > > > > But for bot/msc drives: $some_device is some other device: > > https://github.com/torvalds/linux/blob/v5.8/drivers/usb/storage/scsiglue.c#L92 > > > > In that case, max_sectors remains to be 2048 blocks for SS BOT drives > > connected to the ASM1142 controller. > > > > I tried to change it to `sdev->host->dma_dev`. It lowered their > > max_sectors to 512 blocks. > > > > So my questions are: > > 1. Do we really need to do any clamping to max_sectors in either (uas/bot) case? > > 2. If so, should we change the $some_device in either case? > > > > For the record, in the original commit (for BOT drives): > > https://github.com/torvalds/linux/commit/d74ffae8b8dd17eaa8b82fc163e6aa2076dc8fb1 > > The commit message states that the clamping is needed because of some > > other change, which is supposed to cause max_segment_size to raise > > from 0x10000 to 0xffffffff, but when I check the sysfs files for both > > kinds of drives, all of the values are 0x10000. > > > > Also see https://github.com/virtio-win/kvm-guest-drivers-windows/issues/498 > > You should take a look at Arnd Bergmann's commit a8c06e407ef9 ("usb: > separate out sysdev pointer from usb_bus"), which is what introduced > this difference originally. > > As far as I know, both drivers (usb-storage and uas) should be using the > bus->sysdev device for the purpose of clamping max_sectors. If uas > doesn't use that device then it should be fixed. The uas driver relies on the scsi driver (https://github.com/torvalds/linux/blob/v5.8/drivers/scsi/scsi_lib.c#L1796) for max_sector setting and clamping (except when US_FL_MAX_SECTORS_64 or US_FL_MAX_SECTORS_240 is set, where the clamping will be overridden): https://github.com/torvalds/linux/blob/v5.8/drivers/usb/storage/uas.c#L816 They (uas and scsiglue/bot) call `blk_queue_max_hw_sectors` in different functions btw (`uas_slave_alloc` vs `slave_configure`, instead of `slave_alloc`, that is). I suppose we should do all the clamping and setting in `uas_slave_configure` instead as well? I faintly remember I asked about it but allow me to ask again, should we really NOT raise max_sectors to 2048 blocks for **SS UAS** drives like we do for SS BOT drives? > > And yes, the clamping is necessary. As I mentioned, if the clamping is done against the max mapping size of `bus->sysdev`, max_sectors will not be actually lowered in both cases (XHCI_NO_64BIT_SUPPORT or not). So basically it does nothing, at least on my devices. Allow me to confirm, is that a correct/expected behaviour? > > Alan Stern
