RE: [PATCH v2] usb-storage: Add a limitation for blk_queue_max_hw_sectors()

[Alan Stern <stern@rowland.harvard.edu>]

On Wed, 3 Jul 2019, Yoshihiro Shimoda wrote:

> > I would really prefer to see a different solution.
> > 
> > The actual problem is that the usb_device and usb_interface structures
> > are supposed to inherit all of their DMA properties from the bus's host
> > controller.  But the existing code copies only the dma_mask and
> > dma_pfn_offset fields in the embedded device structures.  If we copied
> > all of the important DMA fields then this patch wouldn't be needed; the
> > max_sectors value for the request queue would be set up correctly to
> > begin with.
> 
> I'm sorry, but I cannot understand what are important DMA fields.

Probably all of them are important; I don't know.

> IIUC, usb-storage driver should take care of calling blk_queue_ APIs anyway because:
> 
>  - As Christoph mentioned before on the email [1], usb-storage has a special
>    max_sectors quirk for tape and SuperSpeed devices.
>  - Since blk_queue_* APIs don't take device structure pointer, the block layer
>    cannot call any DMA mapping APIs. So, even if any other DMA fields are copied,
>    the behavior is not changed.

Although the blk_queue_* APIs don't take device structure pointers, the
SCSI layer does know about devices.  And since it is the SCSI layer
which creates the request queue, changing the DMA fields should change
the behavior.

However, you are correct that usb-storage has to call the blk_queue_* 
APIs anyway.  So I guess your patch is the right thing to do after all.

Acked-by: Alan Stern <stern@rowland.harvard.edu>

I still think that copying the DMA fields would be a good idea, though.

Alan Stern

> [1]
> https://www.spinics.net/lists/linux-usb/msg181527.html
> 
> What do you think?
> 
> Best regards,
> Yoshihiro Shimoda
> 
> > So what I would like to see is a new subroutine -- perhaps in the
> > driver core -- that copies the DMA fields from one struct device to
> > another.  Then we could call this subroutine in usb_alloc_dev() and
> > usb_set_configuration() instead of copying the information manually.
> > 
> > Greg and Christoph, does that make sense?
> > 
> > Yoshihiro, would you like to write a patch that does this?
> > 
> > Alan Stern