Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Mon, 19 Jan 2015, Alan Stern wrote:
...
If anyone (still?) cares about this bug, commit 3a9794d3 ("sd: Fix max
transfer length for 4k disks") fixes it, with no patches required.
-Kenny
--
Kenneth R. Crudup Sr. SW Engineer, Scott County Consulting, Silicon Valley
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Mon, 19 Jan 2015, Alan Stern wrote: > Are you certain the patch had been applied to the kernel you were testing? Yeah, I definitely remember the result of "git status" having the patch ready for commit at the time of build. I'll test it again later tonight. > Maybe you can add a printk statement to scsiglue.c:slave_configure() Yeah, I'll add this too. -Kenny -- Kenneth R. Crudup Sr. SW Engineer, Scott County Consulting, Silicon Valley -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Mon, 19 Jan 2015, Kenneth R. Crudup wrote: > Sorry for taking so long to get back to you guys about this (the 3.19-rc > series has been > problematic for me in a couple of areas, so I'd let it go for a while): > > On Mon, 5 Jan 2015, Alan Stern wrote: > > > The patch I posted sets a general limit of 32 MB for USB drives that > > don't have a quirk flag for a smaller limit. > > Kenneth, have you tried that patch? Does it fix your problem? > > No, it fails in the same way (applied to 3.19-rc5): > > > Jan 19 01:38:16 tosh-p75a kernel: [ 268.606258] sd 5:0:0:0: [sdc] FAILED > Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK > Jan 19 01:38:16 tosh-p75a kernel: [ 268.606262] sd 5:0:0:0: [sdc] CDB: > Jan 19 01:38:16 tosh-p75a kernel: [ 268.606263] Write(10): 2a 00 0f e8 e0 40 > 00 3c 00 00 > Jan 19 01:38:16 tosh-p75a kernel: [ 268.606268] blk_update_request: I/O > error, dev sdc, sector 213536 > Jan 19 01:38:16 tosh-p75a kernel: [ 268.839597] sd 5:0:0:0: [sdc] FAILED > Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK > Jan 19 01:38:16 tosh-p75a kernel: [ 268.839603] sd 5:0:0:0: [sdc] CDB: > Jan 19 01:38:16 tosh-p75a kernel: [ 268.839605] Write(10): 2a 00 0f e9 68 40 > 00 3c 00 00 > Jan 19 01:38:16 tosh-p75a kernel: [ 268.839615] blk_update_request: I/O > error, dev sdc, sector 2135638528 > > Jan 19 01:38:47 tosh-p75a kernel: [ 300.103494] usb 2-4.2: reset SuperSpeed > USB device number 3 using xhci_hcd > Jan 19 01:38:47 tosh-p75a kernel: [ 300.115145] xhci_hcd :00:14.0: xHCI > xhci_drop_endpoint called with disabled ep 880448c9bf00 > Jan 19 01:38:47 tosh-p75a kernel: [ 300.115150] xhci_hcd :00:14.0: xHCI > xhci_drop_endpoint called with disabled ep 880448c9bf48 > Jan 19 01:38:48 tosh-p75a kernel: [ 300.443729] INFO: task kworker/u16:6:197 > blocked for more than 30 seconds. > > > FWIW, my quick fix gets me going again: > > > diff --git a/block/blk-settings.c b/block/blk-settings.c > index 6ed2cbe..3a10bf6 100644 > --- a/block/blk-settings.c > +++ b/block/blk-settings.c > @@ -257,7 +257,8 @@ void blk_limits_max_hw_sectors(struct queue_limits > *limits, unsigned int max_hw_ >__func__, max_hw_sectors); > } > > - limits->max_sectors = limits->max_hw_sectors = max_hw_sectors; > + limits->max_sectors = limits->max_hw_sectors = > + min_t(unsigned int, max_hw_sectors, 65535); > } > EXPORT_SYMBOL(blk_limits_max_hw_sectors); Are you certain the patch had been applied to the kernel you were testing? The patch adds a line that calls blk_queue_max_hw_sectors() with max_hw_sectors set to 65535, so it should have the same effect as your quick fix. Maybe you can add a printk statement to scsiglue.c:slave_configure() in drivers/usb/storage to make sure that the patch's call really does take place as intended. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
Sorry for taking so long to get back to you guys about this (the 3.19-rc series has been problematic for me in a couple of areas, so I'd let it go for a while): On Mon, 5 Jan 2015, Alan Stern wrote: > The patch I posted sets a general limit of 32 MB for USB drives that > don't have a quirk flag for a smaller limit. > Kenneth, have you tried that patch? Does it fix your problem? No, it fails in the same way (applied to 3.19-rc5): Jan 19 01:38:16 tosh-p75a kernel: [ 268.606258] sd 5:0:0:0: [sdc] FAILED Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK Jan 19 01:38:16 tosh-p75a kernel: [ 268.606262] sd 5:0:0:0: [sdc] CDB: Jan 19 01:38:16 tosh-p75a kernel: [ 268.606263] Write(10): 2a 00 0f e8 e0 40 00 3c 00 00 Jan 19 01:38:16 tosh-p75a kernel: [ 268.606268] blk_update_request: I/O error, dev sdc, sector 213536 Jan 19 01:38:16 tosh-p75a kernel: [ 268.839597] sd 5:0:0:0: [sdc] FAILED Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK Jan 19 01:38:16 tosh-p75a kernel: [ 268.839603] sd 5:0:0:0: [sdc] CDB: Jan 19 01:38:16 tosh-p75a kernel: [ 268.839605] Write(10): 2a 00 0f e9 68 40 00 3c 00 00 Jan 19 01:38:16 tosh-p75a kernel: [ 268.839615] blk_update_request: I/O error, dev sdc, sector 2135638528 Jan 19 01:38:47 tosh-p75a kernel: [ 300.103494] usb 2-4.2: reset SuperSpeed USB device number 3 using xhci_hcd Jan 19 01:38:47 tosh-p75a kernel: [ 300.115145] xhci_hcd :00:14.0: xHCI xhci_drop_endpoint called with disabled ep 880448c9bf00 Jan 19 01:38:47 tosh-p75a kernel: [ 300.115150] xhci_hcd :00:14.0: xHCI xhci_drop_endpoint called with disabled ep 880448c9bf48 Jan 19 01:38:48 tosh-p75a kernel: [ 300.443729] INFO: task kworker/u16:6:197 blocked for more than 30 seconds. FWIW, my quick fix gets me going again: diff --git a/block/blk-settings.c b/block/blk-settings.c index 6ed2cbe..3a10bf6 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -257,7 +257,8 @@ void blk_limits_max_hw_sectors(struct queue_limits *limits, unsigned int max_hw_ __func__, max_hw_sectors); } - limits->max_sectors = limits->max_hw_sectors = max_hw_sectors; + limits->max_sectors = limits->max_hw_sectors = + min_t(unsigned int, max_hw_sectors, 65535); } EXPORT_SYMBOL(blk_limits_max_hw_sectors); -Kenny -- Kenneth R. Crudup Sr. SW Engineer, Scott County Consulting, Silicon Valley -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
I will later tonight. For now, I'm using 3.18.x (for another reason), which doesn't have this commit. My Linus-latest tree has a "fix" that imposed a hard limit of 32767, which worked for me then. On January 5, 2015 12:07:56 PM PST, Alan Stern wrote: >On Mon, 5 Jan 2015, Christoph Hellwig wrote: > >> On Tue, Dec 30, 2014 at 08:36:34AM -0800, Kenneth R. Crudup wrote: >> > OP here. FWIW, this is what I get when running that command on the >SCSI >> > generic device that corresponds to the USB-3 (non-UAS) disk[1] that >had the >> > issue: >> >> So it looks like this one actually provides sane values, but we don't >> we never even look at EVPD pages for usb devices due to the wrong >SCSI >> level? > >According to James Bottomley, it doesn't matter much what the EVPD >pages say. The limitation is imposed by the USB _bridge_, whereas the >EVPD data indicates what the _drive_ is capable of. So we can't rely >on that data anyway. (Although if the EVPD data indicates a limit >smaller than the bridge can handle, then we'd need to pay attention to >it.) > >The patch I posted sets a general limit of 32 MB for USB drives that >don't have a quirk flag for a smaller limit. > >Kenneth, have you tried that patch? Does it fix your problem? > >Alan Stern -- Sent from my 2014 Galaxy Note 10.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Mon, 5 Jan 2015, Christoph Hellwig wrote: > On Tue, Dec 30, 2014 at 08:36:34AM -0800, Kenneth R. Crudup wrote: > > OP here. FWIW, this is what I get when running that command on the SCSI > > generic device that corresponds to the USB-3 (non-UAS) disk[1] that had the > > issue: > > So it looks like this one actually provides sane values, but we don't > we never even look at EVPD pages for usb devices due to the wrong SCSI > level? According to James Bottomley, it doesn't matter much what the EVPD pages say. The limitation is imposed by the USB _bridge_, whereas the EVPD data indicates what the _drive_ is capable of. So we can't rely on that data anyway. (Although if the EVPD data indicates a limit smaller than the bridge can handle, then we'd need to pay attention to it.) The patch I posted sets a general limit of 32 MB for USB drives that don't have a quirk flag for a smaller limit. Kenneth, have you tried that patch? Does it fix your problem? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Tue, Dec 30, 2014 at 08:36:34AM -0800, Kenneth R. Crudup wrote: > OP here. FWIW, this is what I get when running that command on the SCSI > generic device that corresponds to the USB-3 (non-UAS) disk[1] that had the > issue: So it looks like this one actually provides sane values, but we don't we never even look at EVPD pages for usb devices due to the wrong SCSI level? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Tue, Dec 30, 2014 at 11:19:07AM -0500, Douglas Gilbert wrote: > In SCSI, the VPD page 0xb0 (Block limits) has a "Maximum transfer > length" field (32 bits long). Its units are logical blocks. Useful > if >0 as 0 means "unreported". > > USB to SATA bridges should comply with SAT. However SAT and SAT-2 > don't even mention that VPD page. SAT-3 and SAT-4 mention that page > but have "unspecified" next to that field. Basically useless. Maybe we need to cap the max sectors value to something fairly low unless we have the block limits VPD page and it contains useful information. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Tue, 2014-12-30 at 11:45 -0500, Alan Stern wrote: > PS: What's the current situation of my "SCSI: fix regression in > scsi_send_eh_cmnd()" patch: > > http://marc.info/?l=linux-scsi&m=141658469207765&w=2 > > submitted on November 21? Since it was a bug-fix, I rather expected it > to get merged before 3.18 was released. Since it didn't, I certainly > hope it will get in before 3.19. Looks like it just got overlooked. Since Hannes has reviewed it, I'll add it to the fixes branch. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Tue, 30 Dec 2014, James Bottomley wrote: > > All right. How does this patch look? > > OK, I suppose. The transfer limits are a little on the low side, but > for usb-storage (i.e. non-UAS) performance devices, they should be OK. If you're referring to the 32-KB and 64-KB limits, we know from experience that some devices really do need them. If you're referring to the 32-MB limit... Well, that's what this whole thread is about, right? That limit could be restricted to apply only when a device is not connected over a SuperSpeed USB-3 link. But knowing the low quality of commodity USB hardware, I suspect it wouldn't work well. > For TYPE_TAPE, you still have no guarantee that the bridge won't screw > up ... and if the argument is that tapes are always connected to > sensible bridges, why aren't SATA devices? True, we have no guarantee. But tape drives do have special requirements because of the way they write blocks and gaps; this code was added by someone with a tape drive who did need the large limit. I guess there are a lot more bridges in the low-budget consumer world targeted to disk drives than to tape drives. That could explain a lot. > There's also a spelling mistake below. I'll fix it in the final patch submission. Thanks. Alan Stern PS: What's the current situation of my "SCSI: fix regression in scsi_send_eh_cmnd()" patch: http://marc.info/?l=linux-scsi&m=141658469207765&w=2 submitted on November 21? Since it was a bug-fix, I rather expected it to get merged before 3.18 was released. Since it didn't, I certainly hope it will get in before 3.19. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Tue, 30 Dec 2014, Douglas Gilbert wrote: > IMO about the best SATL is in the MPT SAS-2 and SAS-3 HBAs and > here is that page for a SAS expander attached SATA disk: > # sg_vpd -p bl /dev/sg3 OP here. FWIW, this is what I get when running that command on the SCSI generic device that corresponds to the USB-3 (non-UAS) disk[1] that had the issue: $ sudo sg_vpd -p bl /dev/sg4 Block limits VPD page (SBC): Write same no zero (WSNZ): 0 Maximum compare and write length: 0 blocks Optimal transfer length granularity: 1 blocks Maximum transfer length: 8191 blocks Optimal transfer length: 8191 blocks Maximum prefetch length: 8191 blocks Maximum unmap LBA count: 0 Maximum unmap block descriptor count: 0 Optimal unmap granularity: 0 Unmap granularity alignment valid: 0 Unmap granularity alignment: 0 Maximum write same length: 0x0 blocks $ If more info is needed, just let me know. -Kenny [1] - SCSI generic devices follow "/proc/scsi/scsi" order, right? -- Kenneth R. Crudup Sr. SW Engineer, Scott County Consulting, Silicon Valley -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Tue, 2014-12-30 at 11:12 -0500, Alan Stern wrote:
> On Tue, 30 Dec 2014, James Bottomley wrote:
>
> > > _Is_ there any way to communicate the maximum transfer size? I'm not
> > > aware of any SCSI command for it. It isn't part of the USB
> > > mass-storage spec.
> >
> > For the device, it's in the Block limits VPD page. However, what the
> > device supports isn't necessarily what the bridge or host bus adapter
> > will support. We need to set the limit to the lowest of what the
> > device, the bridge and the HBA support. We know the device (provided
> > the bridge allows VPD inquiries ... not all do) and host, so we really
> > need to know what the bridge will support. From the error it does look
> > like we're running into a bridge limit.
>
> I see.
>
> > > usb-storage has no clear idea what sort of device lies on the other
> > > side of the USB bridge. It might be an ATA drive, it might be a flash
> > > drive, it might not be a disk at all -- usb-storage does its best not
> > > to know or care.
> >
> > That's fine, but is there any way in USB to query the bridge to get it's
> > transfer characteristics?
>
> No, there isn't. The only query that a USB mass-storage bridge accepts
> is for the maximum LUN value.
>
> > > If you think that usb-storage needs to set a maximum transfer size for
> > > disk drives, it won't be hard to write a patch. But what about all the
> > > other possible transports? Will they each have to implement the same
> > > transfer limit? If so, shouldn't the limit be set up from a more
> > > central location, such as the sd driver?
> >
> > This isn't a transport problem, this is a bridge problem. T10 has
> > always recognised there might be a bridge issue linking two transports,
> > so it did initially come up with a bridge spec (BCC) but it was
> > abandoned a decade ago in favour of transparent bridges (every switch in
> > a FC topology is effectively a transparent bridge) or making them
> > explicit in the standards, like SAS expanders.
> >
> > > Why not have sd always set max_sectors_kb to 32767 if it isn't already
> > > smaller? Would that cause any problems?
> >
> > This wouldn't be sd ... we have lots of requirements for large transfer
> > sizes for efficiency. It has to be the layer that knows there's a
> > bridge, so that would make it usb.
>
> All right. How does this patch look?
OK, I suppose. The transfer limits are a little on the low side, but
for usb-storage (i.e. non-UAS) performance devices, they should be OK.
For TYPE_TAPE, you still have no guarantee that the bridge won't screw
up ... and if the argument is that tapes are always connected to
sensible bridges, why aren't SATA devices?
There's also a spelling mistake below.
> Alan Stern
>
>
>
> Index: usb-3.18/drivers/usb/storage/scsiglue.c
> ===
> --- usb-3.18.orig/drivers/usb/storage/scsiglue.c
> +++ usb-3.18/drivers/usb/storage/scsiglue.c
> @@ -114,26 +114,30 @@ static int slave_alloc (struct scsi_devi
> static int slave_configure(struct scsi_device *sdev)
> {
> struct us_data *us = host_to_us(sdev->host);
> + unsigned int max_sectors;
>
> - /* Many devices have trouble transferring more than 32KB at a time,
> - * while others have trouble with more than 64K. At this time we
> - * are limiting both to 32K (64 sectores).
> + /*
> + * Many devices have trouble transferring more than 32 KB at a time,
> + * while others have trouble with more than 64 KB. At this time we
> + * are limiting both to 32 KB (64 sectores).
sectors
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On 14-12-30 10:34 AM, Alan Stern wrote: On Tue, 30 Dec 2014, Christoph Hellwig wrote: The only limits usb-storage imposes on max_sectors are those needed to work around bugs in the devices' USB bridges. (Okay, there's also something for tape drive devices, but it probably doesn't belong in usb-storage -- it should be handled by the SCSI tape driver.) If the ATA layer needs to set a limit on max_sectors, why doesn't it simply go ahead and do so? Because the ATA layer doesn't control the device, the bridge does. And it seems like it doesn't communicate the maximum transfer size properly. _Is_ there any way to communicate the maximum transfer size? I'm not aware of any SCSI command for it. It isn't part of the USB mass-storage spec. In SCSI, the VPD page 0xb0 (Block limits) has a "Maximum transfer length" field (32 bits long). Its units are logical blocks. Useful if >0 as 0 means "unreported". USB to SATA bridges should comply with SAT. However SAT and SAT-2 don't even mention that VPD page. SAT-3 and SAT-4 mention that page but have "unspecified" next to that field. Basically useless. IMO about the best SATL is in the MPT SAS-2 and SAS-3 HBAs and here is that page for a SAS expander attached SATA disk: # sg_vpd -p bl /dev/sg3 Block limits VPD page (SBC): Write same non-zero (WSNZ): 0 Maximum compare and write length: 0 blocks Optimal transfer length granularity: 0 blocks Maximum transfer length: 0 blocks Optimal transfer length: 0 blocks Maximum prefetch length: 0 blocks Maximum unmap LBA count: 0 Maximum unmap block descriptor count: 0 Optimal unmap granularity: 0 Unmap granularity alignment valid: 0 Unmap granularity alignment: 0 Maximum write same length: 0x0 blocks Maximum atomic transfer length: 0 Atomic alignment: 0 Atomic transfer length granularity: 0 Maximum atomic transfer length with atomic boundary: 0 Maximum atomic boundary size: 0 Not sure why LSI/Avago even bothered even implementing that page ... Doug Gilbert BTW here is the output of that page from a SAS SSD: # sg_vpd -p bl /dev/sg5 Block limits VPD page (SBC): Write same non-zero (WSNZ): 0 Maximum compare and write length: 0 blocks Optimal transfer length granularity: 8 blocks Maximum transfer length: 65535 blocks Optimal transfer length: 0 blocks Maximum prefetch length: 0 blocks Maximum unmap LBA count: 393216 Maximum unmap block descriptor count: 512 Optimal unmap granularity: 8 Unmap granularity alignment valid: 0 Unmap granularity alignment: 0 Maximum write same length: 0x0 blocks Maximum atomic transfer length: 0 Atomic alignment: 0 Atomic transfer length granularity: 0 Maximum atomic transfer length with atomic boundary: 0 Maximum atomic boundary size: 0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Tue, 30 Dec 2014, James Bottomley wrote:
> > _Is_ there any way to communicate the maximum transfer size? I'm not
> > aware of any SCSI command for it. It isn't part of the USB
> > mass-storage spec.
>
> For the device, it's in the Block limits VPD page. However, what the
> device supports isn't necessarily what the bridge or host bus adapter
> will support. We need to set the limit to the lowest of what the
> device, the bridge and the HBA support. We know the device (provided
> the bridge allows VPD inquiries ... not all do) and host, so we really
> need to know what the bridge will support. From the error it does look
> like we're running into a bridge limit.
I see.
> > usb-storage has no clear idea what sort of device lies on the other
> > side of the USB bridge. It might be an ATA drive, it might be a flash
> > drive, it might not be a disk at all -- usb-storage does its best not
> > to know or care.
>
> That's fine, but is there any way in USB to query the bridge to get it's
> transfer characteristics?
No, there isn't. The only query that a USB mass-storage bridge accepts
is for the maximum LUN value.
> > If you think that usb-storage needs to set a maximum transfer size for
> > disk drives, it won't be hard to write a patch. But what about all the
> > other possible transports? Will they each have to implement the same
> > transfer limit? If so, shouldn't the limit be set up from a more
> > central location, such as the sd driver?
>
> This isn't a transport problem, this is a bridge problem. T10 has
> always recognised there might be a bridge issue linking two transports,
> so it did initially come up with a bridge spec (BCC) but it was
> abandoned a decade ago in favour of transparent bridges (every switch in
> a FC topology is effectively a transparent bridge) or making them
> explicit in the standards, like SAS expanders.
>
> > Why not have sd always set max_sectors_kb to 32767 if it isn't already
> > smaller? Would that cause any problems?
>
> This wouldn't be sd ... we have lots of requirements for large transfer
> sizes for efficiency. It has to be the layer that knows there's a
> bridge, so that would make it usb.
All right. How does this patch look?
Alan Stern
Index: usb-3.18/drivers/usb/storage/scsiglue.c
===
--- usb-3.18.orig/drivers/usb/storage/scsiglue.c
+++ usb-3.18/drivers/usb/storage/scsiglue.c
@@ -114,26 +114,30 @@ static int slave_alloc (struct scsi_devi
static int slave_configure(struct scsi_device *sdev)
{
struct us_data *us = host_to_us(sdev->host);
+ unsigned int max_sectors;
- /* Many devices have trouble transferring more than 32KB at a time,
-* while others have trouble with more than 64K. At this time we
-* are limiting both to 32K (64 sectores).
+ /*
+* Many devices have trouble transferring more than 32 KB at a time,
+* while others have trouble with more than 64 KB. At this time we
+* are limiting both to 32 KB (64 sectores).
+* Still other devices have trouble unless the transfer size is as
+* small as possible (one memory page).
+*
+* Tape drives need much higher max_sector limits, so just
+* raise it to the maximum possible (4 GB / 512) and let the
+* queue segment size sort out the real limit.
+* For safety, limit all other devices to 32 MB transfer size.
*/
- if (us->fflags & (US_FL_MAX_SECTORS_64 | US_FL_MAX_SECTORS_MIN)) {
- unsigned int max_sectors = 64;
-
- if (us->fflags & US_FL_MAX_SECTORS_MIN)
- max_sectors = PAGE_CACHE_SIZE >> 9;
- if (queue_max_hw_sectors(sdev->request_queue) > max_sectors)
- blk_queue_max_hw_sectors(sdev->request_queue,
- max_sectors);
- } else if (sdev->type == TYPE_TAPE) {
- /* Tapes need much higher max_sector limits, so just
-* raise it to the maximum possible (4 GB / 512) and
-* let the queue segment size sort out the real limit.
-*/
- blk_queue_max_hw_sectors(sdev->request_queue, 0x7F);
- }
+ if (us->fflags & US_FL_MAX_SECTORS_MIN)
+ max_sectors = PAGE_CACHE_SIZE >> 9;
+ else if (us->fflags & US_FL_MAX_SECTORS_64)
+ max_sectors = 64;
+ else if (sdev->type == TYPE_TAPE)
+ max_sectors = 0x7F;
+ else
+ max_sectors = 65535;
+ if (queue_max_hw_sectors(sdev->request_queue) > max_sectors)
+ blk_queue_max_hw_sectors(sdev->request_queue, max_sectors);
/* Some USB host controllers can't do DMA; they have to use PIO.
* They indicate this by setting their dma_mask to NULL. For
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a messa
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Tue, 2014-12-30 at 10:34 -0500, Alan Stern wrote: > On Tue, 30 Dec 2014, Christoph Hellwig wrote: > > > > The only limits usb-storage imposes on max_sectors are those needed to > > > work around bugs in the devices' USB bridges. (Okay, there's also > > > something for tape drive devices, but it probably doesn't belong in > > > usb-storage -- it should be handled by the SCSI tape driver.) > > > > > > If the ATA layer needs to set a limit on max_sectors, why doesn't it > > > simply go ahead and do so? > > > > Because the ATA layer doesn't control the device, the bridge does. > > And it seems like it doesn't communicate the maximum transfer size > > properly. > > _Is_ there any way to communicate the maximum transfer size? I'm not > aware of any SCSI command for it. It isn't part of the USB > mass-storage spec. For the device, it's in the Block limits VPD page. However, what the device supports isn't necessarily what the bridge or host bus adapter will support. We need to set the limit to the lowest of what the device, the bridge and the HBA support. We know the device (provided the bridge allows VPD inquiries ... not all do) and host, so we really need to know what the bridge will support. From the error it does look like we're running into a bridge limit. > usb-storage has no clear idea what sort of device lies on the other > side of the USB bridge. It might be an ATA drive, it might be a flash > drive, it might not be a disk at all -- usb-storage does its best not > to know or care. That's fine, but is there any way in USB to query the bridge to get it's transfer characteristics? > If you think that usb-storage needs to set a maximum transfer size for > disk drives, it won't be hard to write a patch. But what about all the > other possible transports? Will they each have to implement the same > transfer limit? If so, shouldn't the limit be set up from a more > central location, such as the sd driver? This isn't a transport problem, this is a bridge problem. T10 has always recognised there might be a bridge issue linking two transports, so it did initially come up with a bridge spec (BCC) but it was abandoned a decade ago in favour of transparent bridges (every switch in a FC topology is effectively a transparent bridge) or making them explicit in the standards, like SAS expanders. > Why not have sd always set max_sectors_kb to 32767 if it isn't already > smaller? Would that cause any problems? This wouldn't be sd ... we have lots of requirements for large transfer sizes for efficiency. It has to be the layer that knows there's a bridge, so that would make it usb. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Tue, 30 Dec 2014, Christoph Hellwig wrote: > > The only limits usb-storage imposes on max_sectors are those needed to > > work around bugs in the devices' USB bridges. (Okay, there's also > > something for tape drive devices, but it probably doesn't belong in > > usb-storage -- it should be handled by the SCSI tape driver.) > > > > If the ATA layer needs to set a limit on max_sectors, why doesn't it > > simply go ahead and do so? > > Because the ATA layer doesn't control the device, the bridge does. > And it seems like it doesn't communicate the maximum transfer size > properly. _Is_ there any way to communicate the maximum transfer size? I'm not aware of any SCSI command for it. It isn't part of the USB mass-storage spec. usb-storage has no clear idea what sort of device lies on the other side of the USB bridge. It might be an ATA drive, it might be a flash drive, it might not be a disk at all -- usb-storage does its best not to know or care. If you think that usb-storage needs to set a maximum transfer size for disk drives, it won't be hard to write a patch. But what about all the other possible transports? Will they each have to implement the same transfer limit? If so, shouldn't the limit be set up from a more central location, such as the sd driver? Why not have sd always set max_sectors_kb to 32767 if it isn't already smaller? Would that cause any problems? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Sun, Dec 28, 2014 at 10:10:01PM -0500, Alan Stern wrote: > (Is this a USB device? Presumably you wouldn't have CC'ed the > linux-usb and usb-storage mailing lists if it wasn't...) It's a usb attached device. From the inquity information and the product name it looks like a SATA device attached via a usb bridge. > The only limits usb-storage imposes on max_sectors are those needed to > work around bugs in the devices' USB bridges. (Okay, there's also > something for tape drive devices, but it probably doesn't belong in > usb-storage -- it should be handled by the SCSI tape driver.) > > If the ATA layer needs to set a limit on max_sectors, why doesn't it > simply go ahead and do so? Because the ATA layer doesn't control the device, the bridge does. And it seems like it doesn't communicate the maximum transfer size properly. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Sat, 27 Dec 2014, Christoph Hellwig wrote: > On Tue, Dec 23, 2014 at 11:48:40PM -0800, Kenneth R. Crudup wrote: > > > Looks like we need to quirk it. Can you try to echo different limits > > > to the /sys/block/sdc/queue/max_hw_sectors_kb file for the device to > > > find the limit for it? > > > > Looks like it's 32767; making it an even 32K sectors starts the EIOs again. > > > > To recap for the various lists: > > > > Vendor: Samsung Model: D3 Station Rev: 0202 > > Type: Direct-AccessANSI SCSI revision: 06 > > > > It's the 4TB model: > > > > /sys/block/sdc/size:7814037160 > > /sys/block/sdc/queue/max_hw_sectors_kb:61440 > > > > Works OK if I echo 32767 > /sys/block/sdc/queue/max_sectors_kb > > Interesting. Basically this is the limit for modern ATA disks: > > drivers/ata/libata-core.c: dev->max_sectors = > ATA_MAX_SECTORS_LBA48; > drivers/ata/libata-core.c: dev->max_sectors = > ATA_MAX_SECTORS_LBA48; > include/linux/ata.h:ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */ > > Seems like the USB bridge isn't properly communicating this limit to the SCSI > layer. Maybe it's a good idea to generally limit USB storage to this size? (Is this a USB device? Presumably you wouldn't have CC'ed the linux-usb and usb-storage mailing lists if it wasn't...) The only limits usb-storage imposes on max_sectors are those needed to work around bugs in the devices' USB bridges. (Okay, there's also something for tape drive devices, but it probably doesn't belong in usb-storage -- it should be handled by the SCSI tape driver.) If the ATA layer needs to set a limit on max_sectors, why doesn't it simply go ahead and do so? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Tue, Dec 23, 2014 at 11:48:40PM -0800, Kenneth R. Crudup wrote: > > Looks like we need to quirk it. Can you try to echo different limits > > to the /sys/block/sdc/queue/max_hw_sectors_kb file for the device to > > find the limit for it? > > Looks like it's 32767; making it an even 32K sectors starts the EIOs again. > > To recap for the various lists: > > Vendor: Samsung Model: D3 Station Rev: 0202 > Type: Direct-AccessANSI SCSI revision: 06 > > It's the 4TB model: > > /sys/block/sdc/size:7814037160 > /sys/block/sdc/queue/max_hw_sectors_kb:61440 > > Works OK if I echo 32767 > /sys/block/sdc/queue/max_sectors_kb Interesting. Basically this is the limit for modern ATA disks: drivers/ata/libata-core.c: dev->max_sectors = ATA_MAX_SECTORS_LBA48; drivers/ata/libata-core.c: dev->max_sectors = ATA_MAX_SECTORS_LBA48; include/linux/ata.h:ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */ Seems like the USB bridge isn't properly communicating this limit to the SCSI layer. Maybe it's a good idea to generally limit USB storage to this size? > Has anyone seen a disk out there that's got a (properly-supported) > "max_hw_sectors_kb" >= 32K? If not, maybe we should clamp that value to > prevent other misreporting(?) disks? Yes, lots of SCSI disks and arrays properly support this. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
> To recap for the various lists: > Vendor: Samsung Model: D3 Station Rev: 0202 > Type: Direct-AccessANSI SCSI revision: 06 > It's the 4TB model: I forgot to add the USB PID/VID information: Bus 002 Device 002: ID 04e8:6125 Samsung Electronics Co., Ltd -Kenny -- Kenneth R. Crudup Sr. SW Engineer, Scott County Consulting, Silicon Valley -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Sun, Dec 21, 2014 at 02:44:20PM -0800, Kenneth R. Crudup wrote: > > I had issues with my Samsung 4GB disk recently in Linus' master tree > > kernels, > > and have bisected it to the above commit. When doing running bonnie++ (etc. > > I get EIOs. > > Is there a "quirks" available for this option so I may special-case this > > particular disk? On Tue, 23 Dec 2014, Christoph Hellwig wrote: > Looks like we need to quirk it. Can you try to echo different limits > to the /sys/block/sdc/queue/max_hw_sectors_kb file for the device to > find the limit for it? Looks like it's 32767; making it an even 32K sectors starts the EIOs again. To recap for the various lists: Vendor: Samsung Model: D3 Station Rev: 0202 Type: Direct-AccessANSI SCSI revision: 06 It's the 4TB model: /sys/block/sdc/size:7814037160 /sys/block/sdc/queue/max_hw_sectors_kb:61440 Works OK if I echo 32767 > /sys/block/sdc/queue/max_sectors_kb Has anyone seen a disk out there that's got a (properly-supported) "max_hw_sectors_kb" >= 32K? If not, maybe we should clamp that value to prevent other misreporting(?) disks? -Kenny -- Kenneth R. Crudup Sr. SW Engineer, Scott County Consulting, Silicon Valley -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")
On Sun, Dec 21, 2014 at 02:44:20PM -0800, Kenneth R. Crudup wrote: > > I had issues with my Samsung 4GB disk recently in Linus' master tree kernels, > and have bisected it to the above commit. When doing running bonnie++ (etc. > I get EIOs. > > Some info is below, let me know if you need anything else. Is there a "quirks" > available for this option so I may special-case this particular disk? Looks like we need to quirk it. Can you try to echo different limits to the /sys/block/sdc/queue/max_hw_sectors_kb file for the device to find the limit for it? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
