RE: [PATCH][SCSI] megaraid_sas: addded support for big endian architecture
-Original Message-
From: James Bottomley [mailto:jbottom...@parallels.com]
Sent: Wednesday, September 11, 2013 3:42 AM
To: Saxena, Sumit
Cc: linux-scsi@vger.kernel.org; Desai, Kashyap; aradf...@gmail.com
Subject: Re: [PATCH][SCSI] megaraid_sas: addded support for big endian
architecture
On Fri, 2013-09-06 at 15:50 +0530, sumit.sax...@lsi.com wrote:
This patch will add big endian architecture support to megaraid_sas
driver. The support added is for LSI MegaRAID all generation
controllers- (3Gb/s, 6Gb/s and 12 Gb/s controllers).
This patch will be applied on top of recently submitted patch for High
Avaibility support-
http://marc.info/?l=linux-scsim=137799326426659w=2
We have done basic sanity test @ppc64 arch and @x86_64. Additional
testing/observations are welcome.
Well, no it won't actually; it gives a rejection here:
--- drivers/scsi/megaraid/megaraid_sas_base.c
+++ drivers/scsi/megaraid/megaraid_sas_base.c
@@ -5097,8 +5142,9 @@
for (i = 0; i ioc-sge_count kbuff_arr[i]; i++) {
dma_free_coherent(instance-pdev-dev,
-kern_sge32[i].length,
-kbuff_arr[i], kern_sge32[i].phys_addr);
+ le32_to_cpu(kern_sge32[i].length),
+ kbuff_arr[i],
+ le32_to_cpu(kern_sge32[i].phys_addr));
}
megasas_return_cmd(instance, cmd);
Because you didn't take into account this patch:
Author: Bjørn Mork bj...@mork.no
Date: Wed Nov 21 09:54:48 2012 +0100
[SCSI] megaraid_sas: fix memory leak if SGL has zero length entries
I fixed it up this time, but could you please work against the misc
branch of the scsi git repo to prevent this type of problem in future?
Sorry for inconvenience. In future, I will take care of this.
Thanks,
James
Thanks,
Sumit
--
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] enclosure: remove all possible sysfs entries before add device
On 09/10/13 20:46, James Bottomley wrote: During our test, multipath used, each LUN has 2 paths. when adding second path enclousure did not check if will adding device's symlink existed or no. The description doesn't look helpful. The problem, presumably in a remove/re-add test that the add event gets processed before the remove event, which is why the link is still there? Attach my debug info to here: sd 7:0:27:0: [ses_intf_add]:cdev:8817e81fcba0,intf:a00c9400,sdev:8817e81fc800 sd 7:0:27:0: [ses_intf_add] call ses_match_to_enclosure(edev=8817e812c000,sdev=8817e81fc800), cdev=8817e81fcba0 sd 7:0:27:0: *** inq[6]: 48 sd 7:0:27:0: [sdq] 1172123568 512-byte logical blocks: (600 GB/558 GiB) sd 7:0:27:0: [sdq] Write Protect is off ADD: [enclosure_add_links]: kobj: 8817e812cce8 target: 8817e81fc948, device ADD: [enclosure_add_links]: kobj: 8817e81fc948 target: 8817e812cce8, name: enclosure_device:HDD10 [ses_enclosure_find_by_addr] call enclosure_add_device(edev=8817e812c000,i=4,efd-dev=8817e81fc938),cdev=8817e812ccd0 sd 7:0:27:0: [ses_intf_add] call ses_match_to_enclosure(edev=8817ebd18000,sdev=8817e81fc800), cdev=8817e81fcba0 sd 7:0:27:0: *** inq[6]: 48 sd 7:0:27:0: [sdq] Write cache: disabled, read cache: enabled, supports DPO and FUA [ses_enclosure_find_by_addr] call enclosure_add_device(edev=8817e812c000,i=4,efd-dev=8817e81fc938),cdev=8817e812ccd0 sd 7:0:27:0: Attached scsi generic sg17 type 0 sdq: sdq1 sdq2 scsi 6:0:27:0: SSP: handle(0x001c), sas_addr(0x5000c56bd15e), phy(2), device_name(0x5000c56bd15e) scsi 6:0:27:0: SSP: enclosure_logical_id(0x508002a3a510), slot(10) scsi 6:0:27:0: serial_number(000934E00P0S3SL00P0S) scsi 6:0:27:0: qdepth(254), tagged(1), simple(0), ordered(0), scsi_level(6), cmd_que(1) sd 6:0:27:0: [ses_intf_add]:cdev:8817e8304ba0,intf:a00c9400,sdev:8817e8304800 sd 6:0:27:0: [ses_intf_add] call ses_match_to_enclosure(edev=8817e0c5c000,sdev=8817e8304800), cdev=8817e8304ba0 sd 6:0:27:0: *** inq[6]: 48 sd 6:0:27:0: [sdac] 1172123568 512-byte logical blocks: (600 GB/558 GiB) sd 7:0:27:0: [sdq] Attached SCSI disk RM: [enclosure_remove_links]: kobj: 8817e81fc948 name: [enclosure_device:HDD10] RM: [enclosure_remove_links]: kobj: 8817e812cce8 device sd 6:0:27:0: [sdac] Write Protect is off ADD: [enclosure_add_links]: kobj: 8817e812cce8 target: 8817e8304948, device ADD: [enclosure_add_links]: kobj: 8817e8304948 target: 8817e812cce8, name: enclosure_device:HDD10 [ses_enclosure_find_by_addr] call enclosure_add_device(edev=8817e812c000,i=4,efd-dev=8817e8304938),cdev=8817e812ccd0 sd 6:0:27:0: [ses_intf_add] call ses_match_to_enclosure(edev=8817e4094000,sdev=8817e8304800), cdev=8817e8304ba0 sd 6:0:27:0: *** inq[6]: 48 RM: [enclosure_remove_links]: kobj: 8817e9a80948 name: [enclosure_device:HDD10] RM: [enclosure_remove_links]: kobj: 8817e4094ce8 device ADD: [enclosure_add_links]: kobj: 8817e4094ce8 target: 8817e8304948, device ADD: [enclosure_add_links]: kobj: 8817e8304948 target: 8817e4094ce8, name: enclosure_device:HDD10 [ cut here ] WARNING: at fs/sysfs/dir.c:455 sysfs_add_one+0xbc/0xe0() Hardware name: SUN FIRE X4370 M2 SERVER sysfs: cannot create duplicate filename '/devices/pci:00/:00:03.0/:0d:00.0/host6/port-6:1/expander-6:1/port-6:1:14/end_device-6:1:14/target6:0:27/6:0:27:0/enclosure_device:HDD10' Modules linked in: oracleacfs(P)(U) oracleadvm(P)(U) oracleoks(P)(U) mptctl mptbase autofs4 hidp bluetooth rfkill lockd sunrpc bonding be2iscsi iscsi_boot_sysfs ib_iser rdma_cm ib_cm iw_cm ib_sa ib_mad ib_core ib_addr iscsi_tcp bnx2i cnic uio ipv6 cxgb3i libcxgbi cxgb3 mdio libiscsi_tcp libiscsi scsi_transport_iscsi dm_round_robin dm_multipath video sbs sbshc acpi_pad acpi_memhotplug acpi_ipmi parport_pc lp parport ipmi_si ipmi_devintf ipmi_msghandler sg ses enclosure ixgbe e1000e hwmon igb snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd soundcore snd_page_alloc iTCO_wdt pcspkr i2c_i801 ioatdma ghes iTCO_vendor_support hed dca i2c_core i7core_edac edac_core dm_snapshot dm_zero dm_mirror dm_region_hash dm_log dm_mod usb_storage shpchp mpt2sas scsi_transport_sas raid_class ahci libahci sd_mod crc_t10dif raid1 ext3 jbd mbcache Pid: 23302, comm: kworker/u:2 Tainted: P 2.6.39-400.124.1.el5uek.bug17342873V2 #1 Call Trace: [811daf8c] ? sysfs_add_one+0xbc/0xe0 [8106f030] warn_slowpath_common+0x90/0xc0 [8106f15e] warn_slowpath_fmt+0x6e/0x70 [81258bd4] ? strlcat+0x54/0x70 [811daf8c] sysfs_add_one+0xbc/0xe0 [811dbec8] sysfs_do_create_link+0x148/0x1d0 [811dbf83] sysfs_create_link+0x13/0x20 [a00de307] enclosure_add_links+0xe7/0x110 [enclosure] [8125325d] ?
Re: [PATCH 2/3] scsi: improved eh timeout handler
Hi, Hannes: On 09/02/2013 07:58 PM, Hannes Reinecke wrote: If abort succeeds the command is either retried or terminated, depending on the number of allowed retries. However, 'eh_eflags' records the abort, so if the retry would fail again the command is pushed onto the error handler without trying to abort it (again); it'll be cleared up from SCSI EH. I'm still thinking about the aborting 'scsi_eh_abort_cmds()' in SCSI EH - does it make sense to abort in SCSI EH since we've tried to abort via your scsi_abort_command()? Though the aborting in SCSI EH will handle commands which havn't been aborted in scsi_abort_command since EH has been engaged. Thanks, Ren -- 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: READ_CAPACITY_16 vs. READ_CAPACITY_10
On Tue, 10 Sep 2013, Oliver Neukum wrote: On Tue, 2013-09-10 at 13:25 -0400, Alan Stern wrote: On Tue, 10 Sep 2013, Oliver Neukum wrote: Hi Hannes, you objected to this patch saying there's a possibilty that HS devices may also need this feature, which would require a quirk. Does this mean that the patch is acceptable only with an additional predefined quirk, or do you insist that all devices be handled with quirks? Indeed, we already know of one or two high-speed devices that suffer from this bug: http://marc.info/?l=linux-usbm=133586313307042w=2 This may influence your decision. I'm not certain whether it is important enough to merit a new quirk flag, but people experiencing the problem may have some strong opinions. What is the alternative? There are three possibilities: nothing, your proposed patch, and a new quirk flag. The flag is safest, but also the hardest to maintain. I think we can be sure that no drive enclosure will crash with READ_CAPACITY_16. I wouldn't count on it, but I don't know of any examples. I am not sure about card readers. Or flash drives. Does anybody know what Windows does? Not me. It probably varies with different versions of Windows. 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: [PATCH 1/3] scsi: Fix erratic device offline during EH
On 9/2/2013 6:58 AM, Hannes Reinecke wrote:
+static int scsi_eh_action(struct scsi_cmnd *scmd, int rtn) +{ + static
unsigned char tur_command[6] = {TEST_UNIT_READY, 0, 0, 0, 0, 0}; + + if
(scmd-request-cmd_type != REQ_TYPE_BLOCK_PC) { +struct
scsi_driver
*sdrv = scsi_cmd_to_driver(scmd); + if (sdrv-eh_action) +
rtn =
sdrv-eh_action(scmd, tur_command, 6, rtn); + } + return rtn; +} +
Is there are reason for using TUR here instead of STD inquiry? STD
inquiry has
the advantage that it can act like a ping but doesn't return unit
attentions.
Per my previous comments, trapping unit attentions in the error handler
has
caused UA's like luns changed, or power loss to get lost without being
processed. For tape devices loosing UA's like this often means that the higher
level driver won't be notified that the tape is rewound, resulting in serious
issues.
--
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
[PATCH v2] hpsa: remove unneeded loop
From: Tomas Henzl the...@redhat.com
Originally this was first patch in a series, but while the other patches
were taken into 'for-next', this one was forgotten.
The code below is adapted for the 'for=next' branch.
The cmd_pool_bits is protected everywhere with a spinlock,
we don't need the test_and_set_bit, set_bit is enough and the loop
can be removed too.
Signed-off-by: Tomas Henzl the...@redhat.com
---
drivers/scsi/hpsa.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 31489b5..6069c45 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -2736,15 +2736,12 @@ static struct CommandList *cmd_alloc(struct ctlr_info
*h)
unsigned long flags;
spin_lock_irqsave(h-lock, flags);
- do {
- i = find_first_zero_bit(h-cmd_pool_bits, h-nr_cmds);
- if (i == h-nr_cmds) {
- spin_unlock_irqrestore(h-lock, flags);
- return NULL;
- }
- } while (test_and_set_bit
-(i (BITS_PER_LONG - 1),
- h-cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
+ i = find_first_zero_bit(h-cmd_pool_bits, h-nr_cmds);
+ if (i == h-nr_cmds) {
+ spin_unlock_irqrestore(h-lock, flags);
+ return NULL;
+ }
+ set_bit(i (BITS_PER_LONG - 1), h-cmd_pool_bits + (i /
BITS_PER_LONG));
spin_unlock_irqrestore(h-lock, flags);
c = h-cmd_pool + i;
--
1.8.3.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: esas2r: fix potential format string flaw
On 09/11/2013 12:38 AM, Kees Cook wrote: This makes sure format strings cannot leak into the printk call via the constructed buffer. Signed-off-by: Kees Cook keesc...@chromium.org --- Acked-by: Bradley Grove bgr...@attotech.com Brad -- 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: READ_CAPACITY_16 vs. READ_CAPACITY_10
On Wed, 2013-09-11 at 10:14 -0400, Alan Stern wrote: There are three possibilities: nothing, your proposed patch, and a new Nothing is feasible only if Windows uses READ_CAPACITY_10. quirk flag. The flag is safest, but also the hardest to maintain. Again the same answer. I think we can be sure that no drive enclosure will crash with READ_CAPACITY_16. I wouldn't count on it, but I don't know of any examples. I am not sure about card readers. Or flash drives. Does anybody know what Windows does? Not me. It probably varies with different versions of Windows. I'll try to get a Windows machine for a trace. Can you suggest a tracer for Win7? Regards Oliver -- 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: READ_CAPACITY_16 vs. READ_CAPACITY_10
On Wed, 11 Sep 2013, Oliver Neukum wrote: On Wed, 2013-09-11 at 10:14 -0400, Alan Stern wrote: There are three possibilities: nothing, your proposed patch, and a new Nothing is feasible only if Windows uses READ_CAPACITY_10. It seems clear that your patch isn't feasible either, as it doesn't help high-speed devices. quirk flag. The flag is safest, but also the hardest to maintain. Again the same answer. I think we can be sure that no drive enclosure will crash with READ_CAPACITY_16. I wouldn't count on it, but I don't know of any examples. I am not sure about card readers. Or flash drives. Does anybody know what Windows does? Not me. It probably varies with different versions of Windows. I'll try to get a Windows machine for a trace. Can you suggest a tracer for Win7? I don't know of any, offhand. Maybe Google can help. Alternatively, you could install Windows 7 in a virtual machine under Linux and use usbmon. 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: READ_CAPACITY_16 vs. READ_CAPACITY_10
On Wed, 2013-09-11 at 11:42 -0400, Alan Stern wrote: On Wed, 11 Sep 2013, Oliver Neukum wrote: I'll try to get a Windows machine for a trace. Can you suggest a tracer for Win7? I don't know of any, offhand. Maybe Google can help. Alternatively, you could install Windows 7 in a virtual machine under Linux and use usbmon. USBPcap can capture Windows USB transactions which Wireshark 1.10 or later can interpret: http://desowin.org/usbpcap/ Note, if you're used to WinPCap or usbmon capture integration with Wireshark this will be an adjustment. You have to run USBPcap from the command line to do the capture to a file, quit the capture, then open the file in Wireshark. I think depending on the options you pass to USBPcap you could end up with .pcap frames that are larger than Wireshark can handle (i.e., for a READ(x) transfer of 128 blocks). Regards, Steven J. Magnani I claim this network for MARS! www.digidescorp.com Earthling, return my space modulator! #include standard.disclaimer -- 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 2/9] fnic: host reset returns nonzero value(errno) on success
On Mon, 2013-09-09 at 13:31 -0700, Hiral Patel wrote:
From: Narsimhulu Musini nmus...@cisco.com
Fixed appropriate error codes that returns -1 on failure, and 0 on success
This is about as undescriptive as they come. What you mean is that
fnic_reset() is used directly by the fc transport callback
issue_fc_host_lip which requires a negative error number on failure.
I really don't think you want to be returning -1, though; that's -EPERM.
Signed-off-by: Narsimhulu Musini nmus...@cisco.com
Signed-off-by: Hiral Patel hiral...@cisco.com
---
drivers/scsi/fnic/fnic_scsi.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index a97e6e5..ef3c463 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -2208,7 +2208,7 @@ int fnic_reset(struct Scsi_Host *shost)
{
struct fc_lport *lp;
struct fnic *fnic;
- int ret = SUCCESS;
+ int ret = 0;
lp = shost_priv(shost);
fnic = lport_priv(lp);
@@ -2221,11 +2221,11 @@ int fnic_reset(struct Scsi_Host *shost)
* reset remote port sessions, and if link is up, begin flogi
*/
if (lp-tt.lport_reset(lp))
- ret = FAILED;
+ ret = -1;
tt.lport_reset can't actually fail anyway, but if it did, it would
return a negative error no, so why not just do
ret = lp-tt.lport_reset()
instead of the if?
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
[PATCH RESUBMIT 0/1] lpfc: remove unnecessary read of PCI_CAP_ID_EXP
This is a resubmission of a patch from ~1 year ago, which was a resubmission of a patch series from ~1 year before that. There was originally some confusion regarding this patch, but it has been acked twice now by the driver maintainer. http://www.spinics.net/lists/linux-scsi/msg53169.html http://www.spinics.net/lists/linux-scsi/msg61195.html I would appreciate them being included, lest I have to resubmit them again next year :) Thanks, Jon -- 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
[PATCH RESUBMIT 1/1] lpfc: remove unnecessary read of PCI_CAP_ID_EXP
The PCIE capability offset is saved during PCI bus walking. It will remove an unnecessary search in the PCI configuration space if this value is referenced instead of reacquiring it. Also, pci_is_pcie is a better way of determining if the device is PCIE or not (as it uses the same saved PCIE capability offset). Signed-off-by: Jon Mason jdma...@kudzu.us Acked-by: James Smart james.sm...@emulex.com --- drivers/scsi/lpfc/lpfc_init.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c index 501147c..4d0f570 100644 --- a/drivers/scsi/lpfc/lpfc_init.c +++ b/drivers/scsi/lpfc/lpfc_init.c @@ -4545,7 +4545,7 @@ lpfc_enable_pci_dev(struct lpfc_hba *phba) pci_save_state(pdev); /* PCIe EEH recovery on powerpc platforms needs fundamental reset */ - if (pci_find_capability(pdev, PCI_CAP_ID_EXP)) + if (pci_is_pcie(pdev)) pdev-needs_freset = 1; return 0; -- 1.7.10.4 -- 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
[PATCH RESEND] scsi_dh_rdac: Add new IBM 1813 product id to rdac devlist
Add new IBM product id to the RDAC devlist
Signed-off-by: Sean Stewart sean.stew...@netapp.com
---
diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c
b/drivers/scsi/device_handler/scsi_dh_rdac.c
index 69c915a..4b9cf93 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -786,6 +786,7 @@ static const struct scsi_dh_devlist rdac_dev_list[] = {
{IBM, 1742},
{IBM, 1745},
{IBM, 1746},
+ {IBM, 1813},
{IBM, 1814},
{IBM, 1815},
{IBM, 1818},
--
--
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 8/9] fnic: Fnic Statistics Collection
On Mon, 2013-09-09 at 13:31 -0700, Hiral Patel wrote:
This feature gathers active and cumulative per fnic stats for io,
abort, terminate, reset, vlan discovery path and it also includes
various important stats for debugging issues. It also provided
debugfs and ioctl interface for user to retrieve these stats.
It also provides functionality to reset cumulative stats through
user interface.
Checkpatch has quite a bit to say about this, but in particular it
doesn't like the instances of
+ if (fnic_stats_debugfs_root) {
+ debugfs_remove(fnic_stats_debugfs_root);
Because debugfs_remove() can be called on a NULL pointer
and
+ ret = strict_strtoul(buf, 10, val);
+ if (ret 0)
+ return ret;
Because this should be using kstrtoul(); we're trying to obsolete
strict_strtoul.
Please fix.
Thanks,
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: READ_CAPACITY_16 vs. READ_CAPACITY_10
On 09/11/2013 04:14 PM, Alan Stern wrote: On Tue, 10 Sep 2013, Oliver Neukum wrote: On Tue, 2013-09-10 at 13:25 -0400, Alan Stern wrote: On Tue, 10 Sep 2013, Oliver Neukum wrote: Hi Hannes, you objected to this patch saying there's a possibilty that HS devices may also need this feature, which would require a quirk. Does this mean that the patch is acceptable only with an additional predefined quirk, or do you insist that all devices be handled with quirks? Indeed, we already know of one or two high-speed devices that suffer from this bug: http://marc.info/?l=linux-usbm=133586313307042w=2 This may influence your decision. I'm not certain whether it is important enough to merit a new quirk flag, but people experiencing the problem may have some strong opinions. What is the alternative? There are three possibilities: nothing, your proposed patch, and a new quirk flag. The flag is safest, but also the hardest to maintain. I think we can be sure that no drive enclosure will crash with READ_CAPACITY_16. I wouldn't count on it, but I don't know of any examples. Me neither. The whole issue just smells of some firmware coders messing up their stuff. So I would think that other firmwares might not have this problem. (But as HS enclosures aren't that common chances are we've hit the same firmware by chance on every test machine we've had.) I think it would warrant a quirk approach. Yes, it might be slightly more coding, but at the same time it's more selective on where it should trigger. Also looking at scsiglue.c it would make things quite tricky if you'd want to _disable_ this feature for HS devices; other firmware from other vendors might not exhibit this issue after all. And scsiglue.c already has tons of quirks set, adding one more doesn't do any harm. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- 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: READ_CAPACITY_16 vs. READ_CAPACITY_10
On 09/10/2013 03:56 PM, Oliver Neukum wrote: Hi Hannes, you objected to this patch saying there's a possibilty that HS devices may also need this feature, which would require a quirk. Does this mean that the patch is acceptable only with an additional predefined quirk, or do you insist that all devices be handled with quirks? Regards Oliver +++ b/drivers/usb/storage/scsiglue.c @@ -211,8 +211,11 @@ static int slave_configure(struct scsi_device0*sdev) /* * Many devices do not respond properly to READ_CAPACITY_16. * Tell the SCSI layer to try READ_CAPACITY_10 first. + * However some USB 3.0 drive enclosures return capacity + * modulo 2TB */ - sdev-try_rc_10_first = 1; + if (us-pusb_dev-speed USB_SPEED_SUPER) + sdev-try_rc_10_first = 1; /* assume SPC3 or latter devices support sense size 18 */ if (sdev-scsi_level SCSI_SPC_2) Predefined quirks is okay. My main objection here is that the original issue most likely is a buggy firmware, so there is a _very_ good chance that it'll be resolved in firmware in the near future. At the same time, other firmwares might continue to not support READ_CAPACITY_16 while enabling HS. So both issues really should be kept separate, which'll warrant a new flag. Which of course could be set to on per default on HS devices. As long as we can switch is off again ... Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- 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
