Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Christoph Hellwig]

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?

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Kenneth R. Crudup]

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-Access                    ANSI  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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Kenneth R. Crudup]

> To recap for the various lists:
>   Vendor: Samsung  Model: D3 Station       Rev: 0202
>   Type:   Direct-Access                    ANSI  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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Christoph Hellwig]

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-Access                    ANSI  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.

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Alan Stern]

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-Access                    ANSI  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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Christoph Hellwig]

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.

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Alan Stern]

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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[James Bottomley]

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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Alan Stern]

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, 0x7FFFFF);
-	}
+	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 = 0x7FFFFF;
+	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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[James Bottomley]

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-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Alan Stern]

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.

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[James Bottomley]

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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Douglas Gilbert]

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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Kenneth R. Crudup]

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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Christoph Hellwig]

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?

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Alan Stern]

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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Kenneth R. Crudup]

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 <stern@rowland.harvard.edu> 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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Kenneth R. Crudup]

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 2135360000
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
<snip>
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 0000:00:14.0: xHCI xhci_drop_endpoint called with disabled ep ffff880448c9bf00
Jan 19 01:38:47 tosh-p75a kernel: [  300.115150] xhci_hcd 0000:00:14.0: xHCI xhci_drop_endpoint called with disabled ep ffff880448c9bf48
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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Alan Stern]

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 2135360000
> 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
> <snip>
> 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 0000:00:14.0: xHCI xhci_drop_endpoint called with disabled ep ffff880448c9bf00
> Jan 19 01:38:47 tosh-p75a kernel: [  300.115150] xhci_hcd 0000:00:14.0: xHCI xhci_drop_endpoint called with disabled ep ffff880448c9bf48
> 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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Kenneth R. Crudup]

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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Kenneth R. Crudup]

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

Re: Issues with commit 34b48db6 ("block: remove artifical max_hw_sectors cap")

[Christoph Hellwig]

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.