Re: [PATCH] scsi disk: Use its own buffer for the vpd request
Bernd == Bernd Schubert bernd.schub...@fastmail.fm writes: [Sorry about the delay. Catching up on a couple of weeks worth of email] Bernd So if anything else than a Seagate drive is connected to an Areca Bernd controller with older firmware it will still fail. It's just blacklisting a specific Seagate drive. However, skip_vpd_pages is also set by USB. I'm still completely in favor of your patch to reduce the VPD buffer size. Please resubmit and feel free to add my Acked-by:. -- Martin K. Petersen Oracle Linux Engineering -- 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: [PATCH] scsi disk: Use its own buffer for the vpd request
On 08/31/2013 09:48 PM, Nix wrote: On 31 Aug 2013, Greg KH said: On Fri, Aug 30, 2013 at 11:01:56AM +0100, Nix wrote: On 1 Aug 2013, Bernd Schubert said: Once I noticed that scsi_get_vpd_page() works fine from other function calls and that it is not 0x89, but already 0x0 that fails fixing it became easy. Nix, any chance you could verify it also works for you? As an aside, this commit does indeed fix the bug I reported, but it doesn't seem to have gone anywhere, not even into -stable. Is it held up somehow? (stable has commit 0ac10bd036f0f3b8ce7ac2390446eab9531c72eb Author: Martin K. Petersen martin.peter...@oracle.com Date: Tue Jul 30 22:58:34 2013 -0400 SCSI: Don't attempt to send extended INQUIRY command if skip_vpd_pages is set which IIRC was eventually found not to be necessary, because this fix works fine instead?) Possibly I'm misremembering the order of month-old events and Martin's fix was eventually considered better... in which case, sorry for the noise. Is that other patch even needed anymore, now that Martin's patch is in the tree? My understanding is that this patch is rather better, since Martin's patch prevents sending of the extended INQUIRY command at all: this one just uses a reduced buffer size, but can still issue the command. (But I may be misunderstanding everything.) Hmm, I wonder if 7562523e84ddc742fe1f9db8bd76b01acca89f6b (linus tree) / 0ac10bd036f0f3b8ce7ac2390446eab9531c72eb (stable-tree) always works . It tests if sdev-skip_vpd_pages is set, but as far as I can see this only gets set for Seagate drives via BLIST_SKIP_VPD_PAGES. So if anything else than a Seagate drive is connected to an Areca controller with older firmware it will still fail. Cheers, Bernd -- 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: [PATCH] scsi disk: Use its own buffer for the vpd request
On 31 Aug 2013, Greg KH said: On Fri, Aug 30, 2013 at 11:01:56AM +0100, Nix wrote: On 1 Aug 2013, Bernd Schubert said: Once I noticed that scsi_get_vpd_page() works fine from other function calls and that it is not 0x89, but already 0x0 that fails fixing it became easy. Nix, any chance you could verify it also works for you? As an aside, this commit does indeed fix the bug I reported, but it doesn't seem to have gone anywhere, not even into -stable. Is it held up somehow? (stable has commit 0ac10bd036f0f3b8ce7ac2390446eab9531c72eb Author: Martin K. Petersen martin.peter...@oracle.com Date: Tue Jul 30 22:58:34 2013 -0400 SCSI: Don't attempt to send extended INQUIRY command if skip_vpd_pages is set which IIRC was eventually found not to be necessary, because this fix works fine instead?) Possibly I'm misremembering the order of month-old events and Martin's fix was eventually considered better... in which case, sorry for the noise. Is that other patch even needed anymore, now that Martin's patch is in the tree? My understanding is that this patch is rather better, since Martin's patch prevents sending of the extended INQUIRY command at all: this one just uses a reduced buffer size, but can still issue the command. (But I may be misunderstanding everything.) -- NULL (void) -- 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: [PATCH] scsi disk: Use its own buffer for the vpd request
On 1 Aug 2013, Bernd Schubert said: Once I noticed that scsi_get_vpd_page() works fine from other function calls and that it is not 0x89, but already 0x0 that fails fixing it became easy. Nix, any chance you could verify it also works for you? As an aside, this commit does indeed fix the bug I reported, but it doesn't seem to have gone anywhere, not even into -stable. Is it held up somehow? (stable has commit 0ac10bd036f0f3b8ce7ac2390446eab9531c72eb Author: Martin K. Petersen martin.peter...@oracle.com Date: Tue Jul 30 22:58:34 2013 -0400 SCSI: Don't attempt to send extended INQUIRY command if skip_vpd_pages is set which IIRC was eventually found not to be necessary, because this fix works fine instead?) Possibly I'm misremembering the order of month-old events and Martin's fix was eventually considered better... in which case, sorry for the noise. -- NULL (void) -- 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: [PATCH] scsi disk: Use its own buffer for the vpd request
On Fri, Aug 30, 2013 at 11:01:56AM +0100, Nix wrote: On 1 Aug 2013, Bernd Schubert said: Once I noticed that scsi_get_vpd_page() works fine from other function calls and that it is not 0x89, but already 0x0 that fails fixing it became easy. Nix, any chance you could verify it also works for you? As an aside, this commit does indeed fix the bug I reported, but it doesn't seem to have gone anywhere, not even into -stable. Is it held up somehow? (stable has commit 0ac10bd036f0f3b8ce7ac2390446eab9531c72eb Author: Martin K. Petersen martin.peter...@oracle.com Date: Tue Jul 30 22:58:34 2013 -0400 SCSI: Don't attempt to send extended INQUIRY command if skip_vpd_pages is set which IIRC was eventually found not to be necessary, because this fix works fine instead?) Possibly I'm misremembering the order of month-old events and Martin's fix was eventually considered better... in which case, sorry for the noise. Is that other patch even needed anymore, now that Martin's patch is in the tree? thanks, greg k-h -- 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: [PATCH] scsi disk: Use its own buffer for the vpd request
Martin,
sorry for my late reply, I entirely lost track of this (customer issues,
vacation, lots of main work, ...).
On 08/02/2013 05:00 AM, Martin K. Petersen wrote:
Bernd == Bernd Schubert bernd.schub...@fastmail.fm writes:
Bernd,
Bernd Once I noticed that scsi_get_vpd_page() works fine from other
Bernd function calls and that it is not 0x89, but already 0x0 that
Bernd fails fixing it became easy.
Bernd Nix, any chance you could verify it also works for you?
Do we get an appropriate error back when we try to issue WRITE SAME
10/16? If so, I'm OK with this fix.
And thanks for looking into this!
Is testing with sg_write_same sufficient?
With F/W V1.49:
(squeeze)fslab2:~# lsscsi | grep sda
[2:0:0:0]diskATA HDS724040KLSA80 KFAO /dev/sda
(squeeze)fslab2:~# strace -f sg_write_same --10 -v --num=0 --lba=0 /dev/sda
ioctl(3, SG_IO, {'S', SG_DXFER_TO_DEV, cmd[10]=[41, 00, 00, 00, 00, 00, 00,
00, 00, 00], mx_sb_len=32, iovec_count=0, dxfer_len=512, timeout=6,
flags=0,
data[512]=[\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0...],
status=02, masked_status=01, sb[18]=[70, 00, 05, 00, 00, 00, 00, 0a, 00, 00,
00, 00, 20, 00, 00, 00, 00, 00], host_status=0, driver_status=0x8, resid=0,
duration=0, info=0x1}) = 0
write(2, Write same: Fixed format, curre..., 114Write same: Fixed format,
current; Sense key: Illegal Request
Additional sense: Invalid command operation code
) = 114
write(2, Write same(10) command not suppo..., 37Write same(10) command not
supported
) = 37
(squeeze)fslab2:~# strace -f sg_write_same --16 -v --num=0 --lba=0 /dev/sda
ioctl(3, SG_IO, {'S', SG_DXFER_TO_DEV, cmd[16]=[93, 00, 00, 00, 00, 00, 00,
00, 00, 00, 00, 00, 00, 00, 00, 00], mx_sb_len=32, iovec_count=0,
dxfer_len=512, timeout=6, flags=0,
data[512]=[\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0...],
status=02, masked_status=01, sb[18]=[70, 00, 05, 00, 00, 00, 00, 0a, 00, 00,
00, 00, 24, 00, 00, 00, 00, 00], host_status=0, driver_status=0x8, resid=0,
duration=0, info=0x1}) = 0
write(2, Write same: Fixed format, curre..., 104Write same: Fixed format,
current; Sense key: Illegal Request
Additional sense: Invalid field in cdb
) = 104
write(2, bad field in Write same(16) cdb,..., 63bad field in Write same(16)
cdb, option probably not supported
) = 63
Now with F/W V1.46
(squeeze)fslab2:~# lsscsi | grep sdk
[10:0:1:2] diskHitachi HDS724040KLSA80 R001 /dev/sdk
(squeeze)fslab2:~# cat /sys/class/scsi_host/host10/host_fw_model
ARC-1260
(squeeze)fslab2:~# strace -f sg_write_same --10 -v --num=0 --lba=0 /dev/sdk
execve(/usr/bin/sg_write_same, [sg_write_same, --10, -v, --num=0,
--lba=0, /dev/sdk], [/* 26 vars */]) = 0
ioctl(3, SG_IO, {'S', SG_DXFER_TO_DEV, cmd[10]=[41, 00, 00, 00, 00, 00, 00,
00, 00, 00], mx_sb_len=32, iovec_count=0, dxfer_len=512, timeout=6,
flags=0,
data[512]=[\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0...],
status=00, masked_status=00, sb[19]=[f0, 00, 05, 00, 00, 00, 00, 0b, 00, 00,
00, 00, 20, 00, 00, 00, 02, 00, 00], host_status=0, driver_status=0x8,
resid=0, duration=0, info=0x1}) = 0
write(2, Write same: Fixed format, curre..., 134Write same: Fixed format,
current; Sense key: Illegal Request
Additional sense: Invalid command operation code
Info fld=0x0 [0]
) = 134
write(2, Write same(10) command not suppo..., 37Write same(10) command not
supported
) = 37
(squeeze)fslab2:~# strace -f sg_write_same --16 -v --num=0 --lba=0 /dev/sdk
execve(/usr/bin/sg_write_same, [sg_write_same, --16, -v, --num=0,
--lba=0, /dev/sdk], [/* 26 vars */]) = 0
ioctl(3, SG_IO, {'S', SG_DXFER_TO_DEV, cmd[16]=[93, 00, 00, 00, 00, 00, 00,
00, 00, 00, 00, 00, 00, 00, 00, 00], mx_sb_len=32, iovec_count=0,
dxfer_len=512, timeout=6, flags=0,
data[512]=[\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0...],
status=00, masked_status=00, sb[19]=[f0, 00, 05, 00, 00, 00, 00, 0b, 00, 00,
00, 00, 20, 00, 00, 00, 02, 00, 00], host_status=0, driver_status=0x8,
resid=0, duration=0, info=0x1}) = 0
write(2, Write same: Fixed format, curre..., 134Write same: Fixed format,
current; Sense key: Illegal Request
Additional sense: Invalid command operation code
Info fld=0x0 [0]
) = 134
write(2, Write same(16) command not suppo..., 37Write same(16) command not
supported
) = 37
Is this sufficient, or do you need something else?
Thanks,
Bernd
--
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: [PATCH] scsi disk: Use its own buffer for the vpd request
On 1 Aug 2013, Bernd Schubert stated: Once I noticed that scsi_get_vpd_page() works fine from other function calls and that it is not 0x89, but already 0x0 that fails fixing it became easy. Nix, any chance you could verify it also works for you? Confirmed, thank you! Somehow older areca firmware versions have issues with scsi_get_vpd_page() and a large buffer. I wonder if they're using math modulo SD_BUF_SIZE-1 by mistake, so they misinterpret this as zero? (Still, doing math modulo 511 seems very odd, even if this firmware *does* only support 512-byte sectors.) -- NULL (void) -- 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: [PATCH] scsi disk: Use its own buffer for the vpd request
On 1 Aug 2013, Bernd Schubert told this: Once I noticed that scsi_get_vpd_page() works fine from other function calls and that it is not 0x89, but already 0x0 that fails fixing it became easy. Nix, any chance you could verify it also works for you? Sorry for the delay: it's hard for me to verify this during the working week. I'll check it tomorrow -- after I've run a backup! :} (why yes, bugs of this nature do frighten me a bit. I know it's superstition, but I'm always wondering whether the SCSI controller will come back again whenever that post-error bus reset happens.) -- NULL (void) -- 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: [PATCH] scsi disk: Use its own buffer for the vpd request
Whoops, the title is wrong, it should have been:
[PATCH] scsi disk: Limit get_vpd_page buf size
On 08/01/2013 04:34 PM, Bernd Schubert wrote:
Once I noticed that scsi_get_vpd_page() works fine from other function
calls and that it is not 0x89, but already 0x0 that fails fixing it became
easy.
Nix, any chance you could verify it also works for you?
From: Bernd Schubert bernd.schub...@itwm.fraunhofer.de
Somehow older areca firmware versions have issues with
scsi_get_vpd_page() and a large buffer.
Even scsi_get_vpd_page(, page=0,) failed in sd_read_write_same(),
while a similar request from sd_read_block_limits() worked fine.
Limiting the buf-size to 64-bytes fixes the issue with F/W V1.46.
Fixes a regression with areca controllers and older firmware versions
introduced by commit: 66c28f97120e8a621afd5aa7a31c4b85c547d33d
Reported-by: Nix n...@esperi.org.uk
Signed-off-by: Bernd Schubert bernd.schub...@itwm.fraunhofer.de
CC: sta...@vger.kernel.org
---
drivers/scsi/sd.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 80f39b8..02e50ae 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2651,13 +2651,16 @@ static void sd_read_write_same(struct scsi_disk *sdkp,
unsigned char *buffer)
struct scsi_device *sdev = sdkp-device;
if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, INQUIRY) 0) {
+ /* too large values might cause issues with arcmsr */
+ int vpd_buf_len = 64;
+
sdev-no_report_opcodes = 1;
/* Disable WRITE SAME if REPORT SUPPORTED OPERATION
* CODES is unsupported and the device has an ATA
* Information VPD page (SAT).
*/
- if (!scsi_get_vpd_page(sdev, 0x89, buffer, SD_BUF_SIZE))
+ if (!scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len))
sdev-no_write_same = 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: [PATCH] scsi disk: Use its own buffer for the vpd request
Bernd == Bernd Schubert bernd.schub...@fastmail.fm writes: Bernd, Bernd Once I noticed that scsi_get_vpd_page() works fine from other Bernd function calls and that it is not 0x89, but already 0x0 that Bernd fails fixing it became easy. Bernd Nix, any chance you could verify it also works for you? Do we get an appropriate error back when we try to issue WRITE SAME 10/16? If so, I'm OK with this fix. And thanks for looking into this! -- Martin K. Petersen Oracle Linux Engineering -- 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
