[PATCH] scsi: aha152x: Remove residual AHA152X_DEBUG references
From: "Ewan D. Milne"
Commit f75ae8ed.. ("aha152x: debug output update and whitespace cleanup")
removed all AHA152X_DEBUG code from aha152x.c, so the build option no longer
does anything useful.
Signed-off-by: Ewan D. Milne
---
Documentation/scsi/aha152x.txt | 3 ---
drivers/scsi/aha152x.h | 22 --
2 files changed, 25 deletions(-)
diff --git a/Documentation/scsi/aha152x.txt b/Documentation/scsi/aha152x.txt
index 9484873..248dde1 100644
--- a/Documentation/scsi/aha152x.txt
+++ b/Documentation/scsi/aha152x.txt
@@ -40,9 +40,6 @@ COMPILE TIME CONFIGURATION (go into AHA152X in
drivers/scsi/Makefile):
-DSETUP1="{ IOPORT, IRQ, SCSI_ID, RECONNECT, PARITY, SYNCHRONOUS, DELAY,
EXT_TRANS }"
override for the second controller
--DAHA152X_DEBUG
- enable debugging output
-
-DAHA152X_STAT
enable some statistics
diff --git a/drivers/scsi/aha152x.h b/drivers/scsi/aha152x.h
index ac4bfa438b..1addbe6 100644
--- a/drivers/scsi/aha152x.h
+++ b/drivers/scsi/aha152x.h
@@ -294,25 +294,6 @@ typedef union {
#define SETRATE(RATE) SETPORT(SCSIRATE,(RATE) & 0x7f)
-#if defined(AHA152X_DEBUG)
-enum {
- debug_procinfo = 0x0001,
- debug_queue = 0x0002,
- debug_locking = 0x0004,
- debug_intr = 0x0008,
- debug_selection = 0x0010,
- debug_msgo = 0x0020,
- debug_msgi = 0x0040,
- debug_status= 0x0080,
- debug_cmd = 0x0100,
- debug_datai = 0x0200,
- debug_datao = 0x0400,
- debug_eh = 0x0800,
- debug_done = 0x1000,
- debug_phases= 0x2000,
-};
-#endif
-
/* for the pcmcia stub */
struct aha152x_setup {
int io_port;
@@ -324,9 +305,6 @@ struct aha152x_setup {
int delay;
int ext_trans;
int tc1550;
-#if defined(AHA152X_DEBUG)
- int debug;
-#endif
char *conf;
};
--
2.1.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
[PATCH] scsi_debug: fix failure to probe with scsi_level=1 or 2 due to NULL devip
From: "Ewan D. Milne"
commit cbf67842c3d9 ("scsi_debug: support scsi-mq, queues and locks")
added a test for devip == NULL in schedule_resp which returned
SCSI_MLQUEUE_HOST_BUSY. Unfortunately, if scsi_level 1 or 2 is specified,
devip will be NULL for the INQUIRY command for the next LUN above the
configured value and it will be retried indefinitely with an error message.
Fix this by returning the command in the same context if no devip exists.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_debug.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 30268bb..25f5cee1 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3944,8 +3944,11 @@ schedule_resp(struct scsi_cmnd *cmnd, struct
sdebug_dev_info *devip,
struct sdebug_queued_cmd *sqcp = NULL;
struct scsi_device *sdp = cmnd->device;
- if (NULL == cmnd || NULL == devip) {
- pr_warn("%s: called with NULL cmnd or devip pointer\n",
+ /* devip will be NULL when probing nonexistent LUNs w/o REPORT LUNS */
+ if (NULL == devip)
+ goto respond_in_thread;
+ if (NULL == cmnd) {
+ pr_warn("%s: called with NULL cmnd pointer\n",
__func__);
/* no particularly good error to report back */
return SCSI_MLQUEUE_HOST_BUSY;
--
2.1.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: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE
On Sat, 2016-10-08 at 19:35 +0200, Hannes Reinecke wrote: > You might actually be hitting a limitation in the exchange manager code. > The libfc exchange manager tries to be really clever and will assign a > per-cpu exchange manager (probably to increase locality). However, we > only have a limited number of exchanges, so on large systems we might > actually run into a exchange starvation problem, where we have in theory > enough free exchanges, but none for the submitting cpu. > > (Personally, the exchange manager code is in urgent need of reworking. > It should be replaced by the sbitmap code from Omar). > > Do check how many free exchanges are actually present for the stalling > CPU; it might be that you run into a starvation issue. We are still looking into this but one thing that looks bad is that the exchange manager code rounds up the number of CPUs to the next power of 2 before dividing up the exchange id space (and uses the lsbs of the xid to extract the CPU when looking up an xid). We have a machine with 288 CPUs, this code is just begging for a rewrite as it looks to be wasting most of the limited xid space on ixgbe FCoE. Looks like we get 512 offloaded xids on this adapter and 4096-512 non-offloaded xids. This would give 1 + 7 xids per CPU. However, I'm not sure that even 4096 / 288 = 14 would be enough to prevent stalling. And, of course, potentially most of the CPUs aren't submitting I/O, so the whole idea of per-CPU xid space is questionable. -Ewan -- 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: [Open-FCoE] Issue with fc_exch_alloc failing initiated by fc_queuecommand on NUMA or large configurations with Intel ixgbe running FCOE
On Tue, 2016-10-11 at 10:51 -0400, Ewan D. Milne wrote: > On Sat, 2016-10-08 at 19:35 +0200, Hannes Reinecke wrote: > > You might actually be hitting a limitation in the exchange manager code. > > The libfc exchange manager tries to be really clever and will assign a > > per-cpu exchange manager (probably to increase locality). However, we > > only have a limited number of exchanges, so on large systems we might > > actually run into a exchange starvation problem, where we have in theory > > enough free exchanges, but none for the submitting cpu. > > > > (Personally, the exchange manager code is in urgent need of reworking. > > It should be replaced by the sbitmap code from Omar). > > > > Do check how many free exchanges are actually present for the stalling > > CPU; it might be that you run into a starvation issue. > > We are still looking into this but one thing that looks bad is that > the exchange manager code rounds up the number of CPUs to the next > power of 2 before dividing up the exchange id space (and uses the lsbs > of the xid to extract the CPU when looking up an xid). We have a machine > with 288 CPUs, this code is just begging for a rewrite as it looks to > be wasting most of the limited xid space on ixgbe FCoE. > > Looks like we get 512 offloaded xids on this adapter and 4096-512 > non-offloaded xids. This would give 1 + 7 xids per CPU. However, I'm > not sure that even 4096 / 288 = 14 would be enough to prevent stalling. > > And, of course, potentially most of the CPUs aren't submitting I/O, so > the whole idea of per-CPU xid space is questionable. > fc_exch_alloc() used to try all the available exchange managers in the list for an available exchange id, but this was changed in 2010 so that if the first matched exchange manager couldn't allocate one, it fails and we end up returning host busy. This was due to commit: commit 3e22760d4db6fd89e0be46c3d132390a251da9c6 Author: Vasu Dev Date: Fri Mar 12 16:08:39 2010 -0800 [SCSI] libfc: use offload EM instance again instead jumping to next EM Since use of offloads is more efficient than switching to non-offload EM. However kept logic same to call em_match if it is provided in the list of EMs. Converted fc_exch_alloc to inline being now tiny a function and already not an exported libfc API any more. Signed-off-by: Vasu Dev Signed-off-by: Robert Love Signed-off-by: James Bottomley --- Setting the ddp_min module parameter to fcoe to 128MB prevents the ->match function from permitting the use of the offload exchange manager for the frame, and we no longer see the problem with host busy status, since it uses the larger non-offloaded pool. I couldn't find any history on the motivation for the above commit, was there a significant benefit in some case? There seems to be a big downside. -Ewan -- 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 4/7] megaraid_sas: Send SYNCHRONIZE_CACHE command to firmware
On Mon, 2016-10-17 at 12:08 -0400, Ric Wheeler wrote: > On 10/17/2016 11:55 AM, Christoph Hellwig wrote: > > On Mon, Oct 17, 2016 at 09:01:29AM -0400, Ric Wheeler wrote: > >> This must go in - without this fix, there is no data integrity for any > >> file system. > > megaraid always had odd ideas on cache flushing, and this might be > > a opportunity to write down all the assumptions and document them. > > > >> In effect, this driver by default has been throwing away SYNCHRONIZE_CACHE > >> commands even when acting in JBOD/non-RAID mode. > > That would explain some issues we've seen with megaraid hardware, but > > it seems a bit too shocking to be true. > > > > Looking over the patch I disagree with the module option - we must do > > the right thing by default, which is sending SYNCHRONIZE_CACHE commands > > if the WCE bit is set. If there are controllers where this is harmful > > for RAID mode and we can't fix the firmware in time we'll need to make > > special exceptions for this case in the driver based on the PCI ID > > and knowing what we talk to instead of leaving it to the user. > > I do agree - having users be able to disable this easily is asking for > trouble. > It will be slower, but slower because the cache flush actually is effective. Absolutely. It is a bad idea to allow users to disable correct behavior in order to achieve better performance, you will always get people who will do this and then pay the price eventually when they get corruption. > > > > >> * having T10 & T13 report the existence of a volatile write cache - this is > >> different than WCE set, some devices have a write cache and are > >> battery/flash backed. > > T10 is pretty clear now the WCE should only be set for a non-voltile > > cache. For a while they had odd NV bits to allow flushing a > > non-volatile cache, but in the latest revisions all that is gone. > > If T10 has clarity on this, then the actual fix would be to have the driver > advertise WCE enabled only for the pass through/non-RAID luns (assuming the > drive's write cache was enabled) and then we would leave WCE disabled for the > targets that the firmware handles cache destaging for. > > -- 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 v3 1/6] scsi: fnic: use kernel's '%pM' format option to print MAC
On Sat, 2016-10-22 at 20:32 +0300, Andy Shevchenko wrote:
> Instead of supplying each byte through stack let's use %pM specifier.
>
> Cc: Hiral Patel
> Cc: Suma Ramars
> Acked-by: Tom Tucker
> Signed-off-by: Andy Shevchenko
> ---
> drivers/scsi/fnic/vnic_dev.c | 10 ++
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/fnic/vnic_dev.c b/drivers/scsi/fnic/vnic_dev.c
> index 9795d6f..ba69d61 100644
> --- a/drivers/scsi/fnic/vnic_dev.c
> +++ b/drivers/scsi/fnic/vnic_dev.c
> @@ -499,10 +499,7 @@ void vnic_dev_add_addr(struct vnic_dev *vdev, u8 *addr)
>
> err = vnic_dev_cmd(vdev, CMD_ADDR_ADD, &a0, &a1, wait);
> if (err)
> - printk(KERN_ERR
> - "Can't add addr [%02x:%02x:%02x:%02x:%02x:%02x], %d\n",
> - addr[0], addr[1], addr[2], addr[3], addr[4], addr[5],
> - err);
> + pr_err("Can't add addr [%pM], %d\n", addr, err);
> }
>
> void vnic_dev_del_addr(struct vnic_dev *vdev, u8 *addr)
> @@ -517,10 +514,7 @@ void vnic_dev_del_addr(struct vnic_dev *vdev, u8 *addr)
>
> err = vnic_dev_cmd(vdev, CMD_ADDR_DEL, &a0, &a1, wait);
> if (err)
> - printk(KERN_ERR
> - "Can't del addr [%02x:%02x:%02x:%02x:%02x:%02x], %d\n",
> - addr[0], addr[1], addr[2], addr[3], addr[4], addr[5],
> - err);
> + pr_err("Can't del addr [%pM], %d\n", addr, err);
> }
>
> int vnic_dev_notify_set(struct vnic_dev *vdev, u16 intr)
Reviewed-by: Ewan D. Milne
--
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 v3 2/6] fusion: print lan address via %pMR
On Sat, 2016-10-22 at 20:32 +0300, Andy Shevchenko wrote:
> LAN MAC addresses can be printed directly using %pMR specifier.
>
> Cc: Sathya Prakash
> Cc: Chaitra P B
> Cc: Suganath Prabu Subramani
> Signed-off-by: Andy Shevchenko
> ---
> drivers/message/fusion/mptbase.c | 14 --
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/message/fusion/mptbase.c
> b/drivers/message/fusion/mptbase.c
> index 89c7ed1..f82745c 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -2585,10 +2585,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u32 reason, int
> sleepFlag)
> (void) GetLanConfigPages(ioc);
> a =
> (u8*)&ioc->lan_cnfg_page1.HardwareAddressLow;
> dprintk(ioc, printk(MYIOC_s_DEBUG_FMT
> - "LanAddr = %02X:%02X:%02X"
> - ":%02X:%02X:%02X\n",
> - ioc->name, a[5], a[4],
> - a[3], a[2], a[1], a[0]));
> + "LanAddr = %pMR\n", ioc->name, a));
> }
> break;
>
> @@ -6783,8 +6780,7 @@ static int mpt_iocinfo_proc_show(struct seq_file *m,
> void *v)
> if (ioc->bus_type == FC) {
> if (ioc->pfacts[p].ProtocolFlags &
> MPI_PORTFACTS_PROTOCOL_LAN) {
> u8 *a =
> (u8*)&ioc->lan_cnfg_page1.HardwareAddressLow;
> - seq_printf(m, "LanAddr =
> %02X:%02X:%02X:%02X:%02X:%02X\n",
> - a[5], a[4], a[3], a[2], a[1],
> a[0]);
> + seq_printf(m, "LanAddr = %pMR\n", a);
> }
> seq_printf(m, "WWN = %08X%08X:%08X%08X\n",
> ioc->fc_port_page0[p].WWNN.High,
> @@ -6861,8 +6857,7 @@ mpt_print_ioc_summary(MPT_ADAPTER *ioc, char *buffer,
> int *size, int len, int sh
>
> if (showlan && (ioc->pfacts[0].ProtocolFlags &
> MPI_PORTFACTS_PROTOCOL_LAN)) {
> u8 *a = (u8*)&ioc->lan_cnfg_page1.HardwareAddressLow;
> - y += sprintf(buffer+len+y, ",
> LanAddr=%02X:%02X:%02X:%02X:%02X:%02X",
> - a[5], a[4], a[3], a[2], a[1], a[0]);
> + y += sprintf(buffer+len+y, ", LanAddr=%pMR", a);
> }
>
> y += sprintf(buffer+len+y, ", IRQ=%d", ioc->pci_irq);
> @@ -6896,8 +6891,7 @@ static void seq_mpt_print_ioc_summary(MPT_ADAPTER *ioc,
> struct seq_file *m, int
>
> if (showlan && (ioc->pfacts[0].ProtocolFlags &
> MPI_PORTFACTS_PROTOCOL_LAN)) {
> u8 *a = (u8*)&ioc->lan_cnfg_page1.HardwareAddressLow;
> - seq_printf(m, ", LanAddr=%02X:%02X:%02X:%02X:%02X:%02X",
> - a[5], a[4], a[3], a[2], a[1], a[0]);
> + seq_printf(m, ", LanAddr=%pMR", a);
> }
>
> seq_printf(m, ", IRQ=%d", ioc->pci_irq);
Reviewed-by: Ewan D. Milne
--
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 v3 3/6] scsi: qla4xxx: print MAC and SID via %p[mM][R]
On Sat, 2016-10-22 at 20:32 +0300, Andy Shevchenko wrote:
> From: Oleksandr Khoshaba
>
> In the kernel we have nice specifier to print MAC by given pointer to the
> address in a binary form.
>
> Signed-off-by: Oleksandr Khoshaba
> Acked-by: Vikas Chaudhary
> Cc: qlogic-storage-upstr...@qlogic.com
> Signed-off-by: Andy Shevchenko
> ---
> drivers/scsi/qla4xxx/ql4_mbx.c | 5 +
> drivers/scsi/qla4xxx/ql4_nx.c | 8 ++--
> drivers/scsi/qla4xxx/ql4_os.c | 15 ---
> 3 files changed, 7 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c b/drivers/scsi/qla4xxx/ql4_mbx.c
> index c291fdf..1da04f3 100644
> --- a/drivers/scsi/qla4xxx/ql4_mbx.c
> +++ b/drivers/scsi/qla4xxx/ql4_mbx.c
> @@ -2032,10 +2032,7 @@ int qla4xxx_set_param_ddbentry(struct scsi_qla_host
> *ha,
> ptid = (uint16_t *)&fw_ddb_entry->isid[1];
> *ptid = cpu_to_le16((uint16_t)ddb_entry->sess->target_id);
>
> - DEBUG2(ql4_printk(KERN_INFO, ha, "ISID [%02x%02x%02x%02x%02x%02x]\n",
> - fw_ddb_entry->isid[5], fw_ddb_entry->isid[4],
> - fw_ddb_entry->isid[3], fw_ddb_entry->isid[2],
> - fw_ddb_entry->isid[1], fw_ddb_entry->isid[0]));
> + DEBUG2(ql4_printk(KERN_INFO, ha, "ISID [%pmR]\n", fw_ddb_entry->isid));
>
> iscsi_opts = le16_to_cpu(fw_ddb_entry->iscsi_options);
> memset(fw_ddb_entry->iscsi_alias, 0, sizeof(fw_ddb_entry->iscsi_alias));
> diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c
> index 06ddd13..bccd8b6 100644
> --- a/drivers/scsi/qla4xxx/ql4_nx.c
> +++ b/drivers/scsi/qla4xxx/ql4_nx.c
> @@ -4094,12 +4094,8 @@ int qla4_8xxx_get_sys_info(struct scsi_qla_host *ha)
> ha->phy_port_num = sys_info->port_num;
> ha->iscsi_pci_func_cnt = sys_info->iscsi_pci_func_cnt;
>
> - DEBUG2(printk("scsi%ld: %s: "
> - "mac %02x:%02x:%02x:%02x:%02x:%02x "
> - "serial %s\n", ha->host_no, __func__,
> - ha->my_mac[0], ha->my_mac[1], ha->my_mac[2],
> - ha->my_mac[3], ha->my_mac[4], ha->my_mac[5],
> - ha->serial_number));
> + DEBUG2(printk("scsi%ld: %s: mac %pM serial %s\n",
> + ha->host_no, __func__, ha->my_mac, ha->serial_number));
>
> status = QLA_SUCCESS;
>
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index 01c3610..9fbb33f 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -6304,13 +6304,9 @@ static int qla4xxx_compare_tuple_ddb(struct
> scsi_qla_host *ha,
>* ISID would not match firmware generated ISID.
>*/
> if (is_isid_compare) {
> - DEBUG2(ql4_printk(KERN_INFO, ha, "%s: old ISID [%02x%02x%02x"
> - "%02x%02x%02x] New ISID [%02x%02x%02x%02x%02x%02x]\n",
> - __func__, old_tddb->isid[5], old_tddb->isid[4],
> - old_tddb->isid[3], old_tddb->isid[2], old_tddb->isid[1],
> - old_tddb->isid[0], new_tddb->isid[5], new_tddb->isid[4],
> - new_tddb->isid[3], new_tddb->isid[2], new_tddb->isid[1],
> - new_tddb->isid[0]));
> + DEBUG2(ql4_printk(KERN_INFO, ha,
> + "%s: old ISID [%pmR] New ISID [%pmR]\n",
> + __func__, old_tddb->isid, new_tddb->isid));
>
> if (memcmp(&old_tddb->isid[0], &new_tddb->isid[0],
> sizeof(old_tddb->isid)))
> @@ -7925,10 +7921,7 @@ qla4xxx_sysfs_ddb_get_param(struct
> iscsi_bus_flash_session *fnode_sess,
> rc = sprintf(buf, "%u\n", fnode_conn->keepalive_timeout);
> break;
> case ISCSI_FLASHNODE_ISID:
> - rc = sprintf(buf, "%02x%02x%02x%02x%02x%02x\n",
> - fnode_sess->isid[0], fnode_sess->isid[1],
> - fnode_sess->isid[2], fnode_sess->isid[3],
> - fnode_sess->isid[4], fnode_sess->isid[5]);
> + rc = sprintf(buf, "%pm\n", fnode_sess->isid);
> break;
> case ISCSI_FLASHNODE_TSID:
> rc = sprintf(buf, "%u\n", fnode_sess->tsid);
Reviewed-by: Ewan D. Milne
--
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 v3 4/6] [SCSI] ips: don't use custom hex_asc_upper[] table
On Sat, 2016-10-22 at 20:32 +0300, Andy Shevchenko wrote:
> From: Andy Shevchenko
>
> We have table of the HEX characters in the kernel. Replace custom by a generic
> one.
>
> Cc: Adaptec OEM Raid Solutions
> Signed-off-by: Andy Shevchenko
> ---
> drivers/scsi/ips.c | 13 +
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
> index 02cb76f..3419e1b 100644
> --- a/drivers/scsi/ips.c
> +++ b/drivers/scsi/ips.c
> @@ -2241,9 +2241,6 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
> uint8_t minor;
> uint8_t subminor;
> uint8_t *buffer;
> - char hexDigits[] =
> - { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C',
> - 'D', 'E', 'F' };
>
> METHOD_TRACE("ips_get_bios_version", 1);
>
> @@ -2374,13 +2371,13 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
> }
> }
>
> - ha->bios_version[0] = hexDigits[(major & 0xF0) >> 4];
> + ha->bios_version[0] = hex_asc_upper_hi(major);
> ha->bios_version[1] = '.';
> - ha->bios_version[2] = hexDigits[major & 0x0F];
> - ha->bios_version[3] = hexDigits[subminor];
> + ha->bios_version[2] = hex_asc_upper_lo(major);
> + ha->bios_version[3] = hex_asc_upper_lo(subminor);
> ha->bios_version[4] = '.';
> - ha->bios_version[5] = hexDigits[(minor & 0xF0) >> 4];
> - ha->bios_version[6] = hexDigits[minor & 0x0F];
> + ha->bios_version[5] = hex_asc_upper_hi(minor);
> + ha->bios_version[6] = hex_asc_upper_lo(minor);
> ha->bios_version[7] = 0;
> }
>
Reviewed-by: Ewan D. Milne
--
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 v3 5/6] scsi: replace custom approach to hexdump small buffers
On Sat, 2016-10-22 at 20:32 +0300, Andy Shevchenko wrote:
> In kernel we have defined specifier (%*ph[C]) to dump small buffers in a hex
> format. Replace custom approach by a generic one.
>
> Cc: Jon Mason
> Signed-off-by: Andy Shevchenko
> ---
> drivers/scsi/scsi_transport_srp.c | 11 +--
> drivers/scsi/sd.c | 4 +---
> 2 files changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_srp.c
> b/drivers/scsi/scsi_transport_srp.c
> index e3cd3ec..02cfc6b 100644
> --- a/drivers/scsi/scsi_transport_srp.c
> +++ b/drivers/scsi/scsi_transport_srp.c
> @@ -115,21 +115,12 @@ static DECLARE_TRANSPORT_CLASS(srp_host_class,
> "srp_host", srp_host_setup,
> static DECLARE_TRANSPORT_CLASS(srp_rport_class, "srp_remote_ports",
> NULL, NULL, NULL);
>
> -#define SRP_PID(p) \
> - (p)->port_id[0], (p)->port_id[1], (p)->port_id[2], (p)->port_id[3], \
> - (p)->port_id[4], (p)->port_id[5], (p)->port_id[6], (p)->port_id[7], \
> - (p)->port_id[8], (p)->port_id[9], (p)->port_id[10], (p)->port_id[11], \
> - (p)->port_id[12], (p)->port_id[13], (p)->port_id[14], (p)->port_id[15]
> -
> -#define SRP_PID_FMT "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x:" \
> - "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x"
> -
> static ssize_t
> show_srp_rport_id(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> struct srp_rport *rport = transport_class_to_srp_rport(dev);
> - return sprintf(buf, SRP_PID_FMT "\n", SRP_PID(rport));
> + return sprintf(buf, "%16phC\n", rport->port_id);
> }
>
> static DEVICE_ATTR(port_id, S_IRUGO, show_srp_rport_id, NULL);
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b9618ff..5634b54 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2472,9 +2472,7 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp,
> unsigned char *buffer)
> if (sdkp->first_scan || old_wp != sdkp->write_prot) {
> sd_printk(KERN_NOTICE, sdkp, "Write Protect is %s\n",
> sdkp->write_prot ? "on" : "off");
> - sd_printk(KERN_DEBUG, sdkp,
> - "Mode Sense: %02x %02x %02x %02x\n",
> - buffer[0], buffer[1], buffer[2], buffer[3]);
> + sd_printk(KERN_DEBUG, sdkp, "Mode Sense: %4ph\n",
> buffer);
> }
> }
> }
Reviewed-by: Ewan D. Milne
--
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/1] libfc: don't have fc_exch_find log errors on a new exchange
On Fri, 2016-10-21 at 14:10 -0700, Chris Leech wrote:
> With the error message I added in "libfc: sanity check cpu number
> extracted from xid" I didn't account for the fact that fc_exch_find is
> called with FC_XID_UNKNOWN at the start of a new exchange if we are the
> responder.
>
> It doesn't come up with the initiator much, but that's basically every
> exchange for a target. By checking the xid for FC_XID_UNKNOWN first, we
> not only prevent the erroneous error message, but skip the unnecessary
> lookup attempt as well.
>
> Signed-off-by: Chris Leech
> ---
> drivers/scsi/libfc/fc_exch.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
> index 16ca31a..42cc403 100644
> --- a/drivers/scsi/libfc/fc_exch.c
> +++ b/drivers/scsi/libfc/fc_exch.c
> @@ -910,6 +910,9 @@ static struct fc_exch *fc_exch_find(struct fc_exch_mgr
> *mp, u16 xid)
> struct fc_exch *ep = NULL;
> u16 cpu = xid & fc_cpu_mask;
>
> + if (xid == FC_XID_UNKNOWN)
> + return NULL;
> +
> if (cpu >= nr_cpu_ids || !cpu_possible(cpu)) {
> printk_ratelimited(KERN_ERR
> "libfc: lookup request for XID = %d, "
Reviewed-by: Ewan D. Milne
--
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 v3 4/8] megaraid_sas: Send SYNCHRONIZE_CACHE for non-raid to firmware
On Fri, 2016-10-21 at 06:33 -0700, Kashyap Desai wrote:
> Commit- " 02b01e0 [SCSI] megaraid_sas: return sync cache call with success"
> added the code in driver to return SYNCHRONIZE_CACHE without sending it to
> firmware back in 2007. Earlier MR was mainly for Virtual Disk,
> so same code continue for JBOD as well whenever JBOD support was added and it
> introduced bug that
> SYNCHRONIZE_CACHE is not passed to FW for JBOD (non Raid disk).
>
> But our recent analysis indicates that, From Day-1 MR Firmware always
> expect Driver to forward SYNCHRONIZE_CACHE for JBOD (non Raid disk) to the
> Firmware.
> We have fixed this as part of this patch.
>
> CC: sta...@vger.kernel.org
> Signed-off-by: Kashyap Desai
> Signed-off-by: Sumit Saxena
> ---
> drivers/scsi/megaraid/megaraid_sas_base.c | 13 +
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index ba57be6..c98d4f9 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -1700,16 +1700,13 @@ megasas_queue_command(struct Scsi_Host *shost, struct
> scsi_cmnd *scmd)
> goto out_done;
> }
>
> - switch (scmd->cmnd[0]) {
> - case SYNCHRONIZE_CACHE:
> - /*
> - * FW takes care of flush cache on its own
> - * No need to send it down
> - */
> + /*
> + * FW takes care of flush cache on its own for Virtual Disk.
> + * No need to send it down for VD. For JBOD send SYNCHRONIZE_CACHE to
> FW.
> + */
> + if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) && MEGASAS_IS_LOGICAL(scmd)) {
> scmd->result = DID_OK << 16;
> goto out_done;
> - default:
> - break;
> }
>
> return instance->instancet->build_and_issue_cmd(instance, scmd);
Along with 4/8 in this v3 series...
Reviewed-by: Ewan D. Milne
--
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 v3 5/8] megaraid_sas: Send SYNCHRONIZE_CACHE for VD to firmware
araid/megaraid_sas_fusion.c > @@ -748,6 +748,11 @@ megasas_ioc_init_fusion(struct megasas_instance > *instance) > goto fail_fw_init; > } > > + instance->fw_sync_cache_support = (scratch_pad_2 & > + MR_CAN_HANDLE_SYNC_CACHE_OFFSET) ? 1 : 0; > + dev_info(&instance->pdev->dev, "FW supports sync cache\t: %s\n", > + instance->fw_sync_cache_support ? "Yes" : "No"); > + > IOCInitMessage = > dma_alloc_coherent(&instance->pdev->dev, >sizeof(struct MPI2_IOC_INIT_REQUEST), Reviewed-by: Ewan D. Milne -- 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] scsi_debug: Fix memory leak if LBP enabled and module is unloaded
From: "Ewan D. Milne" map_storep was not being vfree()'d in the module_exit call. Signed-off-by: Ewan D. Milne --- drivers/scsi/scsi_debug.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index c905709..cf04a36 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -5134,6 +5134,7 @@ static void __exit scsi_debug_exit(void) bus_unregister(&pseudo_lld_bus); root_device_unregister(pseudo_primary); + vfree(map_storep); vfree(dif_storep); vfree(fake_storep); kfree(sdebug_q_arr); -- 2.1.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: [PATCH] vmw_pvscsi: return SUCCESS for successful command aborts
On Fri, 2016-10-28 at 12:27 -0400, David Jeffery wrote:
> The vmw_pvscsi driver reports most successful aborts as FAILED to the scsi
> error handler. This is do to a misunderstanding of how completion_done()
> works
> and its interaction with a successful wait using
> wait_for_completion_timeout().
> The vmw_pvscsi driver is expecting completion_done() to always return true if
> complete() has been called on the completion structure. But completion_done()
> returns true after complete() has been called only if no function like
> wait_for_completion_timeout() has seen the completion and cleared it as part
> of
> successfully waiting for the completion.
>
> Instead of using completion_done(), vmw_pvscsi should just use the return
> value from wait_for_completion_timeout() to know if the wait timed out or not.
>
> Signed-off-by: David Jeffery
>
> ---
> vmw_pvscsi.c |5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> --- a/drivers/scsi/vmw_pvscsi.c
> +++ b/drivers/scsi/vmw_pvscsi.c
> @@ -793,6 +793,7 @@ static int pvscsi_abort(struct scsi_cmnd
> unsigned long flags;
> int result = SUCCESS;
> DECLARE_COMPLETION_ONSTACK(abort_cmp);
> + int done;
>
> scmd_printk(KERN_DEBUG, cmd, "task abort on host %u, %p\n",
> adapter->host->host_no, cmd);
> @@ -824,10 +825,10 @@ static int pvscsi_abort(struct scsi_cmnd
> pvscsi_abort_cmd(adapter, ctx);
> spin_unlock_irqrestore(&adapter->hw_lock, flags);
> /* Wait for 2 secs for the completion. */
> - wait_for_completion_timeout(&abort_cmp, msecs_to_jiffies(2000));
> + done = wait_for_completion_timeout(&abort_cmp, msecs_to_jiffies(2000));
> spin_lock_irqsave(&adapter->hw_lock, flags);
>
> - if (!completion_done(&abort_cmp)) {
> + if (!done) {
> /*
>* Failed to abort the command, unmark the fact that it
>* was requested to be aborted.
> --
> 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
Reviewed-by: Ewan D. Milne
--
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_dh_rdac: switch to scsi_execute_req_flags()
On Tue, 2016-11-01 at 22:49 +0100, Hannes Reinecke wrote:
...
> static int get_lun_info(struct scsi_device *sdev, struct rdac_dh_data *h,
> char *array_name, u8 *array_id)
> {
> - int err, i;
> - struct c8_inquiry *inqp;
> + int err = SCSI_DH_IO, i;
> + struct c8_inquiry *inqp = &h->inq.c8;
>
> - err = submit_inquiry(sdev, 0xC8, sizeof(struct c8_inquiry), h);
> - if (err == SCSI_DH_OK) {
> - inqp = &h->inq.c8;
> + if (!scsi_get_vpd_page(sdev, 0xC8, (unsigned char *)inqp,
> +sizeof(struct c8_inquiry))) {
> if (inqp->page_code != 0xc8)
> return SCSI_DH_NOSYS;
> if (inqp->page_id[0] != 'e' || inqp->page_id[1] != 'd' ||
> @@ -453,20 +382,20 @@ static int get_lun_info(struct scsi_device *sdev,
> struct rdac_dh_data *h,
> *(array_name+ARRAY_LABEL_LEN-1) = '\0';
> memset(array_id, 0, UNIQUE_ID_LEN);
> memcpy(array_id, inqp->array_unique_id,
> inqp->array_uniq_id_len);
> + err = SCSI_DH_OK;
> }
> return err;
> }
>
In this and other places the patch changes the code from submitting the
INQUIRY/EVPD command for the page it wants, to calling scsi_get_vpd_page().
scsi_get_vpd_page() will return -EINVAL if the device did not report in
VPD page 0 that the requested page is in the list of supported pages.
Did you actually verify that the RDAC returns all of these VPD pages in
its VPD page 0 list?
I mean, I know it's supposed to, but these are old devices.
-Ewan
--
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_dh_rdac: switch to scsi_execute_req_flags()
On Wed, 2016-11-02 at 22:27 +0100, Hannes Reinecke wrote: > On 11/02/2016 04:44 PM, Ewan D. Milne wrote: > > In this and other places the patch changes the code from submitting the > > INQUIRY/EVPD command for the page it wants, to calling scsi_get_vpd_page(). > > scsi_get_vpd_page() will return -EINVAL if the device did not report in > > VPD page 0 that the requested page is in the list of supported pages. > > Did you actually verify that the RDAC returns all of these VPD pages in > > its VPD page 0 list? > > > > I mean, I know it's supposed to, but these are old devices. > > > Errm. > > Old? Don't tell that to the E-Series folk; you'll never again get > something sponsored :-) > > No, seriously: RDAC mode is continued to be supported even on the latest > models, and all firmware revisions I've got support VPD page 0x0. > And incidentally, this is the very same method we're using for basically > all tools accessing VPD page 0x83, be it in the kernel or something like > sg3_utils. > _Not_ asking for page 0x0 lead to crashes on several older devices. You're right, I was curious, so I resurrected our old RDAC, and it does in fact report the necessary supported VPD pages: # sg_vpd -H /dev/sg4 Supported VPD pages VPD page: 00 20 00 00 12 00 80 83 85 86 87 b0 b1 c0 c1 c2 c3 ... 10 c4 c8 c9 ca d0 e0 .. My concern was that older devices might not do this, I think there have been some arrays that had hidden commands/pages. -Ewan -- 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/4] qla2xxx: Fix mailbox command timeout due to starvation
On Fri, 2016-11-04 at 09:33 -0700, himanshu.madh...@cavium.com wrote:
...
> @@ -2349,6 +2349,17 @@ uint32_t qla2x00_isp_reg_stat(struct qla_hw_data *ha)
> return atomic_read(&vha->loop_state) == LOOP_READY;
> }
>
> +static void qla2x00_destroy_mbx_wq(struct qla_hw_data *ha)
> +{
> + struct workqueue_struct *wq = ha->mbx_wq;
> +
> + if (wq) {
> + ha->mbx_wq = NULL;
> + flush_workqueue(wq);
> + destroy_workqueue(wq);
> + }
> +}
> +
> /*
> * PCI driver interface
> */
There is already a function qla2x00_destroy_deferred_work() that
destroys 3 other workqueues.
...
> @@ -3059,6 +3079,8 @@ uint32_t qla2x00_isp_reg_stat(struct qla_hw_data *ha)
>
> qla2x00_free_fw_dump(ha);
>
> + qla2x00_destroy_mbx_wq(ha);
> +
> pci_disable_pcie_error_reporting(pdev);
> pci_disable_device(pdev);
> }
This code path (pci_driver->shutdown) does not appear to destroy the
other workqueues created by the driver. ???
> @@ -5011,6 +5033,8 @@ void qla2x00_relogin(struct scsi_qla_host *vha)
>*/
> qla2x00_free_sysfs_attr(base_vha, false);
>
> + qla2x00_destroy_mbx_wq(ha);
> +
> fc_remove_host(base_vha->host);
>
> scsi_remove_host(base_vha->host);
See above.
-Ewan
--
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 3/4] qla2xxx: Add Block Multi Queue functionality.
On Fri, 2016-11-04 at 09:33 -0700, himanshu.madh...@cavium.com wrote: > From: Michael Hernandez > > Tell the SCSI layer how many hardware queues we have based on the > number of max queue pairs created. The number of max queue pairs > created will depend on number of MSI X vector count or number of CPU's > in a system. > > This feature can be turned on via CONFIG_SCSI_MQ_DEFAULT or passing > scsi_mod.use_blk_mq=Y as a parameter to the kernel > Queue pair creation depend on module parameter "ql2xmqsupport", which > need to be enabled to create queue pair. > I don't understand this change at all. Setting ->nr_hw_queues causes the block layer to allocate a number of queues for that Scsi_Host object, but it does not appear as if this code uses that functionality. There is nothing in the patch that examines the tag to see which queue the request came in on, in order to map it to a hardware queue. Instead, this patch seems to be reworking the mechanism involved in NPIV vport creation, which creates an entirely separate Scsi_Host object. The driver was already creating separate request queues to the card for vports, so what does this have to do with Block-MQ? -Ewan -- 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: SG does not ignore dxferp (direct io + mmap)
On Mon, 2016-11-21 at 11:23 +0200, Eyal Ben David wrote: > Hi, > > The utility I mentioned is just a small program that I wrote to learn > more about the problem. > > It is a very simple read16 with options for mmap and dxferp as null or other. > > Here is the source code: > > == cut here == > > 2016-11-21 2:04 GMT+02:00 Laurence Oberman : > > > > > > - Original Message - > >> From: "Eyal Ben David" > >> To: linux-scsi@vger.kernel.org > >> Sent: Sunday, November 20, 2016 11:02:49 AM > >> Subject: SG does not ignore dxferp (direct io + mmap) > >> > >> Hi all, > >> > >> We have some IO utility that perform the IOs using sg and direct io with > >> mmap. > >> Our current systems are Ubuntu 14.04, RHEL 6,7 > >> The IO utility always set dxferp to either the address or mmap of > >> other allocation (valloc) > >> Setting dxferp was harmless since SG is supposed to ignore the address > >> if mmap IO is selected. > >> When porting to Ubuntu 16.04, we had a corruption problem - first byte > >> of a read task is always 0. > >> When setting dxferp as NULL the corruption does not occur any more. > >> This is a regression and not according to SCSI generic documentation. > >> > >> I wrote a small program that shows the change: > >> > >> Read indirect (no mmap), lba=0: > >> === > >> $ ./sg_mmap_read -d /dev/sg0 -l 0 > >> 000 eb 63 90 10 8e d0 bc 00 b0 b8 00 00 8e d8 8e c0 > >> > >> Read with mmap, lba=0, dxferp=NULL: > >> > >> $ ./sg_mmap_read -d /dev/sg0 -l 0 -m > >> 000 eb 63 90 10 8e d0 bc 00 b0 b8 00 00 8e d8 8e c0 > >> > >> Read with mmap, lba=0, dxferp=address from mmap > >> == > >> $ ./sg_mmap_read -d /dev/sg0 -l 0 -m -b > >> 000 00 63 90 10 8e d0 bc 00 b0 b8 00 00 8e d8 8e c0 > >> > >> On the older systems all results are the same. > >> > >> Thanks for any answer! > >> -- For what it's worth, I ran this on a 4.8 kernel and did not see your problem. I couldn't reproduce it on a RHEL kernel either. # ./sg_mmap_read -d /dev/sg0 -l 0 | od -x 000 63eb 1090 d08e 00bc b8b0 d88e c08e 020 befb 7c00 00bf b906 0200 a4f3 21ea 0006 040 be00 07be 0438 0b75 c683 8110 fefe 7507 060 ebf3 b416 b002 bb01 7c00 80b2 748a 8b01 100 # ./sg_mmap_read -d /dev/sg0 -l 0 -m | od -x 000 63eb 1090 d08e 00bc b8b0 d88e c08e 020 befb 7c00 00bf b906 0200 a4f3 21ea 0006 040 be00 07be 0438 0b75 c683 8110 fefe 7507 060 ebf3 b416 b002 bb01 7c00 80b2 748a 8b01 100 # ./sg_mmap_read -d /dev/sg0 -l 0 -m -b | od -x 000 63eb 1090 d08e 00bc b8b0 d88e c08e 020 befb 7c00 00bf b906 0200 a4f3 21ea 0006 040 be00 07be 0438 0b75 c683 8110 fefe 7507 060 ebf3 b416 b002 bb01 7c00 80b2 748a 8b01 100 The only recent relevant change I see is: commit 5ecee0a3ee8d74b6950cb41e8989b0c2174568d4 Author: Douglas Gilbert Date: Thu Mar 3 00:31:29 2016 -0500 sg: fix dxferp in from_to case However the kernel I used has that change, and the change purposely does not clear hp->dxferp if SG_DXFER_TO_FROM_DEV is specified, which your program does not, and the behavior is correct regardless. Can you modify your program to output the userspace address of your buffer? -Ewan -- 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: SG does not ignore dxferp (direct io + mmap)
On Mon, 2016-11-21 at 16:15 +0100, Johannes Thumshirn wrote: > > FWIW: > jthumshirn@linux-x5ow:~$ sudo ./sg_mmap_read -d /dev/sg0 -l 0 | hexdump > 000 c033 d08e 00bc 8e7c 8ec0 bed8 7c00 00bf > 010 b906 0200 f3fc 50a4 1c68 cb06 b9fb 0004 > 020 bebd 8007 007e 7c00 0f0b 0e85 8301 10c5 > 030 f1e2 18cd 5688 5500 46c6 0511 46c6 0010 > 040 > jthumshirn@linux-x5ow:~$ sudo ./sg_mmap_read -d /dev/sg0 -l 0 -m | hexdump > 000 c033 d08e 00bc 8e7c 8ec0 bed8 7c00 00bf > 010 b906 0200 f3fc 50a4 1c68 cb06 b9fb 0004 > 020 bebd 8007 007e 7c00 0f0b 0e85 8301 10c5 > 030 f1e2 18cd 5688 5500 46c6 0511 46c6 0010 > 040 > jthumshirn@linux-x5ow:~$ sudo ./sg_mmap_read -d /dev/sg0 -l 0 -m -b | hexdump > 000 c033 d08e 00bc 8e7c 8ec0 bed8 7c00 00bf > 010 b906 0200 f3fc 50a4 1c68 cb06 b9fb 0004 > 020 bebd 8007 007e 7c00 0f0b 0e85 8301 10c5 > 030 f1e2 18cd 5688 5500 46c6 0511 46c6 0010 > 040 > jthumshirn@linux-x5ow:~$ uname -r > 4.8.4-1-default > > root@glass:~ # ~jthumshirn/sg_mmap_read -d /dev/sg0 -l 0 | hexdump > 000 caec 1dfe 3fdc 5b26 5775 eff6 a760 d612 > 010 c43c 8551 cfb8 4630 5e61 0750 44f3 c99e > 020 5718 84fd 073f e2f5 0f30 f1fa 1e1a 51e6 > 030 3379 217a 7d0f 8040 11cd 261f 3f8b 6de2 > 040 > root@glass:~ # ~jthumshirn/sg_mmap_read -d /dev/sg0 -l 0 -m | hexdump > 000 cd00 5f33 8809 0002 > 010 0007 f000 7fff > 020 6e9d 57ac > 030 > 040 > root@glass:~ # ~jthumshirn/sg_mmap_read -d /dev/sg0 -l 0 -m -b | hexdump > 000 4c00 bd7e 8805 0002 > 010 002f f000 7fff > 020 6e9d 57ac > 030 > 040 > root@glass:~ # uname -r > 4.4.21-69-default > root@glass:~ # > > > Bt: > root@glass:~ # ~jthumshirn/sg_mmap_read -d /dev/sg0 -l 0 -| hexdump > 000 46c0 b3b0 8804 0002 > 010 0021 f000 7fff > 020 6e9d 57ac > 030 626c 3170 6f5f 2070 2020 2020 3d20 3020 > 040 > root@glass:~ # ~jthumshirn/sg_mmap_read -d /dev/sg0 -l 0 -| hexdump > 000 > * > 040 > root@glass:~ # ~jthumshirn/sg_mmap_read -d /dev/sg0 -l 0 -| hexdump > 000 > * > 040 > root@glass:~ # ~jthumshirn/sg_mmap_read -d /dev/sg0 -l 0 -| hexdump > 000 73db ffe0 7a7d 1bf6 85cf 39ab a094 7802 > 010 959c 5b58 dc6f 1a36 5e7d 0afe d5be 3d41 > 020 691e 36ee 458c 14d2 06ed 371b 24dd 1f45 > 030 428a 24a0 866a 4a19 4954 e46a afe9 f3df > 040 > glass:~ # > > Maybe we have a race here. > > for i in $(seq 1 100); do > sudo ./sg_mmap_read -d /dev/sg0 -l 0 -m -b | hexdump > done > on 4.8 works fine. > > On the 4.4 host it has some hickups. > > I'll have a look at it. > > Byte, > Johannes > Sure enough. >From a freshly checked-out and built 4.4, i.e.: commit afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc Author: Linus Torvalds Date: Sun Jan 10 15:01:32 2016 -0800 Linux 4.4 --- # ./sg_mmap_read -d /dev/sg0 -l 0 | od -x 000 63eb 1090 d08e 00bc b8b0 d88e c08e 020 befb 7c00 00bf b906 0200 a4f3 21ea 0006 040 be00 07be 0438 0b75 c683 8110 fefe 7507 060 ebf3 b416 b002 bb01 7c00 80b2 748a 8b01 100 # ./sg_mmap_read -d /dev/sg0 -l 0 -m | od -x 000 63eb 1090 d08e 00bc b8b0 d88e c08e 020 befb 7c00 00bf b906 0200 a4f3 21ea 0006 040 be00 07be 0438 0b75 c683 8110 fefe 7507 060 ebf3 b416 b002 bb01 7c00 80b2 748a 8b01 100 # ./sg_mmap_read -d /dev/sg0 -l 0 -m -b | od -x 000 6300 1090 d08e 00bc b8b0 d88e c08e<= WRONG 020 befb 7c00 00bf b906 0200 a4f3 21ea 0006 040 be00 07be 0438 0b75 c683 8110 fefe 7507 060 ebf3 b416 b002 bb01 7c00 80b2 748a 8b01 100 -Ewan -- 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: SG does not ignore dxferp (direct io + mmap)
On Mon, 2016-11-21 at 12:34 -0500, Douglas Gilbert wrote: > There was also this change which seems closer to the problem area: > > commit 461c7fa126794157484dca48e88effa4963e3af3 > Author: Kirill A. Shutemov > Date: Tue Feb 2 16:57:35 2016 -0800 > > drivers/scsi/sg.c: mark VMA as VM_IO to prevent migration > ... > > diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c > index 503ab8b..5e82067 100644 > --- a/drivers/scsi/sg.c > +++ b/drivers/scsi/sg.c > @@ -1261,7 +1261,7 @@ sg_mmap(struct file *filp, struct vm_area_struct *vma) > } > > sfp->mmap_called = 1; > - vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP; > + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; > vma->vm_private_data = sfp; > vma->vm_ops = &sg_mmap_vm_ops; > return 0; > > Doug Gilbert > Neither this change nor "sg: fix dxferp in from_to case" appears to fix the issue when applied on top of 4.4. Still looking... -Ewan -- 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: SG does not ignore dxferp (direct io + mmap)
On Tue, 2016-11-22 at 09:37 +0100, Johannes Thumshirn wrote: > On Mon, Nov 21, 2016 at 01:24:02PM -0500, Ewan Milne wrote: > > On Mon, 2016-11-21 at 12:34 -0500, Douglas Gilbert wrote: > > > There was also this change which seems closer to the problem area: > > > > > > commit 461c7fa126794157484dca48e88effa4963e3af3 > > > Author: Kirill A. Shutemov > > > Date: Tue Feb 2 16:57:35 2016 -0800 > > > > > > drivers/scsi/sg.c: mark VMA as VM_IO to prevent migration > > > ... > > > > > > diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c > > > index 503ab8b..5e82067 100644 > > > --- a/drivers/scsi/sg.c > > > +++ b/drivers/scsi/sg.c > > > @@ -1261,7 +1261,7 @@ sg_mmap(struct file *filp, struct vm_area_struct > > > *vma) > > > } > > > > > > sfp->mmap_called = 1; > > > - vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP; > > > + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; > > > vma->vm_private_data = sfp; > > > vma->vm_ops = &sg_mmap_vm_ops; > > > return 0; > > > > > > Doug Gilbert > > > > > > > Neither this change nor "sg: fix dxferp in from_to case" appears to > > fix the issue when applied on top of 4.4. Still looking... > > This brings bad memories from commit 2d99b55d3 back to live, but this is > applied on all test kernels I have. > > I too will run some bisection as well now that we have an easy reproducer and > my timezone is somewhat ahead. Let's see if we can stretch the workday a bit. I see the behavior (zero byte) on the 4.4.34, 4.5.7, 4.6.7, and 4.7.10 -stable kernels. But not (of course) on 4.8.10 -stable. It doesn't look like the sg driver, might be something in the mmap code? -Ewan -- 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: SG does not ignore dxferp (direct io + mmap)
On Wed, 2016-11-23 at 13:55 -0500, Laurence Oberman wrote: > > - Original Message - > > From: "Eyal Ben David" > > To: "Ewan D. Milne" > > Cc: "Johannes Thumshirn" , dgilb...@interlog.com, > > "Laurence Oberman" , > > linux-scsi@vger.kernel.org > > Sent: Tuesday, November 22, 2016 3:55:44 PM > > Subject: Re: SG does not ignore dxferp (direct io + mmap) > > > > On Tue, Nov 22, 2016 at 8:30 PM, Ewan D. Milne wrote: > > > > > > I see the behavior (zero byte) on the 4.4.34, 4.5.7, 4.6.7, and 4.7.10 > > > -stable kernels. But not (of course) on 4.8.10 -stable. > > > > > > It doesn't look like the sg driver, might be something in the mmap code? > > > > > > A kernel guy colleague suggested to look at copy_from_user / copy_to_user > > code. > > It was changed in 4.8 > > > > It was OK with 3.13 (Ubuntu 14.04) but from some kernel (prior or equal to > > 4.4) > > until 4.7 we see the bug. It was somehow fixed at 4.8. > > > > In order to fully understand what happened, there are two changes to find. > > They might not even be related. > > > > Thanks! > > Eyal > > -- > > 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 > > > > So 4.7.9 fails and 4.8.0 works and 4.8.0 is a rebase so we have > > [loberman@localhost linux-stable-4.8.10]$ git log --oneline v4.7.9..v4.8 | wc > -l > 14552 > > No obvious single commits stand out for me for copy_from* or copy_to* > There is this: > > 3fa6c50 mm: optimize copy_page_to/from_iter_iovec > 6e05050 sh: fix copy_from_user() > e697100 x86/uaccess: force copy_*_user() to be inlined > > I will have to do this the hard way with bisects to figure out which commit > addresses this. > > Back when I have had enough time to do it. > > Thanks > Laurence Yes, in fact... Bisecting between 4.7 and 4.8 to see where the behavior changes so that the zero byte no longer appears leads to this commit: --- Bisecting: 0 revisions left to test after this (roughly 0 steps) [3fa6c507319c897598512da91c010a4ad2ed682c] mm: optimize copy_page_to/from_iter_iovec --- commit 3fa6c507319c897598512da91c010a4ad2ed682c Author: Mikulas Patocka Date: Thu Jul 28 15:48:50 2016 -0700 mm: optimize copy_page_to/from_iter_iovec copy_page_to_iter_iovec() and copy_page_from_iter_iovec() copy some data to userspace or from userspace. These functions have a fast path where they map a page using kmap_atomic and a slow path where they use kmap. kmap is slower than kmap_atomic, so the fast path is preferred. However, on kernels without highmem support, kmap just calls page_address, so there is no need to avoid kmap. On kernels without highmem support, the fast path just increases code size (and cache footprint) and it doesn't improve copy performance in any way. This patch enables the fast path only if CONFIG_HIGHMEM is defined. Code size reduced by this patch: x86 (without highmem) 928 x86-64 960 sparc64 848 alpha 1136 pa-risc1200 [a...@linux-foundation.org: use IS_ENABLED(), per Andi] Link: http://lkml.kernel.org/r/alpine.lrh.2.02.1607221711410.4...@file01.intranet.prod.int.rdu2.redhat.com Signed-off-by: Mikulas Patocka Cc: Hugh Dickins Cc: Michal Hocko Cc: Alexander Viro Cc: Mel Gorman Cc: Johannes Weiner Cc: Andi Kleen Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- In other words, this commit made the bad behavior go away in 4.8. Need to look at this in more detail, it doesn't appear as if this patch was intended to fix such a problem. -Ewan -- 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: SG does not ignore dxferp (direct io + mmap)
On Fri, 2016-11-25 at 12:56 -0500, Ewan Milne wrote: > I think what we need to understand is what caused the regression in the > first place, I probably should have been bisecting the original failure > rather than trying to find where it started working. > Bisecting leads to this commit: commit 37f19e57a0de3c4a3417aa13ff4d04f1e0dee4b3 Author: Christoph Hellwig Date: Sun Jan 18 16:16:33 2015 +0100 block: merge __bio_map_user_iov into bio_map_user_iov And also remove the unused bdev argument. Signed-off-by: Christoph Hellwig Reviewed-by: Ming Lei Signed-off-by: Jens Axboe Specifically, the problem appears to be caused by the removal of the setting of bio->bi_bdev, which would previously be set to NULL. If I add: diff --git a/block/bio.c b/block/bio.c index 0723d4c..ecac37b 100644 --- a/block/bio.c +++ b/block/bio.c @@ -1351,6 +1351,7 @@ struct bio *bio_map_user_iov(struct request_queue *q, if (iter->type & WRITE) bio->bi_rw |= REQ_WRITE; + bio->bi_bdev = NULL; bio->bi_flags |= (1 << BIO_USER_MAPPED); /* The test passes (no zero byte corruption). Setting dxferp would cause map_data.null_mapped to be set before it is passed to blk_rq_map_user(_iov) which would cause a difference in behavior. -Ewan -- 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: SG does not ignore dxferp (direct io + mmap)
On Fri, 2016-12-02 at 04:21 -0800, Christoph Hellwig wrote: > On Thu, Dec 01, 2016 at 08:40:31AM -0500, Martin K. Petersen wrote: > > Specifically, the problem appears to be caused by the removal of > > the setting of bio->bi_bdev, which would previously be set to NULL. > > If I add: > > Very odd. For one I would expect it to be NULL anyway, second > I don't see why the behavior changed. But given that this reverts > to the original assignment and makes things work I'll happily hack it > to get things working again: > > Acked-by: Christoph Hellwig Yeah, I'm not sure I understand this either, apart from the change adjusting the code to effectively do what it used to and making the test case work. I'm reluctant to cc: stable yet, let me look at this a bit more and I'll post the actual patch soon. -Ewan -- 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: SG does not ignore dxferp (direct io + mmap)
On Fri, 2016-12-02 at 15:10 +0100, Hannes Reinecke wrote: > On 12/02/2016 02:29 PM, Ewan D. Milne wrote: > > On Fri, 2016-12-02 at 04:21 -0800, Christoph Hellwig wrote: > >> On Thu, Dec 01, 2016 at 08:40:31AM -0500, Martin K. Petersen wrote: > >>> Specifically, the problem appears to be caused by the removal of > >>> the setting of bio->bi_bdev, which would previously be set to NULL. > >>> If I add: > >> > >> Very odd. For one I would expect it to be NULL anyway, second > >> I don't see why the behavior changed. But given that this reverts > >> to the original assignment and makes things work I'll happily hack it > >> to get things working again: > >> > >> Acked-by: Christoph Hellwig > > > > Yeah, I'm not sure I understand this either, apart from the change > > adjusting the code to effectively do what it used to and making the > > test case work. I'm reluctant to cc: stable yet, let me look at this > > a bit more and I'll post the actual patch soon. > > > Plus we found that this is basically a timing issue; we've found that > supposedly fixed bugs will crop up after ~4k iterations. > (Johannes did a _lot_ of testing here :-) > So just because the bug failed to materialize can also mean that you > simply didn't test long enough. > Yes, and following the code paths it isn't completely clear how this leads to the single zero-byte corruption, I am continuing to investigate. There may very well be more than one problem. On kernel versions I tested where I got a failure it was a solid failure, it never worked no matter how many times I tried, but I did not exhaustively test apparently successful kernel versions. Not thousands, of times, anyway. -Ewan -- 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: SG does not ignore dxferp (direct io + mmap)
On Thu, 2016-12-01 at 08:40 -0500, Martin K. Petersen wrote: > >>>>> "Ewan" == Ewan D Milne writes: ... > Specifically, the problem appears to be caused by the removal of > the setting of bio->bi_bdev, which would previously be set to NULL. > If I add: > > diff --git a/block/bio.c b/block/bio.c > index 0723d4c..ecac37b 100644 > --- a/block/bio.c > +++ b/block/bio.c > @@ -1351,6 +1351,7 @@ struct bio *bio_map_user_iov(struct request_queue > *q, > if (iter->type & WRITE) > bio->bi_rw |= REQ_WRITE; > > + bio->bi_bdev = NULL; > bio->bi_flags |= (1 << BIO_USER_MAPPED); > > /* > > Ewan> The test passes (no zero byte corruption). > Adding this seemed to work on top of the commit commit id mentioned above (37f19e57a0de3c4a3417aa13ff4d04f1e0dee4b3) but does not help on 4.7, because the test case now takes the bio_copy_user_iov() code path rather than bio_map_user_iov(). -Ewan -- 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: avoid a permanent stop of the scsi device's request queue
On Wed, 2016-12-07 at 08:55 -0800, Bart Van Assche wrote: > On 12/07/2016 08:48 AM, Bart Van Assche wrote: > > It's a known bug. Some time ago I posted a patch that serializes all > > scsi_device_set_state() calls but I have not yet found it in the list > > archives. However, that patch has not yet been merged. > > See also https://www.spinics.net/lists/linux-scsi/msg66966.html. > > Bart. > > -- > 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 Yes, however that patch does not fix Wei Fang's issue. In fact I just received a crash dump that appears to be the same thing. It looks like the rport went away right after the initial INQUIRY, so we set the state to SDEV_BLOCK and stop the queue, and then the scan code continues and sets the state back to SDEV_RUNNING. Then, when the devloss timer expires, we call scsi_target_unblock w/SDEV_TRANSPORT_OFFLINE, but the SDEV_RUNNING state prevents the queue from being restarted, so a subsequent command (i.e. the ALUA page 83 inquiry command) is stuck on the stopped queue. (The dump shows 3 devices on the target with queues running in SDEV_TRANSPORT_OFFLINE, and 1 device currently being scanned with the queue stopped in SDEV_RUNNING.) It seems to me the problem is that scsi_device_set_state() is allowing the caller to transition SDEV_BLOCK -> SDEV_RUNNING without actually restarting the queue and that should be an illegal transition. -Ewan -- 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: avoid a permanent stop of the scsi device's request queue
On Wed, 2016-12-07 at 10:16 -0800, James Bottomley wrote: > On Wed, 2016-12-07 at 12:40 -0500, Ewan D. Milne wrote: > > On Wed, 2016-12-07 at 08:55 -0800, Bart Van Assche wrote: > > > On 12/07/2016 08:48 AM, Bart Van Assche wrote: > > > > It's a known bug. Some time ago I posted a patch that serializes > > > > all scsi_device_set_state() calls but I have not yet found it in > > > > the list archives. However, that patch has not yet been merged. > > > > > > See also https://www.spinics.net/lists/linux-scsi/msg66966.html. > > > > > > Bart. > > > > > > -- > > > 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 > > > > Yes, however that patch does not fix Wei Fang's issue. In fact I > > just received a crash dump that appears to be the same thing. It > > looks like the rport went away right after the initial INQUIRY, so we > > set the state to SDEV_BLOCK and stop the queue, and then the scan > > code continues and sets the state back to SDEV_RUNNING. > > So here's the violation of the state model. the rport went CREATED > ->BLOCK which is wrong: it should go CREATED->CREATED_BLOCK and then > the add code would set it to BLOCK instead of RUNNING. > > The question to diagnose is why CREATED->BLOCK worked. > > James > I believe scsi_add_lun() changed the state from CREATED->RUNNING which allowed the state to change from RUNNING->BLOCK, and then scsi_sysfs_add_sdev() called scsi_device_set_state() which changed the state from BLOCK->RUNNING. But did not restart the queue. I have a debug kernel out to the site that found this to make sure, assuming they can reproduce this, but I don't see any other way it could have happened. -Ewan -- 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: avoid a permanent stop of the scsi device's request queue
On Wed, 2016-12-07 at 12:09 -0800, James Bottomley wrote:
> Hm, it looks like the state set in scsi_sysfs_add_sdev() is bogus. We
> expect the state to have been properly set before that (in
> scsi_add_lun), so can we not simply remove it?
>
> James
>
I was considering that, but...
enum scsi_device_state {
SDEV_CREATED = 1, /* device created but not added to sysfs
* Only internal commands allowed (for inq) */
So it seems the intent was for the state to not change until then.
The call to set the SDEV_RUNNING state earlier in scsi_add_lun()
was added with:
commit 6f4267e3bd1211b3d09130e626b0b3d885077610
Author: James Bottomley
Date: Fri Aug 22 16:53:31 2008 -0500
[SCSI] Update the SCSI state model to allow blocking in the created state
Which allows the device to go into ->BLOCK (which is needed, since it
actually happens).
Should we remove the call from scsi_sysfs_add_sdev() and change the
comment in scsi_device.h to reflect the intent?
I have not verified the async vs. non-async scan path yet but it looks
like it would be OK.
-Ewan
--
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: avoid a permanent stop of the scsi device's request queue
On Thu, 2016-12-08 at 14:38 +0800, Wei Fang wrote:
> Hi, James, Ewan, Bart,
>
> On 2016/12/8 11:22, Wei Fang wrote:
> > I looked through those code and found that if we fix this bug
> > by removing setting the state in scsi_sysfs_add_sdev(), it
> > can't be fixed completely:
> >
> > scsi_device_set_state(sdev, SDEV_RUNNING) in scsi_add_lun() and
> > scsi_device_set_state(sdev, SDEV_CREATED_BLOCK) in
> > scsi_internal_device_block()
> > can be called simultaneously. Because there is no synchronization
> > between scsi_device_set_state(), those calls may both return
> > success, and the state may be SDEV_RUNNING after that, and the
> > device queue is stopped.
>
> Can we fix it in this way:
>
> Add a state lock to make sure the result of simultaneously calling
> of scsi_device_set_state() is not unpredictable.
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 253ee74..80cb493 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -2457,10 +2457,16 @@ EXPORT_SYMBOL(scsi_test_unit_ready);
> int
> scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
> {
> - enum scsi_device_state oldstate = sdev->sdev_state;
> + enum scsi_device_state oldstate;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&sdev->state_lock, flags);
> + oldstate = sdev->sdev_state;
>
> - if (state == oldstate)
> + if (state == oldstate) {
> + spin_unlock_irqrestore(&sdev->state_lock, flags);
> return 0;
> + }
>
> switch (state) {
> case SDEV_CREATED:
> @@ -2558,9 +2564,11 @@ scsi_device_set_state(struct scsi_device *sdev, enum
> scsi_device_state state)
>
> }
> sdev->sdev_state = state;
> + spin_unlock_irqrestore(&sdev->state_lock, flags);
> return 0;
>
> illegal:
> + spin_unlock_irqrestore(&sdev->state_lock, flags);
> SCSI_LOG_ERROR_RECOVERY(1,
> sdev_printk(KERN_ERR, sdev,
> "Illegal state transition %s->%s",
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 6f7128f..ba2f38f 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -238,6 +238,7 @@ static struct scsi_device *scsi_alloc_sdev(struct
> scsi_target *starget,
> INIT_LIST_HEAD(&sdev->starved_entry);
> INIT_LIST_HEAD(&sdev->event_list);
> spin_lock_init(&sdev->list_lock);
> + spin_lock_init(&sdev->state_lock);
> mutex_init(&sdev->inquiry_mutex);
> INIT_WORK(&sdev->event_work, scsi_evt_thread);
> INIT_WORK(&sdev->requeue_work, scsi_requeue_run_queue);
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 0734927..82dfe07 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1204,10 +1204,6 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
> struct request_queue *rq = sdev->request_queue;
> struct scsi_target *starget = sdev->sdev_target;
>
> - error = scsi_device_set_state(sdev, SDEV_RUNNING);
> - if (error)
> - return error;
> -
> error = scsi_target_add(starget);
> if (error)
> return error;
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 8990e58..e00764e 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -31,7 +31,7 @@ struct scsi_mode_data {
> enum scsi_device_state {
> SDEV_CREATED = 1, /* device created but not added to sysfs
> * Only internal commands allowed (for inq) */
> - SDEV_RUNNING, /* device properly configured
> + SDEV_RUNNING, /* device properly initialized
> * All commands allowed */
> SDEV_CANCEL,/* beginning to delete device
> * Only error handler commands allowed */
> @@ -207,6 +207,7 @@ struct scsi_device {
> void*handler_data;
>
> unsigned char access_state;
> + spinlock_t state_lock;
> enum scsi_device_state sdev_state;
> unsigned long sdev_data[0];
> } __attribute__((aligned(sizeof(unsigned long;
>
> Haven't tested yet. Sending this for your opinion.
>
> Thanks,
> Wei
>
You would presumably need to take your lock in scsi_internal_device_unblock()
as well, since it also checks and updates sdev_state directly. There are also
places like scsi_device_resume() that examine the state before deciding to
call scsi_device_set_state().
-Ewan
--
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
[LSF/MM TOPIC] [LSF/MM ATTEND] XCOPY
I'd like to attend LSF/MM this year, and I'd like to discuss XCOPY and what we need to do to get this functionality incorporated. Martin/Mikulas' patches have been around for a while, I think the last holdup was some request flag changes that have since been resolved. We do need to have a token-based XCOPY implementation as that is what the array vendors are supporting. -Ewan -- 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 v3] scsi: avoid a permanent stop of the scsi device's request queue
On Mon, 2016-12-12 at 10:20 +0800, Wei Fang wrote:
> A race between scanning and fc_remote_port_delete() may result in a
> permanent stop if the device gets blocked before scsi_sysfs_add_sdev()
> and unblocked after. The reason is that blocking a device sets both
> the SDEV_BLOCKED state and the QUEUE_FLAG_STOPPED. However,
> scsi_sysfs_add_sdev() unconditionally sets SDEV_RUNNING which causes
> the device to be ignored by scsi_target_unblock() and thus never have
> its QUEUE_FLAG_STOPPED cleared leading to a device which is apparently
> running but has a stopped queue.
>
> We actually have two places where SDEV_RUNNING is set: once in
> scsi_add_lun() which respects the blocked flag and once in
> scsi_sysfs_add_sdev() which doesn't. Since the second set is entirely
> spurious, simply remove it to fix the problem.
>
> Reported-by: Zengxi Chen
> Signed-off-by: Wei Fang
> ---
> Changes v1->v2:
> - don't modify scsi_internal_device_unblock(), just remove changing
> state to SDEV_RUNNING in scsi_sysfs_add_sdev(), suggested by
> James Bottomley and Ewan D. Milne.
> Changes v2->v3
> - Use a clearer description of this problem
>
> drivers/scsi/scsi_sysfs.c | 4
> include/scsi/scsi_device.h | 2 +-
> 2 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 0734927..82dfe07 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1204,10 +1204,6 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
> struct request_queue *rq = sdev->request_queue;
> struct scsi_target *starget = sdev->sdev_target;
>
> - error = scsi_device_set_state(sdev, SDEV_RUNNING);
> - if (error)
> - return error;
> -
> error = scsi_target_add(starget);
> if (error)
> return error;
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 8990e58..8bfb37f 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -31,7 +31,7 @@ struct scsi_mode_data {
> enum scsi_device_state {
> SDEV_CREATED = 1, /* device created but not added to sysfs
>* Only internal commands allowed (for inq) */
> - SDEV_RUNNING, /* device properly configured
> + SDEV_RUNNING, /* device properly configured and not blocked
>* All commands allowed */
> SDEV_CANCEL,/* beginning to delete device
>* Only error handler commands allowed */
Well, James said not to bother with the comment, but OK.
I take it this has passed your testing. I have not heard back yet
from the site that reported this problem to me on their reproducer.
The change looks good to me.
Reviewed-by: Ewan D. Milne
--
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] snic: Return error code on memory allocation failure
On Wed, 2016-12-21 at 14:45 +0100, Burak Ok wrote:
> If a call to mempool_create_slab_pool() in snic_probe() returns NULL,
> return -ENOMEM to indicate failure. mempool_creat_slab_pool() only fails if
> it cannot allocate memory.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=189061
>
> Reported-by: bianpan2...@ruc.edu.cn
> Signed-off-by: Burak Ok
> Signed-off-by: Andreas Schaertl
> ---
> drivers/scsi/snic/snic_main.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/scsi/snic/snic_main.c b/drivers/scsi/snic/snic_main.c
> index 396b32d..7cf70aa 100644
> --- a/drivers/scsi/snic/snic_main.c
> +++ b/drivers/scsi/snic/snic_main.c
> @@ -591,6 +591,7 @@ snic_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> if (!pool) {
> SNIC_HOST_ERR(shost, "dflt sgl pool creation failed\n");
>
> + ret = -ENOMEM;
> goto err_free_res;
> }
>
> @@ -601,6 +602,7 @@ snic_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> if (!pool) {
> SNIC_HOST_ERR(shost, "max sgl pool creation failed\n");
>
> + ret = -ENOMEM;
> goto err_free_dflt_sgl_pool;
> }
>
> @@ -611,6 +613,7 @@ snic_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> if (!pool) {
> SNIC_HOST_ERR(shost, "snic tmreq info pool creation failed.\n");
>
> + ret = -ENOMEM;
> goto err_free_max_sgl_pool;
> }
>
Reviewed-by: Ewan D. Milne
--
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 v3] scsi: avoid a permanent stop of the scsi device's request queue
On Mon, 2016-12-12 at 11:23 -0500, Ewan D. Milne wrote: > ...I have not heard back yet > from the site that reported this problem to me on their reproducer. Not that it matters much now, but.. The response from my original reporter is that indeed the state was being changed from SDEV_BLOCK -> SDEV_RUNNING in scsi_sysfs_add_sdev() so the fix is correct. -Ewan -- 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] ses: Fix SAS device detection in enclosure
From: "Ewan D. Milne"
The call to scsi_is_sas_rphy() needs to be made on the
SAS end_device, not on the SCSI device.
Fixes: 835831c57e9b ("ses: use scsi_is_sas_rphy instead of is_sas_attached")
Reviewed-by: Johannes Thumshirn
Signed-off-by: Ewan D. Milne
---
drivers/scsi/ses.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 8c9a35c..50adabb 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -587,7 +587,7 @@ static void ses_match_to_enclosure(struct enclosure_device
*edev,
ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0);
- if (scsi_is_sas_rphy(&sdev->sdev_gendev))
+ if (scsi_is_sas_rphy(sdev->sdev_target->dev.parent))
efd.addr = sas_get_address(sdev);
if (efd.addr) {
--
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: qedi: select UIO
On Tue, 2017-01-10 at 16:27 +0100, Arnd Bergmann wrote:
> The newly added qedi driver links against the UIO framework, but can
> be built without that:
>
> drivers/scsi/qedi/qedi_main.o: In function `qedi_free_uio':
> qedi_main.c:(.text.qedi_free_uio+0x78): undefined reference to
> `uio_unregister_device'
> drivers/scsi/qedi/qedi_main.o: In function `qedi_ll2_recv_thread':
> qedi_main.c:(.text.qedi_ll2_recv_thread+0x18c): undefined reference to
> `uio_event_notify'
> drivers/scsi/qedi/qedi_main.o: In function `__qedi_probe.constprop.1':
> qedi_main.c:(.text.__qedi_probe.constprop.1+0x1368): undefined reference to
> `__uio_register_device'
>
> This adds a compile-time dependency.
>
> Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI driver
> framework.")
> Signed-off-by: Arnd Bergmann
> ---
> drivers/scsi/qedi/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/qedi/Kconfig b/drivers/scsi/qedi/Kconfig
> index 23ca8a274586..913610f3d274 100644
> --- a/drivers/scsi/qedi/Kconfig
> +++ b/drivers/scsi/qedi/Kconfig
> @@ -2,6 +2,7 @@ config QEDI
> tristate "QLogic QEDI 25/40/100Gb iSCSI Initiator Driver Support"
> depends on PCI && SCSI
> depends on QED
> + depends on UIO
> select SCSI_ISCSI_ATTRS
> select QED_LL2
> select QED_ISCSI
Randy posted a similar patch back in December but I don't think there
was ever a reply to Christoph's question about why qedi depends on uio.
-Ewan
--
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] sd: always scan VPD pages if thin provisioning is enabled
On Wed, 2017-01-25 at 11:38 +0100, Hannes Reinecke wrote: > On 01/25/2017 11:23 AM, Christoph Hellwig wrote: > > On Wed, Jan 25, 2017 at 08:26:05AM +0100, Hannes Reinecke wrote: > >> For any device with an older SCSI revision we might not > >> be scanning VPD pages, which results in a wrongly configured > >> discard mode if thin provisioned is enabled. > >> According to sbc3 any thin provisioned device (ie devices which > >> have the LBPME bit set in the output of READ CAPACITY(16)) need > >> to support VPD pages. So this patch always enables VPD pages > >> even for older SCSI revisions if thin provisioning is enabled. > > > > Can you explain what you need this for? A device with a per-SBC3 > > revision that wants us to use UNMAP? > > > Some storage arrays essentially lie about the SCSI revision (most > notably Hitachi :-), and some claim to support SPC-2 (or even SPC) but > support newer features, too. Most notably VPD pages support. > In this case it was an HP EVA claiming to support SPC-2 only, but > providing thin provisioning. Um, isn't this why we added: commit c1d40a527e885a40bb9ea6c46a1b1145d42b66a0 Author: Martin K. Petersen Date: Tue Jul 15 12:49:17 2014 -0400 scsi: add a blacklist flag which enables VPD page inquiries (well, it was for storvsc, but we could add an entry for the HP EVA) > >> + /* > >> + * sbc3r36 states: > >> + * The device server in a logical unit the supports > >> + * logical block provisioning management shall support > >> + * the Logical Block Provisioning VPD page. > >> + * So VPD pages should be supported if lbpme is set. > >> + */ > > > > It's a bit odd to quote SBC3 when the device clearly is pre-SBC3 > > to need this workaround.. > > > _Actually_ it's pre-SPC-3. > > But that was the earliest draft I had :-( > I'd be happy to modify this if I had access to sbc-2 drafts. > > >> + if (!scsi_device_supports_vpd(sdp)) > >> + sdp->try_vpd_pages = 1; > > > > Do the assignment unconditionally? > > > Yeah, can do. > > Cheers, > > Hannes -- 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_error: count medium access timeout only once per EH run
rivers/scsi/sd.h
> index 4dac35e..19e0bab 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -85,6 +85,7 @@ struct scsi_disk {
> unsigned intphysical_block_size;
> unsigned intmax_medium_access_timeouts;
> unsigned intmedium_access_timed_out;
> + unsigned intmedium_access_reset;
This could be an unsigned int : 1 with the other single bit fields at
the end of the structure, with the change above.
> u8 media_present;
> u8 write_prot;
> u8 protection_type;/* Data Integrity Field */
> diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
> index a1e1930..b6c750f 100644
> --- a/include/scsi/scsi.h
> +++ b/include/scsi/scsi.h
> @@ -185,6 +185,7 @@ static inline int scsi_is_wlun(u64 lun)
> #define TIMEOUT_ERROR 0x2007
> #define SCSI_RETURN_NOT_HANDLED 0x2008
> #define FAST_IO_FAIL 0x2009
> +#define NEEDS_RESET 0x2010
See above, this is overloading the use of the parameter.
>
> /*
> * Midlevel queue return values.
Reviewed-by: Ewan D. Milne
Re: [PATCHv3] scsi: disable automatic target scan
user_scan_exit:
> @@ -1836,6 +1837,7 @@ static int iscsi_user_scan(struct Scsi_Host *shost,
> uint channel,
> scan_data.channel = channel;
> scan_data.id = id;
> scan_data.lun = lun;
> + scan_data.rescan = SCSI_SCAN_MANUAL;
>
> return device_for_each_child(&shost->shost_gendev, &scan_data,
>iscsi_user_scan_session);
> @@ -1852,6 +1854,7 @@ static void iscsi_scan_session(struct work_struct *work)
> scan_data.channel = 0;
> scan_data.id = SCAN_WILD_CARD;
> scan_data.lun = SCAN_WILD_CARD;
> + scan_data.rescan = SCSI_SCAN_RESCAN;
>
> iscsi_user_scan_session(&session->dev, &scan_data);
> atomic_dec(&ihost->nr_scans);
> diff --git a/drivers/scsi/scsi_transport_sas.c
> b/drivers/scsi/scsi_transport_sas.c
> index b6f958193..3f0ff07 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -1614,7 +1614,8 @@ int sas_rphy_add(struct sas_rphy *rphy)
> else
> lun = 0;
>
> - scsi_scan_target(&rphy->dev, 0, rphy->scsi_target_id, lun, 0);
> + scsi_scan_target(&rphy->dev, 0, rphy->scsi_target_id, lun,
> + SCSI_SCAN_INITIAL);
> }
>
> return 0;
> @@ -1739,8 +1740,8 @@ static int sas_user_scan(struct Scsi_Host *shost, uint
> channel,
>
> if ((channel == SCAN_WILD_CARD || channel == 0) &&
> (id == SCAN_WILD_CARD || id == rphy->scsi_target_id)) {
> - scsi_scan_target(&rphy->dev, 0,
> - rphy->scsi_target_id, lun, 1);
> + scsi_scan_target(&rphy->dev, 0, rphy->scsi_target_id,
> + lun, SCSI_SCAN_MANUAL);
> }
> }
> mutex_unlock(&sas_host->lock);
> diff --git a/drivers/scsi/snic/snic_disc.c b/drivers/scsi/snic/snic_disc.c
> index 5f63217..5f48795 100644
> --- a/drivers/scsi/snic/snic_disc.c
> +++ b/drivers/scsi/snic/snic_disc.c
> @@ -171,7 +171,7 @@ snic_scsi_scan_tgt(struct work_struct *work)
>tgt->channel,
>tgt->scsi_tgt_id,
>SCAN_WILD_CARD,
> - 1);
> + SCSI_SCAN_RESCAN);
>
> spin_lock_irqsave(shost->host_lock, flags);
> tgt->flags &= ~SNIC_TGT_SCAN_PENDING;
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index c067019..61341d3 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -50,6 +50,12 @@ enum scsi_device_state {
> SDEV_CREATED_BLOCK, /* same as above but for created devices */
> };
>
> +enum scsi_scan_mode {
> + SCSI_SCAN_INITIAL = 0,
> + SCSI_SCAN_RESCAN,
> + SCSI_SCAN_MANUAL,
> +};
> +
> enum scsi_device_event {
> SDEV_EVT_MEDIA_CHANGE = 1,/* media has changed */
> SDEV_EVT_INQUIRY_CHANGE_REPORTED, /* 3F 03 UA reported */
> @@ -391,7 +397,8 @@ extern void scsi_device_resume(struct scsi_device *sdev);
> extern void scsi_target_quiesce(struct scsi_target *);
> extern void scsi_target_resume(struct scsi_target *);
> extern void scsi_scan_target(struct device *parent, unsigned int channel,
> - unsigned int id, u64 lun, int rescan);
> + unsigned int id, u64 lun,
> + enum scsi_scan_mode rescan);
> extern void scsi_target_reap(struct scsi_target *);
> extern void scsi_target_block(struct device *);
> extern void scsi_target_unblock(struct device *, enum scsi_device_state);
Reviewed-by: Ewan D. Milne
--
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_common: do not clobber fixed sense information
On Fri, 2016-03-18 at 09:01 +0100, Hannes Reinecke wrote:
> For fixed sense the information field is 32 bits, to we need
> to truncate the information field to avoid clobbering the
> sense code.
>
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/scsi_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_common.c b/drivers/scsi/scsi_common.c
> index c126966..3459009 100644
> --- a/drivers/scsi/scsi_common.c
> +++ b/drivers/scsi/scsi_common.c
> @@ -279,7 +279,7 @@ int scsi_set_sense_information(u8 *buf, int buf_len, u64
> info)
> put_unaligned_be64(info, &ucp[4]);
> } else if ((buf[0] & 0x7f) == 0x70) {
> buf[0] |= 0x80;
> - put_unaligned_be64(info, &buf[3]);
> + put_unaligned_be32((u32)info, &buf[3]);
> }
>
> return 0;
Well, not clobbering the ADDITIONAL SENSE LENGTH field is good,
however according to SPC-5 what we are really supposed to do here
is not set the VALID bit (buf[0] |= 0x80) if the value to be
returned can't be represented in 32 bits (and set the INFORMATION
field to a "vendor specific" value, whatever that means).
I'm not a T10 member so I don't have immediate access to what the
earlier SPC revisions say.
Reviewed-by: Ewan D. Milne
--
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] qla2xxx: avoid maybe_uninitialized warning
On Wed, 2016-03-16 at 16:03 +0100, Tomas Henzl wrote:
> On 15.3.2016 22:40, Arnd Bergmann wrote:
> > The qlt_check_reserve_free_req() function produces an incorrect warning
> > when CONFIG_PROFILE_ANNOTATED_BRANCHES is set:
> >
> > drivers/scsi/qla2xxx/qla_target.c: In function 'qlt_check_reserve_free_req':
> > drivers/scsi/qla2xxx/qla_target.c:1887:3: error: 'cnt_in' may be used
> > uninitialized in this function [-Werror=maybe-uninitialized]
> >ql_dbg(ql_dbg_io, vha, 0x305a,
> >^~
> >"qla_target(%d): There is no room in the request ring:
> > vha->req->ring_index=%d, vha->req->cnt=%d, req_cnt=%d Req-out=%d Req-in=%d
> > Req-Length=%d\n",
> >
> > ~~~
> >vha->vp_idx, vha->req->ring_index,
> >~~
> >vha->req->cnt, req_cnt, cnt, cnt_in, vha->req->length);
> >~~
> > drivers/scsi/qla2xxx/qla_target.c:1887:3: error: 'cnt' may be used
> > uninitialized in this function [-Werror=maybe-uninitialized]
> >
> > The problem is that gcc fails to track the state of the condition across
> > an annotated branch.
> >
> > This slightly rearranges the code to move the second if() block
> > into the first one, to avoid the warning while retaining the
> > behavior of the code.
>
> When the first 'if' is true the vha->req->ring_index gets a new value
> assigned - so it could be possible that the second 'if' wont be true any more.
> The code should not be merged into that single 'if', or am I missing
> something?
>
> tomash
If the first "if" is false, the second "if" will be false also, because
the vha->req->cnt value has not changed. If the first "if" is true, the
nested second "if" will retest the condition.
The compiler is not at fault, because vha->req->cnt can't be tracked as
it could be modified by another thread/process. It isn't, it's protected
by the ->hardware_lock, but the compiler doesn't know that.
-Ewan
> >
> > Signed-off-by: Arnd Bergmann
> > ---
> > drivers/scsi/qla2xxx/qla_target.c | 16 +---
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/scsi/qla2xxx/qla_target.c
> > b/drivers/scsi/qla2xxx/qla_target.c
> > index 985231900aca..8a44d1541eb4 100644
> > --- a/drivers/scsi/qla2xxx/qla_target.c
> > +++ b/drivers/scsi/qla2xxx/qla_target.c
> > @@ -1881,15 +1881,17 @@ static int qlt_check_reserve_free_req(struct
> > scsi_qla_host *vha,
> > else
> > vha->req->cnt = vha->req->length -
> > (vha->req->ring_index - cnt);
> > - }
> >
> > - if (unlikely(vha->req->cnt < (req_cnt + 2))) {
> > - ql_dbg(ql_dbg_io, vha, 0x305a,
> > - "qla_target(%d): There is no room in the request ring:
> > vha->req->ring_index=%d, vha->req->cnt=%d, req_cnt=%d Req-out=%d Req-in=%d
> > Req-Length=%d\n",
> > - vha->vp_idx, vha->req->ring_index,
> > - vha->req->cnt, req_cnt, cnt, cnt_in, vha->req->length);
> > - return -EAGAIN;
> > + if (unlikely(vha->req->cnt < (req_cnt + 2))) {
> > + ql_dbg(ql_dbg_io, vha, 0x305a,
> > + "qla_target(%d): There is no room in the request
> > ring: vha->req->ring_index=%d, vha->req->cnt=%d, req_cnt=%d Req-out=%d
> > Req-in=%d Req-Length=%d\n",
> > + vha->vp_idx, vha->req->ring_index,
> > + vha->req->cnt, req_cnt, cnt, cnt_in,
> > + vha->req->length);
> > + return -EAGAIN;
> > + }
> > }
> > +
> > vha->req->cnt -= req_cnt;
> >
> > return 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: [PATCH 1/5] Fix to cleanup aborted IO to avoid device being offlined by mid-layer
On Wed, 2016-03-16 at 22:38 +, Satish Kharat (satishkh) wrote:
> Thanks for the review comments. I incorporated the comments, below is the
> updated patch:
>
>
> If an I/O times out and an abort issued by host, if the abort is
> successful we need to set scsi status as DID_ABORT. Or else the
> mid-layer error handler which looks for this error code, will
> offline the device. Also if the original I/O is not found in fnic
> firmware, we will consider the abort as successful.
> The start_time assignment is moved because of the new goto.
> Fnic driver version changed from 1.6.0.17a to 1.6.0.19,
> version 1.6.0.18 has been skipped
>
> Signed-off-by: Satish Kharat
> ---
> drivers/scsi/fnic/fnic.h | 2 +-
> drivers/scsi/fnic/fnic_scsi.c | 35 +--
> 2 files changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> index ce129e5..52a53f8 100644
> --- a/drivers/scsi/fnic/fnic.h
> +++ b/drivers/scsi/fnic/fnic.h
> @@ -39,7 +39,7 @@
>
> #define DRV_NAME "fnic"
> #define DRV_DESCRIPTION "Cisco FCoE HBA Driver"
> -#define DRV_VERSION "1.6.0.17a"
> +#define DRV_VERSION "1.6.0.19"
> #define PFX DRV_NAME ": "
> #define DFX DRV_NAME "%d: "
>
> diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
> index 266b909..b732fa3 100644
> --- a/drivers/scsi/fnic/fnic_scsi.c
> +++ b/drivers/scsi/fnic/fnic_scsi.c
> @@ -1092,6 +1092,11 @@ static void fnic_fcpio_itmf_cmpl_handler(struct fnic
> *fnic,
> atomic64_inc(
> &term_stats->terminate_fw_timeouts);
> break;
> + case FCPIO_ITMF_REJECTED:
> + FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host,
> + "abort reject recd. id %d\n",
> + (int)(id & FNIC_TAG_MASK));
> + break;
> case FCPIO_IO_NOT_FOUND:
> if (CMD_FLAGS(sc) & FNIC_IO_ABTS_ISSUED)
> atomic64_inc(&abts_stats->abort_io_not_found);
> @@ -1112,9 +1117,15 @@ static void fnic_fcpio_itmf_cmpl_handler(struct fnic
> *fnic,
> spin_unlock_irqrestore(io_lock, flags);
> return;
> }
> - CMD_ABTS_STATUS(sc) = hdr_status;
> +
> CMD_FLAGS(sc) |= FNIC_IO_ABT_TERM_DONE;
>
> + /* If the status is IO not found consider it as success */
> + if (hdr_status == FCPIO_IO_NOT_FOUND)
> + CMD_ABTS_STATUS(sc) = FCPIO_SUCCESS;
> + else
> + CMD_ABTS_STATUS(sc) = hdr_status;
> +
> atomic64_dec(&fnic_stats->io_stats.active_ios);
> if (atomic64_read(&fnic->io_cmpl_skip))
> atomic64_dec(&fnic->io_cmpl_skip);
> @@ -1927,21 +1938,33 @@ int fnic_abort_cmd(struct scsi_cmnd *sc)
>
> CMD_STATE(sc) = FNIC_IOREQ_ABTS_COMPLETE;
>
> + start_time = io_req->start_time;
> /*
>* firmware completed the abort, check the status,
> - * free the io_req irrespective of failure or success
> + * free the io_req if successful. If abort fails,
> + * Device reset will clean the I/O.
>*/
> - if (CMD_ABTS_STATUS(sc) != FCPIO_SUCCESS)
> + if (CMD_ABTS_STATUS(sc) == FCPIO_SUCCESS) {
> + CMD_SP(sc) = NULL;
> + }
> + else
> + {
> ret = FAILED;
> -
> - CMD_SP(sc) = NULL;
> + spin_unlock_irqrestore(io_lock, flags);
> + goto fnic_abort_cmd_end;
> + }
>
> spin_unlock_irqrestore(io_lock, flags);
>
> - start_time = io_req->start_time;
> fnic_release_ioreq_buf(fnic, io_req, sc);
> mempool_free(io_req, fnic->io_req_pool);
>
> + if (sc->scsi_done) {
> + /* Call SCSI completion function to complete the IO */
> + sc->result = (DID_ABORT << 16);
> + sc->scsi_done(sc);
> + }
> +
> fnic_abort_cmd_end:
> FNIC_TRACE(fnic_abort_cmd, sc->device->host->host_no,
> sc->request->tag, sc,
> --
> 2.4.3
Reviewed-by: Ewan D. Milne
>
>
> From: Ewan Milne
> Sent: Monday, March 14, 2016 12:59 PM
> To: Satish Kharat (satishkh)
> Cc: linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 1/5] Fix to cl
Re: [PATCH] scsi: fc: use get/put_unaligned64 for wwn access
On Wed, 2016-03-16 at 17:39 +0100, Arnd Bergmann wrote:
> A bug in the gcc-6.0 prerelease version caused at least one
> driver (lpfc) to have excessive stack usage when dealing with
> wwn data, on the ARM architecture.
>
> lpfc_scsi.c: In function 'lpfc_find_next_oas_lun':
> lpfc_scsi.c:117:1: warning: the frame size of 1152 bytes is larger than 1024
> bytes [-Wframe-larger-than=]
>
> I have reported this as a gcc regression in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70232
>
> However, using a better implementation of wwn_to_u64() not only
> helps with the particular gcc problem but also leads to better
> object code for any version or architecture.
>
> The kernel already provides get_unaligned_be64() and
> put_unaligned_be64() helper functions that provide an
> optimized implementation with the desired semantics.
>
> The lpfc_find_next_oas_lun() function in the example that
> grew from 1146 bytes to 5144 bytes when moving from gcc-5.3
> to gcc-6.0 is now 804 bytes, as the optimized
> get_unaligned_be64() load can be done in three instructions.
> The stack usage is now down to 28 bytes from 128 bytes with
> gcc-5.3 before.
>
> Signed-off-by: Arnd Bergmann
> ---
> include/scsi/scsi_transport_fc.h | 15 +++
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/include/scsi/scsi_transport_fc.h
> b/include/scsi/scsi_transport_fc.h
> index 784bc2c0929f..bf66ea6bed2b 100644
> --- a/include/scsi/scsi_transport_fc.h
> +++ b/include/scsi/scsi_transport_fc.h
> @@ -28,6 +28,7 @@
> #define SCSI_TRANSPORT_FC_H
>
> #include
> +#include
> #include
> #include
>
> @@ -797,22 +798,12 @@ fc_remote_port_chkready(struct fc_rport *rport)
>
> static inline u64 wwn_to_u64(u8 *wwn)
> {
> - return (u64)wwn[0] << 56 | (u64)wwn[1] << 48 |
> - (u64)wwn[2] << 40 | (u64)wwn[3] << 32 |
> - (u64)wwn[4] << 24 | (u64)wwn[5] << 16 |
> - (u64)wwn[6] << 8 | (u64)wwn[7];
> + return get_unaligned_be64(wwn);
> }
>
> static inline void u64_to_wwn(u64 inm, u8 *wwn)
> {
> - wwn[0] = (inm >> 56) & 0xff;
> - wwn[1] = (inm >> 48) & 0xff;
> - wwn[2] = (inm >> 40) & 0xff;
> - wwn[3] = (inm >> 32) & 0xff;
> - wwn[4] = (inm >> 24) & 0xff;
> - wwn[5] = (inm >> 16) & 0xff;
> - wwn[6] = (inm >> 8) & 0xff;
> - wwn[7] = inm & 0xff;
> + put_unaligned_be64(inm, wwn);
> }
>
> /**
It would be nice to get rid of these functions completely and just
change the callers to use get/put_unaligned_be64() directly, like libfc
does, but that involves changing 7 drivers and scsi_transport_fc.
Reviewed-by: Ewan D. Milne
--
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] sd: fixup capacity calculation for 4k drives
On Tue, 2016-03-22 at 08:14 +0100, Hannes Reinecke wrote:
> On 03/22/2016 02:16 AM, Martin K. Petersen wrote:
> >> "Hannes" == Hannes Reinecke writes:
> >
> > Hannes> in sd_read_capacity() the sdkp->capacity field changes its
> > Hannes> meaning: after the call to read_capacity_XX() it carries the
> > Hannes> _unscaled_ values, making the comparison between the original
> > Hannes> value and the new value always false for drives with a sector
> > Hannes> size != 512. So introduce a 'new_capacity' carrying the new,
> > Hannes> scaled, capacity.
> >
> > I agree with Christoph.
> >
> > How about something like this instead?
> >
> I've coded it somewhat different, but this one works as well.
> But please modify the description in sd.h, as with this patch
> 'sdkp->capacity' is the _unscaled_ value.
> Might lead to confusion otherwise.
>
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 5f2a84a..5ed7434 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -65,7 +65,7 @@ struct scsi_disk {
> struct device dev;
> struct gendisk *disk;
> atomic_topeners;
> - sector_tcapacity; /* size in 512-byte sectors */
> + sector_tcapacity; /* size in logical sectors */
> u32 max_xfer_blocks;
> u32 opt_xfer_blocks;
> u32 max_ws_blocks;
>
> (Apologies for the mangled patch)
>
> Cheers,
>
> Hannes
If we change the meaning of the scsi_disk->capacity field to be logical
sectors, we also have to change sd_getgeo() in sd.c, do we not?
The ->capacity field is also accessed by last_sector_hacks() in
drivers/usb/storage/transport.c, although it kind of looks like
that code might not have been correct for 4K sectors with the
scaled value.
-Ewan
--
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/3] scsi-trace: Decode MAINTENANCE_IN and MAINTENANCE_OUT commands
On Thu, 2016-03-24 at 11:23 +0100, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/scsi_trace.c | 96
> +++
> 1 file changed, 96 insertions(+)
>
> diff --git a/drivers/scsi/scsi_trace.c b/drivers/scsi/scsi_trace.c
> index 08bb47b..b50cfe8 100644
> --- a/drivers/scsi/scsi_trace.c
> +++ b/drivers/scsi/scsi_trace.c
> @@ -231,6 +231,98 @@ out:
> }
>
> static const char *
> +scsi_trace_maintenance_in(struct trace_seq *p, unsigned char *cdb, int len)
> +{
> + const char *ret = trace_seq_buffer_ptr(p), *cmd;
> + u32 alloc_len = 0;
> +
> + switch (SERVICE_ACTION16(cdb)) {
> + case MI_REPORT_IDENTIFYING_INFORMATION:
> + cmd = "REPORT_IDENTIFYING_INFORMATION";
> + break;
> + case MI_REPORT_TARGET_PGS:
> + cmd = "REPORT_TARGET_PORT_GROUPS";
> + break;
> + case MI_REPORT_ALIASES:
> + cmd = "REPORT_ALIASES";
> + break;
> + case MI_REPORT_SUPPORTED_OPERATION_CODES:
> + cmd = "REPORT_SUPPORTED_OPERATION_CODES";
> + break;
> + case MI_REPORT_SUPPORTED_TASK_MANAGEMENT_FUNCTIONS:
> + cmd = "REPORT_SUPPORTED_TASK_MANAGEMENT_FUNCTIONS";
> + break;
> + case MI_REPORT_PRIORITY:
> + cmd = "REPORT_PRIORITY";
> + break;
> + case MI_REPORT_TIMESTAMP:
> + cmd = "REPORT_TIMESTAMP";
> + break;
> + case MI_MANAGEMENT_PROTOCOL_IN:
> + cmd = "MANAGEMENT_PROTOCOL_IN";
> + break;
> + default:
> + trace_seq_puts(p, "UNKNOWN");
> + goto out;
> + }
> +
> + alloc_len |= (cdb[6] << 24);
> + alloc_len |= (cdb[7] << 16);
> + alloc_len |= (cdb[8] << 8);
> + alloc_len |= cdb[9];
You could use get_unaligned_be32() here instead, as well as in
scsi_trace_maintenance_out() and similar get_unaligned_XX calls
in the other functions in scsi_trace.c
> +
> + trace_seq_printf(p, "%s alloc_len=%u", cmd, alloc_len);
> +
> +out:
> + trace_seq_putc(p, 0);
> +
> + return ret;
> +}
> +
> +static const char *
> +scsi_trace_maintenance_out(struct trace_seq *p, unsigned char *cdb, int len)
> +{
> + const char *ret = trace_seq_buffer_ptr(p), *cmd;
> + u32 alloc_len = 0;
> +
> + switch (SERVICE_ACTION16(cdb)) {
> + case MO_SET_IDENTIFYING_INFORMATION:
> + cmd = "SET_IDENTIFYING_INFORMATION";
> + break;
> + case MO_SET_TARGET_PGS:
> + cmd = "SET_TARGET_PORT_GROUPS";
> + break;
> + case MO_CHANGE_ALIASES:
> + cmd = "CHANGE_ALIASES";
> + break;
> + case MO_SET_PRIORITY:
> + cmd = "SET_PRIORITY";
> + break;
> + case MO_SET_TIMESTAMP:
> + cmd = "SET_TIMESTAMP";
> + break;
> + case MO_MANAGEMENT_PROTOCOL_OUT:
> + cmd = "MANAGEMENT_PROTOCOL_OUT";
> + break;
> + default:
> + trace_seq_puts(p, "UNKNOWN");
> + goto out;
> + }
> +
> + alloc_len |= (cdb[6] << 24);
> + alloc_len |= (cdb[7] << 16);
> + alloc_len |= (cdb[8] << 8);
> + alloc_len |= cdb[9];
> +
> + trace_seq_printf(p, "%s alloc_len=%u", cmd, alloc_len);
> +
> +out:
> + trace_seq_putc(p, 0);
> +
> + return ret;
> +}
> +
> +static const char *
> scsi_trace_varlen(struct trace_seq *p, unsigned char *cdb, int len)
> {
> switch (SERVICE_ACTION32(cdb)) {
> @@ -282,6 +374,10 @@ scsi_trace_parse_cdb(struct trace_seq *p, unsigned char
> *cdb, int len)
> return scsi_trace_service_action_in(p, cdb, len);
> case VARIABLE_LENGTH_CMD:
> return scsi_trace_varlen(p, cdb, len);
> + case MAINTENANCE_IN:
> + return scsi_trace_maintenance_in(p, cdb, len);
> + case MAINTENANCE_OUT:
> + return scsi_trace_maintenance_out(p, cdb, len);
> default:
> return scsi_trace_misc(p, cdb, len);
> }
Reviewed-by: Ewan D. Milne
--
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-trace: remove service action definitions
On Thu, 2016-03-24 at 11:23 +0100, Hannes Reinecke wrote: > scsi_opcode_name() is displaying the opcode, not the service > action. > > Signed-off-by: Hannes Reinecke > --- > include/trace/events/scsi.h | 4 > 1 file changed, 4 deletions(-) > > diff --git a/include/trace/events/scsi.h b/include/trace/events/scsi.h > index 079bd10..5c0d91f 100644 > --- a/include/trace/events/scsi.h > +++ b/include/trace/events/scsi.h > @@ -95,10 +95,6 @@ > scsi_opcode_name(VERIFY_16),\ > scsi_opcode_name(WRITE_SAME_16),\ > scsi_opcode_name(SERVICE_ACTION_IN_16), \ > - scsi_opcode_name(SAI_READ_CAPACITY_16), \ > - scsi_opcode_name(SAI_GET_LBA_STATUS), \ > - scsi_opcode_name(MI_REPORT_TARGET_PGS), \ > - scsi_opcode_name(MO_SET_TARGET_PGS),\ > scsi_opcode_name(READ_32), \ > scsi_opcode_name(WRITE_32), \ > scsi_opcode_name(WRITE_SAME_32),\ Reviewed-by: Ewan D. Milne -- 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 3/3] scsi-trace: define ZBC_IN and ZBC_OUT
0x00 > +/* values for ZBC_OUT */ > +#define ZO_CLOSE_ZONE 0x01 > +#define ZO_OPEN_ZONE 0x02 > +#define ZO_FINISH_ZONE 0x03 > +#define ZO_RESET_WRITE_POINTER 0x04 > /* values for variable length command */ > #define XDREAD_32 0x03 > #define XDWRITE_32 0x04 > diff --git a/include/trace/events/scsi.h b/include/trace/events/scsi.h > index 5c0d91f..9a9b3e2 100644 > --- a/include/trace/events/scsi.h > +++ b/include/trace/events/scsi.h > @@ -94,6 +94,8 @@ > scsi_opcode_name(WRITE_16), \ > scsi_opcode_name(VERIFY_16),\ > scsi_opcode_name(WRITE_SAME_16),\ > + scsi_opcode_name(ZBC_OUT), \ > + scsi_opcode_name(ZBC_IN), \ > scsi_opcode_name(SERVICE_ACTION_IN_16), \ > scsi_opcode_name(READ_32), \ > scsi_opcode_name(WRITE_32), \ Reviewed-by: Ewan D. Milne -- 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: Add intermediate STARGET_REMOVE state to scsi_target_state
On Thu, 2016-03-24 at 10:56 +0100, Johannes Thumshirn wrote:
> The target state machine only knows 'STARGET_DEL', which is set once
> scsi_target_destroy() is called.
> However, by that time the structure is still part of the __stargets
> list of the SCSI host, so any concurrent invocation will see this as
> a valid target, causing an access to freed memory.
>
> This patch adds an intermediate state 'STARGET_REMOVE', which is set
> as soon as the target is scheduled to be removed.
> With this any concurrent invocation will be able to skip these
> targets and avoid the above scenario.
>
> Signed-off-by: Johannes Thumshirn
> Fixes: 90a88d6ef 'scsi: fix soft lockup in scsi_remove_target() on module
> removal'
> Cc: sta...@vger.kernel.org
> Reviewed-by: Hannes Reinecke
> ---
> drivers/scsi/scsi_scan.c | 1 +
> drivers/scsi/scsi_sysfs.c | 2 ++
> include/scsi/scsi_device.h | 1 +
> 3 files changed, 4 insertions(+)
>
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 6a82066..37459dfa 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -315,6 +315,7 @@ static void scsi_target_destroy(struct scsi_target
> *starget)
> struct Scsi_Host *shost = dev_to_shost(dev->parent);
> unsigned long flags;
>
> + BUG_ON(starget->state != STARGET_REMOVE);
> starget->state = STARGET_DEL;
> transport_destroy_device(dev);
> spin_lock_irqsave(shost->host_lock, flags);
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 00bc721..0df82e8 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1279,11 +1279,13 @@ restart:
> spin_lock_irqsave(shost->host_lock, flags);
> list_for_each_entry(starget, &shost->__targets, siblings) {
> if (starget->state == STARGET_DEL ||
> + starget->state == STARGET_REMOVE ||
> starget == last_target)
> continue;
> if (starget->dev.parent == dev || &starget->dev == dev) {
> kref_get(&starget->reap_ref);
> last_target = starget;
> + starget->state = STARGET_REMOVE;
> spin_unlock_irqrestore(shost->host_lock, flags);
> __scsi_remove_target(starget);
> scsi_target_reap(starget);
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index f63a167..2bffaa6 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -240,6 +240,7 @@ scmd_printk(const char *, const struct scsi_cmnd *, const
> char *, ...);
> enum scsi_target_state {
> STARGET_CREATED = 1,
> STARGET_RUNNING,
> + STARGET_REMOVE,
> STARGET_DEL,
> };
>
This looks fine. Do we still need 90a88d6ef (scsi: fix soft lockup in
scsi_remove_target() on module removal) or can that be reverted now,
since the STARGET_REMOVE state will allow the iteration to continue?
Reviewed-by: Ewan D. Milne
--
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 v1 2/3] Cleanup the I/O pending with fw and has timed out and is used to issue LUN reset
_tag(fnic, sc);
> if (unlikely(tag == SCSI_NO_TAG))
> goto fnic_device_reset_end;
> tag_gen_flag = 1;
> + new_sc=1;
> }
> io_lock = fnic_io_lock_hash(fnic, sc);
> spin_lock_irqsave(io_lock, flags);
> @@ -2453,7 +2471,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
>* the lun reset cmd. If all cmds get cleaned, the lun reset
>* succeeds
>*/
> - if (fnic_clean_pending_aborts(fnic, sc)) {
> + if (fnic_clean_pending_aborts(fnic, sc, new_sc)) {
> spin_lock_irqsave(io_lock, flags);
> io_req = (struct fnic_io_req *)CMD_SP(sc);
> FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
For v1:
Reviewed-by: Ewan D. Milne
--
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 v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer
On Fri, 2016-03-18 at 11:22 -0700, Satish Kharat wrote:
> If an I/O times out and an abort issued by host, if the abort is
> successful we need to set scsi status as DID_ABORT. Or else the
> mid-layer error handler which looks for this error code, will
> offline the device. Also if the original I/O is not found in fnic
> firmware, we will consider the abort as successful.
> The start_time assignment is moved because of the new goto.
> Fnic driver version changed from 1.6.0.17a to 1.6.0.19,
> version 1.6.0.18 has been skipped
>
> Signed-off-by: Satish Kharat
> Signed-off-by: Sesidhar Baddela
> Reviewed-by: Ewan D. Milne
> ---
> * v1
> - Moved CMD_ABTS_STATUS assignment to else when not FCPIO_SUCCESS
>
> drivers/scsi/fnic/fnic.h | 2 +-
> drivers/scsi/fnic/fnic_scsi.c | 35 +--
> 2 files changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> index ce129e5..52a53f8 100644
> --- a/drivers/scsi/fnic/fnic.h
> +++ b/drivers/scsi/fnic/fnic.h
> @@ -39,7 +39,7 @@
>
> #define DRV_NAME "fnic"
> #define DRV_DESCRIPTION "Cisco FCoE HBA Driver"
> -#define DRV_VERSION "1.6.0.17a"
> +#define DRV_VERSION "1.6.0.19"
> #define PFX DRV_NAME ": "
> #define DFX DRV_NAME "%d: "
>
> diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
> index 266b909..b732fa3 100644
> --- a/drivers/scsi/fnic/fnic_scsi.c
> +++ b/drivers/scsi/fnic/fnic_scsi.c
> @@ -1092,6 +1092,11 @@ static void fnic_fcpio_itmf_cmpl_handler(struct fnic
> *fnic,
> atomic64_inc(
> &term_stats->terminate_fw_timeouts);
> break;
> + case FCPIO_ITMF_REJECTED:
> + FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host,
> + "abort reject recd. id %d\n",
> + (int)(id & FNIC_TAG_MASK));
> + break;
> case FCPIO_IO_NOT_FOUND:
> if (CMD_FLAGS(sc) & FNIC_IO_ABTS_ISSUED)
> atomic64_inc(&abts_stats->abort_io_not_found);
> @@ -1112,9 +1117,15 @@ static void fnic_fcpio_itmf_cmpl_handler(struct fnic
> *fnic,
> spin_unlock_irqrestore(io_lock, flags);
> return;
> }
> - CMD_ABTS_STATUS(sc) = hdr_status;
> +
> CMD_FLAGS(sc) |= FNIC_IO_ABT_TERM_DONE;
>
> + /* If the status is IO not found consider it as success */
> + if (hdr_status == FCPIO_IO_NOT_FOUND)
> + CMD_ABTS_STATUS(sc) = FCPIO_SUCCESS;
> + else
> + CMD_ABTS_STATUS(sc) = hdr_status;
> +
> atomic64_dec(&fnic_stats->io_stats.active_ios);
> if (atomic64_read(&fnic->io_cmpl_skip))
> atomic64_dec(&fnic->io_cmpl_skip);
> @@ -1927,21 +1938,33 @@ int fnic_abort_cmd(struct scsi_cmnd *sc)
>
> CMD_STATE(sc) = FNIC_IOREQ_ABTS_COMPLETE;
>
> + start_time = io_req->start_time;
> /*
>* firmware completed the abort, check the status,
> - * free the io_req irrespective of failure or success
> + * free the io_req if successful. If abort fails,
> + * Device reset will clean the I/O.
>*/
> - if (CMD_ABTS_STATUS(sc) != FCPIO_SUCCESS)
> + if (CMD_ABTS_STATUS(sc) == FCPIO_SUCCESS) {
> + CMD_SP(sc) = NULL;
> + }
> + else
> + {
> ret = FAILED;
> -
> - CMD_SP(sc) = NULL;
> + spin_unlock_irqrestore(io_lock, flags);
> + goto fnic_abort_cmd_end;
> + }
>
> spin_unlock_irqrestore(io_lock, flags);
>
> - start_time = io_req->start_time;
> fnic_release_ioreq_buf(fnic, io_req, sc);
> mempool_free(io_req, fnic->io_req_pool);
>
> + if (sc->scsi_done) {
> + /* Call SCSI completion function to complete the IO */
> + sc->result = (DID_ABORT << 16);
> + sc->scsi_done(sc);
> + }
> +
> fnic_abort_cmd_end:
> FNIC_TRACE(fnic_abort_cmd, sc->device->host->host_no,
> sc->request->tag, sc,
For v1:
Reviewed-by: Ewan D. Milne
--
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 v1 3/3] Using rport->dd_data to check rport online instead of rport_lookup.
On Fri, 2016-03-18 at 11:22 -0700, Satish Kharat wrote:
> When issuing I/O we check if rport is online through libfc
> rport_lookup() function which needs to be protected by mutex lock
> that cannot acquired in I/O context. The change is to use midlayer
> remote port’s dd_data which is preserved until its devloss timeout
> and no protection is required.
> The the scsi_cmnd error code is expected to be in the left 16 bits
> of the result field. Changed to correct this.
> Fnic driver version changed from 1.6.0.20 to 1.6.0.21
>
> Signed-off-by: Satish Kharat
> Signed-off-by: Sesidhar Baddela
> ---
> * v1
> - DID_NO_CONNECT left shifted 16 bits before assigning to sc->result
> ---
> drivers/scsi/fnic/fnic.h | 2 +-
> drivers/scsi/fnic/fnic_scsi.c | 20 +++-
> 2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> index 1023eae..9ddc920 100644
> --- a/drivers/scsi/fnic/fnic.h
> +++ b/drivers/scsi/fnic/fnic.h
> @@ -39,7 +39,7 @@
>
> #define DRV_NAME "fnic"
> #define DRV_DESCRIPTION "Cisco FCoE HBA Driver"
> -#define DRV_VERSION "1.6.0.20"
> +#define DRV_VERSION "1.6.0.21"
> #define PFX DRV_NAME ": "
> #define DFX DRV_NAME "%d: "
>
> diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
> index 026b93d..0e874a5 100644
> --- a/drivers/scsi/fnic/fnic_scsi.c
> +++ b/drivers/scsi/fnic/fnic_scsi.c
> @@ -439,7 +439,6 @@ static int fnic_queuecommand_lck(struct scsi_cmnd *sc,
> void (*done)(struct scsi_
> int sg_count = 0;
> unsigned long flags = 0;
> unsigned long ptr;
> - struct fc_rport_priv *rdata;
> spinlock_t *io_lock = NULL;
> int io_lock_acquired = 0;
>
> @@ -455,14 +454,17 @@ static int fnic_queuecommand_lck(struct scsi_cmnd *sc,
> void (*done)(struct scsi_
> return 0;
> }
>
> - rdata = lp->tt.rport_lookup(lp, rport->port_id);
> - if (!rdata || (rdata->rp_state == RPORT_ST_DELETE)) {
> - FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
> - "returning IO as rport is removed\n");
> - atomic64_inc(&fnic_stats->misc_stats.rport_not_ready);
> - sc->result = DID_NO_CONNECT;
> - done(sc);
> - return 0;
> + if (rport) {
> + struct fc_rport_libfc_priv *rp = rport->dd_data;
> +
> + if (!rp || rp->rp_state != RPORT_ST_READY) {
> + FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
> + "returning DID_NO_CONNECT for IO as rport is
> removed\n");
> + atomic64_inc(&fnic_stats->misc_stats.rport_not_ready);
> + sc->result = DID_NO_CONNECT<<16;
> + done(sc);
> + return 0;
> + }
> }
>
> if (lp->state != LPORT_ST_READY || !(lp->link_up))
For v1:
Reviewed-by: Ewan D. Milne
--
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/2] Declare local symbols static
On Mon, 2016-03-28 at 11:13 -0700, Bart Van Assche wrote:
> Avoid that building with W=1 causes gcc to report warnings about
> symbols that have not been declared.
>
> Signed-off-by: Bart Van Assche
> Cc: Hannes Reinecke
> ---
> drivers/scsi/scsi_sysfs.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 92ffd24..2b642b1 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -81,6 +81,7 @@ const char *scsi_host_state_name(enum scsi_host_state state)
> return name;
> }
>
> +#ifdef CONFIG_SCSI_DH
> static const struct {
> unsigned char value;
> char*name;
> @@ -94,7 +95,7 @@ static const struct {
> { SCSI_ACCESS_STATE_TRANSITIONING, "transitioning" },
> };
>
> -const char *scsi_access_state_name(unsigned char state)
> +static const char *scsi_access_state_name(unsigned char state)
> {
> int i;
> char *name = NULL;
> @@ -107,6 +108,7 @@ const char *scsi_access_state_name(unsigned char state)
> }
> return name;
> }
> +#endif
>
> static int check_set(unsigned long long *val, char *src)
> {
> @@ -226,7 +228,7 @@ show_shost_state(struct device *dev, struct
> device_attribute *attr, char *buf)
> }
>
> /* DEVICE_ATTR(state) clashes with dev_attr_state for sdev */
> -struct device_attribute dev_attr_hstate =
> +static struct device_attribute dev_attr_hstate =
> __ATTR(state, S_IRUGO | S_IWUSR, show_shost_state, store_shost_state);
>
> static ssize_t
> @@ -401,7 +403,7 @@ static struct attribute *scsi_sysfs_shost_attrs[] = {
> NULL
> };
>
> -struct attribute_group scsi_shost_attr_group = {
> +static struct attribute_group scsi_shost_attr_group = {
> .attrs =scsi_sysfs_shost_attrs,
> };
>
Could have been 2 patches, since the patch subject and the more
verbose description, along with the code, are really 2 separate
issues, but OK.
Reviewed-by: Ewan D. Milne
--
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/2] scsi_dh_alua: Fix a recently introduced deadlock
On Mon, 2016-03-28 at 11:14 -0700, Bart Van Assche wrote:
> While retesting the SRP initiator I ran the command "rmmod mlx4_ib"
> while I/O was in progress. That command triggers SCSI device removal
> indirectly. Avoid that this action triggers the following deadlock:
>
> =
> [ INFO: inconsistent lock state ]
> 4.6.0-rc0-dbg+ #2 Tainted: G O
> -
> inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> multipathd/484 [HC0[0]:SC0[0]:HE1:SE1] takes:
> (&(&pg->lock)->rlock){+.?...}, at: []
> alua_bus_detach+0x52/0xa0 [scsi_dh_alua]
> {IN-SOFTIRQ-W} state was registered at:
> [] __lock_acquire+0x7e9/0x1ad0
> [] lock_acquire+0x60/0x80
> [] _raw_spin_lock_irqsave+0x3e/0x60
> [] alua_rtpg_queue+0x41/0x1d0 [scsi_dh_alua]
> [] alua_check+0xe1/0x220 [scsi_dh_alua]
> [] alua_check_sense+0x99/0xb0 [scsi_dh_alua]
> [] scsi_check_sense+0x71/0x3f0
> [] scsi_decide_disposition+0x18b/0x1d0
> [] scsi_softirq_done+0x52/0x140
> [] blk_done_softirq+0x52/0x90
> [] __do_softirq+0x10f/0x230
> [] irq_exit+0xa8/0xb0
> [] do_IRQ+0x65/0x110
> [] ret_from_intr+0x0/0x19
> [] kmem_cache_alloc+0x151/0x190
> [] create_object+0x34/0x2d0
> [] kmemleak_alloc_percpu+0x56/0xd0
> [] pcpu_alloc+0x38d/0x660
> [] __alloc_percpu_gfp+0xd/0x10
> [] __percpu_counter_init+0x55/0xb0
> [] blkg_alloc+0x79/0x230
> [] blkcg_init_queue+0x26/0x1d0
> [] blk_alloc_queue_node+0x27d/0x2e0
> [] dm_create+0x20c/0x570 [dm_mod]
> [] dev_create+0x56/0x2c0 [dm_mod]
> [] ctl_ioctl+0x26e/0x520 [dm_mod]
> [] dm_ctl_ioctl+0xe/0x20 [dm_mod]
> [] do_vfs_ioctl+0x8e/0x660
> [] SyS_ioctl+0x3c/0x70
> [] entry_SYSCALL_64_fastpath+0x1c/0xac
> irq event stamp: 4290931
> hardirqs last enabled at (4290931): [ 1662.892772]
> [] _raw_spin_unlock_irqrestore+0x31/0x50
> hardirqs last disabled at (4290930): []
> _raw_spin_lock_irqsave+0x17/0x60
> softirqs last enabled at (4290774): []
> __do_softirq+0x1cb/0x230
> softirqs last disabled at (4289831): [] irq_exit+0xa8/0xb0
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
>CPU0
>
> lock(&(&pg->lock)->rlock);
>
> lock(&(&pg->lock)->rlock);
>
> *** DEADLOCK ***
>
> 2 locks held by multipathd/484:
> #0: (&bdev->bd_mutex){+.+.+.}, at: []
> __blkdev_put+0x33/0x360
> #1: (sd_ref_mutex){+.+...}, at: [] scsi_disk_put+0x1c/0x40
>
> stack backtrace:
> CPU: 6 PID: 484 Comm: multipathd Tainted: G O4.6.0-rc0-dbg+ #2
> Call Trace:
> [] dump_stack+0x67/0x92
> [] print_usage_bug+0x215/0x240
> [] mark_lock+0x54a/0x610
> [] __lock_acquire+0x845/0x1ad0
> [] lock_acquire+0x60/0x80
> [] _raw_spin_lock+0x33/0x50
> [] alua_bus_detach+0x52/0xa0 [scsi_dh_alua]
> [] scsi_dh_release_device+0x17/0x50
> [] scsi_device_dev_release_usercontext+0x2a/0x120
> [] execute_in_process_context+0x80/0x90
> [] scsi_device_dev_release+0x17/0x20
> [] device_release+0x2d/0x90
> [] kobject_release+0x7a/0x190
> [] kobject_put+0x26/0x50
> [] put_device+0x12/0x20
> [] scsi_device_put+0x26/0x30
> [] scsi_disk_put+0x2d/0x40
> [] sd_release+0x48/0xb0
> [] __blkdev_put+0x29e/0x360
> [] blkdev_put+0x49/0x170
> [] blkdev_close+0x20/0x30
> [] __fput+0xe8/0x1f0
> [] fput+0x9/0x10
> [] task_work_run+0x6e/0xa0
> [] exit_to_usermode_loop+0xa9/0xb0
> [] syscall_return_slowpath+0xb0/0xc0
> [] entry_SYSCALL_64_fastpath+0xaa/0xac
>
> Fixes: cb0a168cb6b8 (scsi_dh_alua: update 'access_state' field)
> Signed-off-by: Bart Van Assche
> Cc: Hannes Reinecke
> Signed-off-by: Bart Van Assche
> ---
> drivers/scsi/device_handler/scsi_dh_alua.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index a404a41..8eaed05 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -1112,9 +1112,9 @@ static void alua_bus_detach(struct scsi_device *sdev)
> h->sdev = NULL;
> spin_unlock(&h->pg_lock);
> if (pg) {
> - spin_lock(&pg->lock);
> + spin_lock_irq(&pg->lock);
> list_del_rcu(&h->node);
> - spin_unlock(&pg->lock);
> + spin_unlock_irq(&pg->lock);
> kref_put(&pg->kref, release_port_group);
> }
> sdev->handler_data = NULL;
Reviewed-by: Ewan D. Milne
--
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: [RESEND PATCH v2 1/2] Revert "scsi: fix soft lockup in scsi_remove_target() on module removal"
On Wed, 2016-03-30 at 09:09 +0200, Johannes Thumshirn wrote:
> This reverts commit 90a88d6ef88edcfc4f644dddc7eef4ea41bccf8b.
>
> Signed-off-by: Johannes Thumshirn
> Cc: sta...@vger.kernel.org
> ---
> drivers/scsi/scsi_sysfs.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 00bc721..4f18a85 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1272,18 +1272,16 @@ static void __scsi_remove_target(struct scsi_target
> *starget)
> void scsi_remove_target(struct device *dev)
> {
> struct Scsi_Host *shost = dev_to_shost(dev->parent);
> - struct scsi_target *starget, *last_target = NULL;
> + struct scsi_target *starget;
> unsigned long flags;
>
> restart:
> spin_lock_irqsave(shost->host_lock, flags);
> list_for_each_entry(starget, &shost->__targets, siblings) {
> - if (starget->state == STARGET_DEL ||
> - starget == last_target)
> + if (starget->state == STARGET_DEL)
> continue;
> if (starget->dev.parent == dev || &starget->dev == dev) {
> kref_get(&starget->reap_ref);
> - last_target = starget;
> spin_unlock_irqrestore(shost->host_lock, flags);
> __scsi_remove_target(starget);
> scsi_target_reap(starget);
Reviewed-by: Ewan D. Milne
--
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: [RESEND PATCH v2 2/2] scsi: Add intermediate STARGET_REMOVE state to scsi_target_state
On Wed, 2016-03-30 at 09:09 +0200, Johannes Thumshirn wrote:
> Add intermediate STARGET_REMOVE state to scsi_target_state to avoid running
> into the BUG_ON() in scsi_target_reap().
>
> This intermediate state is only valid in the path from scsi_remove_target() to
> scsi_target_destroy() indicating this target is going to be removed.
>
> Signed-off-by: Johannes Thumshirn
> Fixes: 40998193560dab6c3ce8d25f4fa58a23e252ef38
> Cc: sta...@vger.kernel.org
> Reviewed-by: Hannes Reinecke
> Reviewed-by: Ewan D. Milne
> ---
>
> Changes from v1:
> * The state transition from STARGET_CREATED to STARGET_DEL is legitimate,
> so don't BUG() on it. Found by the 0-Day Bot.
>
>
> drivers/scsi/scsi_scan.c | 2 ++
> drivers/scsi/scsi_sysfs.c | 4 +++-
> include/scsi/scsi_device.h | 1 +
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 6a82066..63b8bca 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -315,6 +315,8 @@ static void scsi_target_destroy(struct scsi_target
> *starget)
> struct Scsi_Host *shost = dev_to_shost(dev->parent);
> unsigned long flags;
>
> + BUG_ON(starget->state != STARGET_REMOVE &&
> +starget->state != STARGET_CREATED);
> starget->state = STARGET_DEL;
> transport_destroy_device(dev);
> spin_lock_irqsave(shost->host_lock, flags);
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 4f18a85..9e5f893 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1278,10 +1278,12 @@ void scsi_remove_target(struct device *dev)
> restart:
> spin_lock_irqsave(shost->host_lock, flags);
> list_for_each_entry(starget, &shost->__targets, siblings) {
> - if (starget->state == STARGET_DEL)
> + if (starget->state == STARGET_DEL ||
> + starget->state == STARGET_REMOVE)
> continue;
> if (starget->dev.parent == dev || &starget->dev == dev) {
> kref_get(&starget->reap_ref);
> + starget->state = STARGET_REMOVE;
> spin_unlock_irqrestore(shost->host_lock, flags);
> __scsi_remove_target(starget);
> scsi_target_reap(starget);
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index f63a167..2bffaa6 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -240,6 +240,7 @@ scmd_printk(const char *, const struct scsi_cmnd *, const
> char *, ...);
> enum scsi_target_state {
> STARGET_CREATED = 1,
> STARGET_RUNNING,
> + STARGET_REMOVE,
> STARGET_DEL,
> };
>
Reviewed-by: Ewan D. Milne
--
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 v2 0/2] Update SCSI target removal path
On Wed, 2016-03-30 at 13:01 +0200, jthumshirn wrote: > [+Cc linux-scsi back] > On 2016-03-30 02:59, Martin K. Petersen wrote: > >>>>>> "Ewan" == Ewan D Milne writes: > > > > Ewan> I would probably use an APCON or other physical layer switch to > > Ewan> drop the FC link and test the error recovery/device loss. But we > > Ewan> don't have one. > > > > They go for a couple of hundred bucks on eBay. I had one of these in a > > previous life and it was awesome. > > Though this would work (like any other FC/FCoE switch) I thought more of > a simulated environment. scsi_debug, Qemu, something like that. You might be able to do something with that but it doesn't quite do the same thing in terms of how a HBA/driver will react to a fault. You also need to be sure there is enough entropy in the timing of when the target goes away. (Otherwise, you could just rmmod scsi_debug...) > > I've had a look at scsi_debug but it seems like it's quite some > refactoring needed to get it to a point where one can simulate target > errors. I kinda like the idea of having something in > tools/testing/selftest but it'll probably end up with a FC switch. > > Johannes -- 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 v2] sd: Fix excessive capacity printing on devices with blocks bigger than 512 bytes
On Mon, 2016-03-28 at 21:18 -0400, Martin K. Petersen wrote:
> During revalidate we check whether device capacity has changed before we
> decide whether to output disk information or not.
>
> The check for old capacity failed to take into account that we scaled
> sdkp->capacity based on the reported logical block size. And therefore
> the capacity test would always fail for devices with sectors bigger than
> 512 bytes and we would print several copies of the same discovery
> information.
>
> Avoid scaling sdkp->capacity and instead adjust the value on the fly
> when setting the block device capacity and generating fake C/H/S
> geometry.
>
> Signed-off-by: Martin K. Petersen
> Reported-by: Hannes Reinecke
> ---
> drivers/scsi/sd.c | 28
> drivers/scsi/sd.h | 7 ++-
> 2 files changed, 14 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 5a5457ac9cdb..8401697ff6aa 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1275,18 +1275,19 @@ static int sd_getgeo(struct block_device *bdev,
> struct hd_geometry *geo)
> struct scsi_disk *sdkp = scsi_disk(bdev->bd_disk);
> struct scsi_device *sdp = sdkp->device;
> struct Scsi_Host *host = sdp->host;
> + sector_t capacity = logical_to_sectors(sdp, sdkp->capacity);
> int diskinfo[4];
>
> /* default to most commonly used values */
> -diskinfo[0] = 0x40; /* 1 << 6 */
> - diskinfo[1] = 0x20; /* 1 << 5 */
> - diskinfo[2] = sdkp->capacity >> 11;
> -
> + diskinfo[0] = 0x40; /* 1 << 6 */
> + diskinfo[1] = 0x20; /* 1 << 5 */
> + diskinfo[2] = capacity >> 11;
> +
> /* override with calculated, extended default, or driver values */
> if (host->hostt->bios_param)
> - host->hostt->bios_param(sdp, bdev, sdkp->capacity, diskinfo);
> + host->hostt->bios_param(sdp, bdev, capacity, diskinfo);
> else
> - scsicam_bios_param(bdev, sdkp->capacity, diskinfo);
> + scsicam_bios_param(bdev, capacity, diskinfo);
>
> geo->heads = diskinfo[0];
> geo->sectors = diskinfo[1];
> @@ -2337,14 +2338,6 @@ got_data:
> if (sdkp->capacity > 0x)
> sdp->use_16_for_rw = 1;
>
> - /* Rescale capacity to 512-byte units */
> - if (sector_size == 4096)
> - sdkp->capacity <<= 3;
> - else if (sector_size == 2048)
> - sdkp->capacity <<= 2;
> - else if (sector_size == 1024)
> - sdkp->capacity <<= 1;
> -
> blk_queue_physical_block_size(sdp->request_queue,
> sdkp->physical_block_size);
> sdkp->device->sector_size = sector_size;
> @@ -2812,11 +2805,6 @@ static int sd_try_extended_inquiry(struct scsi_device
> *sdp)
> return 0;
> }
>
> -static inline u32 logical_to_sectors(struct scsi_device *sdev, u32 blocks)
> -{
> - return blocks << (ilog2(sdev->sector_size) - 9);
> -}
> -
> /**
> * sd_revalidate_disk - called the first time a new disk is seen,
> * performs disk spin up, read_capacity, etc.
> @@ -2900,7 +2888,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
> /* Combine with controller limits */
> q->limits.max_sectors = min(rw_max, queue_max_hw_sectors(q));
>
> - set_capacity(disk, sdkp->capacity);
> + set_capacity(disk, logical_to_sectors(sdp, sdkp->capacity));
> sd_config_write_same(sdkp);
> kfree(buffer);
>
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 5f2a84aff29f..654630bb7d0e 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -65,7 +65,7 @@ struct scsi_disk {
> struct device dev;
> struct gendisk *disk;
> atomic_topeners;
> - sector_tcapacity; /* size in 512-byte sectors */
> + sector_tcapacity; /* size in logical blocks */
> u32 max_xfer_blocks;
> u32 opt_xfer_blocks;
> u32 max_ws_blocks;
> @@ -146,6 +146,11 @@ static inline int scsi_medium_access_command(struct
> scsi_cmnd *scmd)
> return 0;
> }
>
> +static inline sector_t logical_to_sectors(struct scsi_device *sdev, sector_t
> blocks)
> +{
> + return blocks << (ilog2(sdev->sector_size) - 9);
> +}
> +
> /*
> * A DIF-capable target device can be formatted with different
> * protection schemes. Currently 0 through 3 are defined:
Reviewed-by: Ewan D. Milne
--
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] mpt3sas - remove unused fw_event_work delayed_work
On Fri, 2016-04-01 at 13:56 -0400, Joe Lawrence wrote:
> The driver's fw events are queued up using the the fw_event_work's
> struct work, not its delayed_work member. The latter appears to be
> unused and may provoke CONFIG_DEBUG_OBJECTS_TIMERS "assert_init not
> available" false warnings in _scsih_fw_event_cleanup_queue. Remove it
> and update _scsih_fw_event_cleanup_queue accordingly.
>
> Signed-off-by: Joe Lawrence
> ---
>
> I think this goes all the way back to the introduction of the mpt3sas
> driver. The previous generation mpt2sas driver uses delayed_work, so
> perhaps it was simply copied and pasted into the mpt3sas but never
> updated.
>
> drivers/scsi/mpt3sas/mpt3sas_scsih.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index e0e4920d0fa6..67643602efbc 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -189,7 +189,6 @@ struct fw_event_work {
> struct list_headlist;
> struct work_struct work;
> u8 cancel_pending_work;
> - struct delayed_work delayed_work;
>
> struct MPT3SAS_ADAPTER *ioc;
> u16 device_handle;
> @@ -2804,12 +2803,12 @@ _scsih_fw_event_cleanup_queue(struct MPT3SAS_ADAPTER
> *ioc)
> /*
>* Wait on the fw_event to complete. If this returns 1, then
>* the event was never executed, and we need a put for the
> - * reference the delayed_work had on the fw_event.
> + * reference the work had on the fw_event.
>*
>* If it did execute, we wait for it to finish, and the put will
>* happen from _firmware_event_work()
>*/
> - if (cancel_delayed_work_sync(&fw_event->delayed_work))
> + if (cancel_work_sync(&fw_event->work))
> fw_event_work_put(fw_event);
>
> fw_event_work_put(fw_event);
Hmm... Fixes: 146b16c8 (mpt3sas: Refcount fw_events and fix unsafe list usage)
Since mpt3sas uses ->work instead of _delayed_work this would seem to be
correct, however the deprecated mpt2sas driver had a commit that changed
the firmware event work mechanism to use ->delayed_work instead of ->work:
commit f1c35e6aea579d5bdb6dc02dfa99c67c7c3b3f67
Author: Kashyap, Desai
Date: Tue Mar 9 16:31:43 2010 +0530
[SCSI] mpt2sas: RESCAN Barrier work is added in case of HBA reset.
Add the cancel_pending_work flag from the fw_event_work structure, and then
to
set the flag during host reset, check the flag later from work threads
context and if cancel_pending_work_flag is set ingore those events.
Now Rescan after host reset is changed.
Added special task MPT2SAS_RESCAN_AFTER_HOST_RESET. This task will be queued
at the time of HBA reset. this task is treated as barrier. All work after
MPT2SAS_RESCAN_AFTER_HOST_RESET will be treated as new work and will be
server by callback handle. If host_recovery is going on while running RESCAN
task, it will wait for shos_recovery_done completion which will be called
from HBA reset DONE context.
Signed-off-by: Kashyap Desai
Reviewed-by: Eric Moore
Signed-off-by: James Bottomley
Portions of that patch include:
@@ -125,11 +127,11 @@ struct sense_info {
*/
struct fw_event_work {
struct list_headlist;
- struct work_struct work;
+ u8 cancel_pending_work;
+ struct delayed_work delayed_work;
struct MPT2SAS_ADAPTER *ioc;
u8 VF_ID;
u8 VP_ID;
- u8 host_reset_handling;
u8 ignore;
u16 event;
void*event_data;
and
@@ -2325,8 +2327,9 @@ _scsih_fw_event_add(struct MPT2SAS_ADAPTER *ioc, struct
fw_event_work *fw_event)
spin_lock_irqsave(&ioc->fw_event_lock, flags);
list_add_tail(&fw_event->list, &ioc->fw_event_list);
- INIT_WORK(&fw_event->work, _firmware_event_work);
- queue_work(ioc->firmware_event_thread, &fw_event->work);
+ INIT_DELAYED_WORK(&fw_event->delayed_work, _firmware_event_work);
+ queue_delayed_work(ioc->firmware_event_thread,
+ &fw_event->delayed_work, 0);
spin_unlock_irqrestore(&ioc->fw_event_lock, flags);
}
The delay argument to queue_delayed_work() is zero though.
So, Broadcom, presumably Joe's fix is the correct fix?
-Ewan
--
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] mpt3sas - remove unused fw_event_work delayed_work
On Fri, 2016-04-01 at 15:13 -0400, Joe Lawrence wrote:
> On 04/01/2016 02:51 PM, Ewan D. Milne wrote:
> > On Fri, 2016-04-01 at 13:56 -0400, Joe Lawrence wrote:
> >> @@ -2804,12 +2803,12 @@ _scsih_fw_event_cleanup_queue(struct
> >> MPT3SAS_ADAPTER *ioc)
> >>/*
> >> * Wait on the fw_event to complete. If this returns 1, then
> >> * the event was never executed, and we need a put for the
> >> - * reference the delayed_work had on the fw_event.
> >> + * reference the work had on the fw_event.
> >> *
> >> * If it did execute, we wait for it to finish, and the put will
> >> * happen from _firmware_event_work()
> >> */
> >> - if (cancel_delayed_work_sync(&fw_event->delayed_work))
> >> + if (cancel_work_sync(&fw_event->work))
> >>fw_event_work_put(fw_event);
> >>
> >>fw_event_work_put(fw_event);
> >
> > Hmm... Fixes: 146b16c8 (mpt3sas: Refcount fw_events and fix unsafe list
> > usage)
>
> This could technically go back to f92363d12359 (mpt3sas: add new driver
> supporting 12GB SAS) ... but will probably only apply cleanly to
> _scsih_fw_event_cleanup_queue after 146b16c8 (mpt3sas: Refcount
> fw_events and fix unsafe list usage), so you're right.
>
> > Since mpt3sas uses ->work instead of _delayed_work this would seem to be
> > correct, however the deprecated mpt2sas driver had a commit that changed
> > the firmware event work mechanism to use ->delayed_work instead of ->work:
> >
> > commit f1c35e6aea579d5bdb6dc02dfa99c67c7c3b3f67
> > Author: Kashyap, Desai
> > Date: Tue Mar 9 16:31:43 2010 +0530
>
> Okay, so this is pre-mpt3sas split.
>
> > [SCSI] mpt2sas: RESCAN Barrier work is added in case of HBA reset.
> >
> > Add the cancel_pending_work flag from the fw_event_work structure, and
> > then to
> > set the flag during host reset, check the flag later from work threads
> > context and if cancel_pending_work_flag is set ingore those events.
>
> More unused mpt2 vestiges in the mpt3 version?
>
> % cd drivers/scsi/mpt3sas/
> % grep 'cancel_pending_work' *.{c,h}
> mpt3sas_scsih.c: * @cancel_pending_work: flag set during reset handling
> mpt3sas_scsih.c:u8 cancel_pending_work;
>
> > Now Rescan after host reset is changed.
> > Added special task MPT2SAS_RESCAN_AFTER_HOST_RESET. This task will be
> > queued
> > at the time of HBA reset. this task is treated as barrier. All work
> > after
> > MPT2SAS_RESCAN_AFTER_HOST_RESET will be treated as new work and will be
> > server by callback handle. If host_recovery is going on while running
> > RESCAN
> > task, it will wait for shos_recovery_done completion which will be
> > called
> > from HBA reset DONE context.
>
> FWIW, I don't see anything like this in today's mpt3sas driver.
Well, that's the question. Is there some functionality missing? Were
the changes abandoned/replaced? mpt2sas used delayed_work for something
else, so maybe that's why the firmware event changes initially used it
(albeit with a 0 delay) but it's hard to know...
cancel_delayed_work() will call del_timer() on delayed_work->timer, but
it looks like kzalloc is used to allocate the fw_event_work objects so
perhaps nothing bad will happen. I was wondering, though, because I
have seen dumps of hung systems with requests that should have timed out
but are not on any timer list.
-Ewan
>
> Regards,
>
> -- Joe
--
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: Cant write to max_sectors_kb on 4.5.0 SRP target
The version of RHEL you are using does not have:
commit ca369d51b3e1649be4a72addd6d6a168cfb3f537
Author: Martin K. Petersen
Date: Fri Nov 13 16:46:48 2015 -0500
block/sd: Fix device-imposed transfer length limits
(which will be added during the next update).
In the upstream kernel queue_max_sectors_store() does not
permit you to set a value larger than the device-imposed
limit. This value, stored in q->limits.max_dev_sectors,
is not visible via the block queue sysfs interface.
The code that sets q->limits.max_sectors and q->limits.io_opt
in sd.c does not take the device limit into account, but
the sysfs code to change max_sectors ("max_sectors_kb") does.
So there are a couple of problems here, one is that RHEL
is not clamping to the device limit, and the other one is
that neither RHEL nor upstream kernels take the device limit
into account when setting q->limits.io_opt. This only seems
to be a problem for you because your target is reporting
an optimal I/O size in VPD page B0 that is *smaller* than
the reported maximum I/O size.
The target is clearly reporting inconsistent data, the
question is whether we should change the code to clamp the
optimal I/O size, or whether we should assume the value
the target is reporting is wrong.
So the question is: does the target actually process
requests that are larger than the VPD page B0 reported
maximum size? If so, maybe we should just issue a warning
message rather than reducing the optimal I/O size.
-Ewan
On Fri, 2016-04-08 at 04:31 -0400, Laurence Oberman wrote:
> Hello Martin
>
> Yes, Ewan also noticed that.
>
> This started out as me testing the SRP stack on RHEL 7.2 and baselining
> against upstream.
> We have a customer that requires 4MB I/O.
> I bumped into a number of SRP issues including sg_map failures so started
> reviewing upstream changes to the SRP code and patches.
>
> The RHEL kernel is ignoring this so perhaps we have an issue on our side
> (RHEL kernel) and upstream is behaving as it should.
>
> What is intersting is that I cannot change the max_sectors_kb at all on the
> upstream for the SRP LUNS.
>
> Here is an HP SmartArray LUN
>
> [root@srptest ~]# sg_inq --p 0xb0 /dev/sda
> VPD INQUIRY: page=0xb0
> inquiry: field in cdb illegal (page not supported) Known that its
> not supported
>
> However
>
> /sys/block/sda/queue
>
> [root@srptest queue]# cat max_hw_sectors_kb max_sectors_kb
> 4096
> 1280
> [root@srptest queue]# echo 4096 > max_sectors_kb
> [root@srptest queue]# cat max_hw_sectors_kb max_sectors_kb
> 4096
> 4096
>
> On the SRP LUNS I am unable to change to a lower value than max_sectors_kb
> unless I change it to 128
> So perhaps the size on the array is the issue here as Nicholas said and the
> RHEL kernel has a bug and ignores it.
>
> /sys/block/sdc/queue
>
> [root@srptest queue]# cat max_hw_sectors_kb max_sectors_kb
> 4096
> 1280
>
> [root@srptest queue]# echo 512 > max_sectors_kb
> -bash: echo: write error: Invalid argument
>
> [root@srptest queue]# echo 256 > max_sectors_kb
> -bash: echo: write error: Invalid argument
>
> 128 works
> [root@srptest queue]# echo 128 > max_sectors_kb
>
>
>
>
> Laurence Oberman
> Principal Software Maintenance Engineer
> Red Hat Global Support Services
>
> - Original Message -
> From: "Martin K. Petersen"
> To: "Laurence Oberman"
> Cc: "linux-scsi" , linux-r...@vger.kernel.org
> Sent: Thursday, April 7, 2016 11:00:16 PM
> Subject: Re: Cant write to max_sectors_kb on 4.5.0 SRP target
>
> > "Laurence" == Laurence Oberman writes:
>
> Laurence,
>
> The target is reporting inconsistent values here:
>
> > [root@srptest queue]# sg_inq --p 0xb0 /dev/sdb
> > VPD INQUIRY: Block limits page (SBC)
> > Maximum compare and write length: 1 blocks
> > Optimal transfer length granularity: 256 blocks
> > Maximum transfer length: 256 blocks
> > Optimal transfer length: 768 blocks
>
> OPTIMAL TRANSFER LENGTH GRANULARITY roughly translates to physical block
> size or RAID chunk size. It's the smallest I/O unit that does not
> require read-modify-write. It would typically be either 1 or 8 blocks
> for a drive and maybe 64, 128 or 256 for a RAID5 array. io_min in
> queue_limits.
>
> OPTIMAL TRANSFER LENGTH indicates the stripe width and is a multiple of
> OPTIMAL TRANSFER LENGTH GRANULARITY. io_opt in queue_limits.
>
> MAXIMUM TRANSFER LENGTH indicates the biggest READ/WRITE command the
> device can handle in a single command. In this case 256 blocks so that's
> 128K. max_dev_sectors in queue_limits.
>
> From SBC:
>
> "A MAXIMUM TRANSFER LENGTH field set to a non-zero value indicates the
> maximum transfer length in logical blocks that the device server accepts
> for a single command shown in table 250. If a device server receives one
> of these commands with a transfer size greater than this value, then the
> device server shall terminate the command with CHECK CONDITION status
> [...]"
>
> So those reported values are off.
>
>l
Re: [Lsf] [LSF/MM TOPIC] block-mq issues with FC
On Fri, 2016-04-08 at 08:11 -0700, James Bottomley wrote: > On Fri, 2016-04-08 at 13:29 +0200, Hannes Reinecke wrote: > > Hi all, > > > > I'd like to propose a topic on block-mq issues with FC. > > During my performance testing using block/scsi-mq with FC I've hit > > several issues I'd like to discuss: > > > > - timeout handling: > > Out of necessity the status of any timed out command is undefined. > > So to be absolutely safe HBAs will be using extended timeouts here > > (eg 70secs for lpfc). During that time we _could_ signal I/O timeout > > to the upper layers, but then the tag will be reused, despite the > > HBA still having a reference to it. > > I'd like to discuss how this could be solved best with blk-mq. > > What's wrong with the obvious answer: the tag shouldn't be re-used > until after at least the TMF abort. If we need to escalate that then > it looks like the controller lost the tag and requires a bigger hammer. > > However, when I look at what we do, it seems the running abort handler > is triggered from the block timeout function, so where's the problem? > ... surely mq can't free the tag until that returns, because it might > extend the time. > > James There was some discussion a while back about whether we could decouple the SCSI EH's recovery of the device from using the failed scmds, so that once the disposition of the original I/O was determined (i.e. they had succeeded, failed or timed out & aborted), the scmds could be returned to a higher layer while the EH attempted to recover the device. That way, in a multipath environment, we could submit the I/O on working paths and avoid lengthy delays while we went through all the resets. We still need a successful abort after a timeout, but at least in the above scenario we shouldn't be reusing the tags until the device is recovered, as further I/O should be blocked while EH is running. -Ewan -- 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] libfc: replace 'rp_mutex' with 'rp_lock'
On Mon, 2016-04-25 at 10:01 +0200, Hannes Reinecke wrote:
> We cannot use an embedded mutex in a structure with reference
> counting, as mutex unlock might be delayed, and the waiters
> might then access an already freed memory area.
> So convert it to a spinlock.
>
> For details cf https://lkml.org/lkml/2015/2/11/245
>
> Signed-off-by: Hannes Reinecke
There are comments at the beginning of fc_rport.c in the section
RPORT LOCKING that still refer to the rport mutex. (I assume this is
really the fc_rport_priv mutex since fc_rport does not have a mutex.)
These comments should be updated if the locking is changed. Also
see the comment about the mutex being held near fc_rport_enter_delete().
We may have a reproducible test case for our FCoE issues, I will
see if we can tell if this fixes our problem.
-Ewan
> ---
> drivers/scsi/libfc/fc_rport.c | 90
> +--
> include/scsi/libfc.h | 4 +-
> 2 files changed, 47 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
> index 589ff9a..4520b5a 100644
> --- a/drivers/scsi/libfc/fc_rport.c
> +++ b/drivers/scsi/libfc/fc_rport.c
> @@ -136,7 +136,7 @@ static struct fc_rport_priv *fc_rport_create(struct
> fc_lport *lport,
> rdata->ids.roles = FC_RPORT_ROLE_UNKNOWN;
>
> kref_init(&rdata->kref);
> - mutex_init(&rdata->rp_mutex);
> + spin_lock_init(&rdata->rp_lock);
> rdata->local_port = lport;
> rdata->rp_state = RPORT_ST_INIT;
> rdata->event = RPORT_EV_NONE;
> @@ -251,7 +251,7 @@ static void fc_rport_work(struct work_struct *work)
> struct fc4_prov *prov;
> u8 type;
>
> - mutex_lock(&rdata->rp_mutex);
> + spin_lock(&rdata->rp_lock);
> event = rdata->event;
> rport_ops = rdata->ops;
> rport = rdata->rport;
> @@ -264,7 +264,7 @@ static void fc_rport_work(struct work_struct *work)
> rdata->event = RPORT_EV_NONE;
> rdata->major_retries = 0;
> kref_get(&rdata->kref);
> - mutex_unlock(&rdata->rp_mutex);
> + spin_unlock(&rdata->rp_lock);
>
> if (!rport)
> rport = fc_remote_port_add(lport->host, 0, &ids);
> @@ -274,7 +274,7 @@ static void fc_rport_work(struct work_struct *work)
> kref_put(&rdata->kref, lport->tt.rport_destroy);
> return;
> }
> - mutex_lock(&rdata->rp_mutex);
> + spin_lock(&rdata->rp_lock);
> if (rdata->rport)
> FC_RPORT_DBG(rdata, "rport already allocated\n");
> rdata->rport = rport;
> @@ -287,7 +287,7 @@ static void fc_rport_work(struct work_struct *work)
> rpriv->flags = rdata->flags;
> rpriv->e_d_tov = rdata->e_d_tov;
> rpriv->r_a_tov = rdata->r_a_tov;
> - mutex_unlock(&rdata->rp_mutex);
> + spin_unlock(&rdata->rp_lock);
>
> if (rport_ops && rport_ops->event_callback) {
> FC_RPORT_DBG(rdata, "callback ev %d\n", event);
> @@ -313,7 +313,7 @@ static void fc_rport_work(struct work_struct *work)
> mutex_unlock(&fc_prov_mutex);
> }
> port_id = rdata->ids.port_id;
> - mutex_unlock(&rdata->rp_mutex);
> + spin_unlock(&rdata->rp_lock);
>
> if (rport_ops && rport_ops->event_callback) {
> FC_RPORT_DBG(rdata, "callback ev %d\n", event);
> @@ -334,18 +334,18 @@ static void fc_rport_work(struct work_struct *work)
> if (rport) {
> rpriv = rport->dd_data;
> rpriv->rp_state = RPORT_ST_DELETE;
> - mutex_lock(&rdata->rp_mutex);
> + spin_lock(&rdata->rp_lock);
> rdata->rport = NULL;
> - mutex_unlock(&rdata->rp_mutex);
> + spin_unlock(&rdata->rp_lock);
> fc_remote_port_delete(rport);
> }
>
> mutex_lock(&lport->disc.disc_mutex);
> - mutex_lock(&rdata->rp_mutex);
> + spin_lock(&rdata->rp_lock);
> if (rdata->rp_state == RPORT_ST_DELETE) {
> if (port_id == FC_FID_DIR_SERV) {
> rdata->event = RPORT_EV_NONE;
> - mutex_unlock(&rdata->rp_mutex);
> + spin_unlock(&rdata->rp_lock);
> kref_put(&rdata->kref, lport->tt.rport_destroy);
> } else if ((rdata->flags & FC_RP_STARTED) &&
> rdata->major_retries <
> @@ -354,11 +354,11 @@ static void fc_rport_work(struct work_struct *work)
> rdata->event = RPORT_EV_NONE;
> FC_RPORT_DBG(rdata, "work restart\n");
>
Re: [PATCH] sd: get disk reference in sd_check_events()
On Tue, 2016-04-26 at 08:06 +0200, Hannes Reinecke wrote:
> sd_check_events() is called asynchronously, and might race
> with device removal. So always take a disk reference when
> processing the event to avoid the device being removed while
> the event is processed.
>
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/sd.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index f52b74c..91f609f 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1398,11 +1398,15 @@ static int media_not_present(struct scsi_disk *sdkp,
> **/
> static unsigned int sd_check_events(struct gendisk *disk, unsigned int
> clearing)
> {
> - struct scsi_disk *sdkp = scsi_disk(disk);
> - struct scsi_device *sdp = sdkp->device;
> + struct scsi_disk *sdkp = scsi_disk_get(disk);
> + struct scsi_device *sdp;
> struct scsi_sense_hdr *sshdr = NULL;
> int retval;
>
> + if (!sdkp)
> + return 0;
> +
> + sdp = sdkp->device;
> SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_check_events\n"));
>
> /*
> @@ -1459,6 +1463,7 @@ out:
> kfree(sshdr);
> retval = sdp->changed ? DISK_EVENT_MEDIA_CHANGE : 0;
> sdp->changed = 0;
> + scsi_disk_put(sdkp);
> return retval;
> }
>
This has been verified to fix a reported crash:
[15111.105473] Workqueue: events_freezable disk_events_workfn
[15111.106229] task: 88082297ae00 ti: 8807b54e task.ti:
8807b54e
[15111.107039] RIP: 0010:[] []
sd_check_events+0x24/0x1a0 [sd_mod]
[15111.107885] RSP: 0018:8807b54e3d88 EFLAGS: 00010293
[15111.108739] RAX: RBX: RCX: dead00200200
[15111.109587] RDX: 0001 RSI: 0001 RDI: 8807c040f400
[15111.110403] RBP: 8807b54e3da0 R08: 8807d233f568 R09: dfe03f37fcb3f560
[15111.111208] R10: dfe03f37fcb3f560 R11: 0001 R12: 8807c040f400
[15111.112018] R13: 88084fc33fc0 R14: 0001 R15: 8807d233f550
[15111.112830] FS: () GS:88084fc2()
knlGS:
[15111.113681] CS: 0010 DS: ES: CR0: 8005003b
[15111.114511] CR2: 0008 CR3: 000f9cbb8000 CR4: 07e0
[15111.115370] DR0: DR1: DR2:
[15111.116268] DR3: DR6: 0ff0 DR7: 0400
[15111.117131] Stack:
[15111.118010] 8807d233f500 8807c040f400 88084fc33fc0
8807b54e3e08
[15111.118932] 812da84b
[15111.119894] 57fe1a80
8807d233f560
[15111.120842] Call Trace:
[15111.121832] [] disk_check_events+0x5b/0x1b0
[15111.122817] [] disk_events_workfn+0x16/0x20
[15111.123848] [] process_one_work+0x17b/0x470
[15111.124872] [] worker_thread+0x11b/0x400
[15111.125904] [] ? rescuer_thread+0x400/0x400
[15111.126928] [] kthread+0xcf/0xe0
[15111.127967] [] ? kthread_create_on_node+0x140/0x140
[15111.129040] [] ret_from_fork+0x58/0x90
[15111.130118] [] ? kthread_create_on_node+0x140/0x140
[15111.131208] Code: 1f 84 00 00 00 00 00 66 66 66 66 90 55 8b 05 28 fb e8 e1
48 89 e5 41 55 c1 e8 15 41 54 83 e0 07 83 f8 03 53 48 8b 9f 68 03 00 00 <4c> 8b
63 08 0f 87 18 01 00 00 41 8b 84 24 d0 06 00 00 8d 50 fa
[15111.133564] RIP [] sd_check_events+0x24/0x1a0 [sd_mod]
[15111.134721] RSP
[15111.135878] CR2: 0008
Reviewed-by: Ewan D. Milne
--
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] scsi: use spinlock instead of mutex for RCU-protected VPD inquiry data
From: "Ewan D. Milne"
A spinlock is sufficient for this purpose, and much smaller.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi.c| 8
drivers/scsi/scsi_scan.c | 2 +-
include/scsi/scsi_device.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 1deb6ad..330d807 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -829,11 +829,11 @@ retry_pg80:
kfree(vpd_buf);
goto retry_pg80;
}
- mutex_lock(&sdev->inquiry_mutex);
+ spin_lock(&sdev->inquiry_lock);
orig_vpd_buf = sdev->vpd_pg80;
sdev->vpd_pg80_len = result;
rcu_assign_pointer(sdev->vpd_pg80, vpd_buf);
- mutex_unlock(&sdev->inquiry_mutex);
+ spin_unlock(&sdev->inquiry_lock);
synchronize_rcu();
if (orig_vpd_buf) {
kfree(orig_vpd_buf);
@@ -858,11 +858,11 @@ retry_pg83:
kfree(vpd_buf);
goto retry_pg83;
}
- mutex_lock(&sdev->inquiry_mutex);
+ spin_lock(&sdev->inquiry_lock);
orig_vpd_buf = sdev->vpd_pg83;
sdev->vpd_pg83_len = result;
rcu_assign_pointer(sdev->vpd_pg83, vpd_buf);
- mutex_unlock(&sdev->inquiry_mutex);
+ spin_unlock(&sdev->inquiry_lock);
synchronize_rcu();
if (orig_vpd_buf)
kfree(orig_vpd_buf);
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index e0a78f5..f445615 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -240,7 +240,7 @@ static struct scsi_device *scsi_alloc_sdev(struct
scsi_target *starget,
INIT_LIST_HEAD(&sdev->starved_entry);
INIT_LIST_HEAD(&sdev->event_list);
spin_lock_init(&sdev->list_lock);
- mutex_init(&sdev->inquiry_mutex);
+ spin_lock_init(&sdev->inquiry_lock);
INIT_WORK(&sdev->event_work, scsi_evt_thread);
INIT_WORK(&sdev->requeue_work, scsi_requeue_run_queue);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index a6c346d..0410ed8 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -115,7 +115,7 @@ struct scsi_device {
char type;
char scsi_level;
char inq_periph_qual; /* PQ from INQUIRY data */
- struct mutex inquiry_mutex;
+ spinlock_t inquiry_lock;
unsigned char inquiry_len; /* valid bytes in 'inquiry' */
unsigned char * inquiry;/* INQUIRY response data */
const char * vendor;/* [back_compat] point into 'inquiry'
... */
--
2.1.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
[PATCH] scsi: Add QEMU CD-ROM to VPD Inquiry Blacklist
Linux fails to boot as a guest with a QEMU CD-ROM:
[4.439488] ata2.00: ATAPI: QEMU CD-ROM, 0.8.2, max UDMA/100
[4.443649] ata2.00: configured for MWDMA2
[4.450267] scsi 1:0:0:0: CD-ROMQEMU QEMU CD-ROM 0.8.
PQ: 0 ANSI: 5
[4.464317] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
[4.464319] ata2.00: BMDMA stat 0x5
[4.464339] ata2.00: cmd a0/01:00:00:00:01/00:00:00:00:00/a0 tag 0 dma 16640
in
[4.464339] Inquiry 12 01 00 00 ff 00res
48/20:02:00:24:00/00:00:00:00:00/a0 Emask 0x2 (HSM violation)
[4.464341] ata2.00: status: { DRDY DRQ }
[4.465864] ata2: soft resetting link
[4.625971] ata2.00: configured for MWDMA2
[4.628290] ata2: EH complete
[4.646670] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
[4.646671] ata2.00: BMDMA stat 0x5
[4.646683] ata2.00: cmd a0/01:00:00:00:01/00:00:00:00:00/a0 tag 0 dma 16640
in
[4.646683] Inquiry 12 01 00 00 ff 00res
48/20:02:00:24:00/00:00:00:00:00/a0 Emask 0x2 (HSM violation)
[4.646685] ata2.00: status: { DRDY DRQ }
[4.648193] ata2: soft resetting link
...
Fix this by suppressing VPD inquiry for this device.
Reported-by: Jan Stancek
Tested-by: Jan Stancek
Cc:
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_devinfo.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index bbfbfd9..09bbd3f 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -228,6 +228,7 @@ static struct {
{"PIONEER", "CD-ROM DRM-624X", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
{"Promise", "VTrak E610f", NULL, BLIST_SPARSELUN | BLIST_NO_RSOC},
{"Promise", "", NULL, BLIST_SPARSELUN},
+ {"QEMU", "QEMU CD-ROM", NULL, BLIST_SKIP_VPD_PAGES},
{"QNAP", "iSCSI Storage", NULL, BLIST_MAX_1024},
{"SYNOLOGY", "iSCSI Storage", NULL, BLIST_MAX_1024},
{"QUANTUM", "XP34301", "1071", BLIST_NOTQ},
--
1.7.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: Add QEMU CD-ROM to VPD Inquiry Blacklist
This is an existing configuration that broke with the VPD rescan
changes. The problem is that the error causes a rescan to be
initiated, which results in endless error messages.
On Fri, 2016-06-03 at 03:15 +, Tom Yan wrote:
> Why not get qemu have it fixed instead?
>
> On Tuesday, 31 May 2016, Johannes Thumshirn
> wrote:
> On Tue, May 31, 2016 at 09:42:29AM -0400, Ewan D. Milne wrote:
> > Linux fails to boot as a guest with a QEMU CD-ROM:
> >
> > [4.439488] ata2.00: ATAPI: QEMU CD-ROM, 0.8.2, max
> UDMA/100
> > [4.443649] ata2.00: configured for MWDMA2
> > [4.450267] scsi 1:0:0:0: CD-ROMQEMU QEMU
> CD-ROM 0.8. PQ: 0 ANSI: 5
> > [4.464317] ata2.00: exception Emask 0x0 SAct 0x0 SErr
> 0x0 action 0x6 frozen
> > [4.464319] ata2.00: BMDMA stat 0x5
> > [4.464339] ata2.00: cmd
> a0/01:00:00:00:01/00:00:00:00:00/a0 tag 0 dma 16640 in
> > [4.464339] Inquiry 12 01 00 00 ff 00res
> 48/20:02:00:24:00/00:00:00:00:00/a0 Emask 0x2 (HSM violation)
> > [4.464341] ata2.00: status: { DRDY DRQ }
> > [4.465864] ata2: soft resetting link
> > [4.625971] ata2.00: configured for MWDMA2
> > [4.628290] ata2: EH complete
> > [4.646670] ata2.00: exception Emask 0x0 SAct 0x0 SErr
> 0x0 action 0x6 frozen
> > [4.646671] ata2.00: BMDMA stat 0x5
> > [4.646683] ata2.00: cmd
> a0/01:00:00:00:01/00:00:00:00:00/a0 tag 0 dma 16640 in
> > [4.646683] Inquiry 12 01 00 00 ff 00res
> 48/20:02:00:24:00/00:00:00:00:00/a0 Emask 0x2 (HSM violation)
> > [4.646685] ata2.00: status: { DRDY DRQ }
> > [4.648193] ata2: soft resetting link
> >
> > ...
> >
> > Fix this by suppressing VPD inquiry for this device.
> >
> > Reported-by: Jan Stancek
> > Tested-by: Jan Stancek
> > Cc:
> > Signed-off-by: Ewan D. Milne
>
> Reviewed-by: Johannes Thumshirn
>
> --
> Johannes Thumshirn
> Storage
> jthumsh...@suse.de+49 911
> 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76
> 0850
> --
> 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
--
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: Add QEMU CD-ROM to VPD Inquiry Blacklist
commit 09e2b0b14690fb13ccfc04af49f156df3e25b152
Author: Hannes Reinecke
Date: Mon Nov 9 13:24:28 2015 +0100
scsi: rescan VPD attributes
---
see also:
commit 5ddfe0858ea7848c5d4efe3f4319e7543522e0ee
Author: Hannes Reinecke
Date: Fri Apr 1 08:57:36 2016 +0200
scsi: Do not attach VPD to devices that don't support it
commit 82c43310508eb19eb41fe7862e89afeb74030b84
Author: Mika Westerberg
Date: Wed Jan 27 16:19:13 2016 +0200
SCSI: Add Marvell Console to VPD blacklist
and presumably:
commit fb2d65d28918ef4f0caa1de3d8c7416949c28b41
Author: Todd Fujinaka
Date: Tue Feb 9 21:02:07 2016 -0500
SCSI: Add Marvell configuration device to VPD blacklist
On Sat, 2016-06-04 at 03:14 +0800, Tom Yan wrote:
> What changes are you referring to? Doesn't that mean the changes are
> flawed and some actual follow-up fixes are needed instead?
>
> On 3 June 2016 at 22:36, Ewan D. Milne wrote:
> > This is an existing configuration that broke with the VPD rescan
> > changes. The problem is that the error causes a rescan to be
> > initiated, which results in endless error messages.
> >
> > On Fri, 2016-06-03 at 03:15 +, Tom Yan wrote:
> >> Why not get qemu have it fixed instead?
> >>
> >> On Tuesday, 31 May 2016, Johannes Thumshirn
> >> wrote:
> >> On Tue, May 31, 2016 at 09:42:29AM -0400, Ewan D. Milne wrote:
> >> > Linux fails to boot as a guest with a QEMU CD-ROM:
> >> >
> >> > [4.439488] ata2.00: ATAPI: QEMU CD-ROM, 0.8.2, max
> >> UDMA/100
> >> > [4.443649] ata2.00: configured for MWDMA2
> >> > [4.450267] scsi 1:0:0:0: CD-ROMQEMU QEMU
> >> CD-ROM 0.8. PQ: 0 ANSI: 5
> >> > [4.464317] ata2.00: exception Emask 0x0 SAct 0x0 SErr
> >> 0x0 action 0x6 frozen
> >> > [4.464319] ata2.00: BMDMA stat 0x5
> >> > [4.464339] ata2.00: cmd
> >> a0/01:00:00:00:01/00:00:00:00:00/a0 tag 0 dma 16640 in
> >> > [4.464339] Inquiry 12 01 00 00 ff 00res
> >> 48/20:02:00:24:00/00:00:00:00:00/a0 Emask 0x2 (HSM violation)
> >> > [4.464341] ata2.00: status: { DRDY DRQ }
> >> > [4.465864] ata2: soft resetting link
> >> > [4.625971] ata2.00: configured for MWDMA2
> >> > [4.628290] ata2: EH complete
> >> > [4.646670] ata2.00: exception Emask 0x0 SAct 0x0 SErr
> >> 0x0 action 0x6 frozen
> >> > [4.646671] ata2.00: BMDMA stat 0x5
> >> > [4.646683] ata2.00: cmd
> >> a0/01:00:00:00:01/00:00:00:00:00/a0 tag 0 dma 16640 in
> >> > [4.646683] Inquiry 12 01 00 00 ff 00res
> >> 48/20:02:00:24:00/00:00:00:00:00/a0 Emask 0x2 (HSM violation)
> >> > [4.646685] ata2.00: status: { DRDY DRQ }
> >> > [4.648193] ata2: soft resetting link
> >> >
> >> > ...
> >> >
> >> > Fix this by suppressing VPD inquiry for this device.
> >> >
> >> > Reported-by: Jan Stancek
> >> > Tested-by: Jan Stancek
> >> > Cc:
> >> > Signed-off-by: Ewan D. Milne
> >>
> >> Reviewed-by: Johannes Thumshirn
> >>
> >> --
> >> Johannes Thumshirn
> >> Storage
> >> jthumsh...@suse.de+49 911
> >> 74053 689
> >> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> >> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> >> HRB 21284 (AG Nürnberg)
> >> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76
> >> 0850
> >> --
> >> 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
> >
> >
--
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: Add QEMU CD-ROM to VPD Inquiry Blacklist
On Mon, 2016-06-06 at 09:34 +0200, Hannes Reinecke wrote:
> On 05/31/2016 03:42 PM, Ewan D. Milne wrote:
> > Linux fails to boot as a guest with a QEMU CD-ROM:
> >
> > [4.439488] ata2.00: ATAPI: QEMU CD-ROM, 0.8.2, max UDMA/100
> > [4.443649] ata2.00: configured for MWDMA2
> > [4.450267] scsi 1:0:0:0: CD-ROMQEMU QEMU CD-ROM
> > 0.8. PQ: 0 ANSI: 5
> > [4.464317] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
> > frozen
> > [4.464319] ata2.00: BMDMA stat 0x5
> > [4.464339] ata2.00: cmd a0/01:00:00:00:01/00:00:00:00:00/a0 tag 0 dma
> > 16640 in
> > [4.464339] Inquiry 12 01 00 00 ff 00res
> > 48/20:02:00:24:00/00:00:00:00:00/a0 Emask 0x2 (HSM violation)
> > [4.464341] ata2.00: status: { DRDY DRQ }
> > [4.465864] ata2: soft resetting link
> > [4.625971] ata2.00: configured for MWDMA2
> > [4.628290] ata2: EH complete
> > [4.646670] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
> > frozen
> > [4.646671] ata2.00: BMDMA stat 0x5
> > [4.646683] ata2.00: cmd a0/01:00:00:00:01/00:00:00:00:00/a0 tag 0 dma
> > 16640 in
> > [4.646683] Inquiry 12 01 00 00 ff 00res
> > 48/20:02:00:24:00/00:00:00:00:00/a0 Emask 0x2 (HSM violation)
> > [4.646685] ata2.00: status: { DRDY DRQ }
> > [4.648193] ata2: soft resetting link
> >
> > ...
> >
> > Fix this by suppressing VPD inquiry for this device.
> >
> > Reported-by: Jan Stancek
> > Tested-by: Jan Stancek
> > Cc:
> > Signed-off-by: Ewan D. Milne
> > ---
> > drivers/scsi/scsi_devinfo.c |1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
> > index bbfbfd9..09bbd3f 100644
> > --- a/drivers/scsi/scsi_devinfo.c
> > +++ b/drivers/scsi/scsi_devinfo.c
> > @@ -228,6 +228,7 @@ static struct {
> > {"PIONEER", "CD-ROM DRM-624X", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
> > {"Promise", "VTrak E610f", NULL, BLIST_SPARSELUN | BLIST_NO_RSOC},
> > {"Promise", "", NULL, BLIST_SPARSELUN},
> > + {"QEMU", "QEMU CD-ROM", NULL, BLIST_SKIP_VPD_PAGES},
> > {"QNAP", "iSCSI Storage", NULL, BLIST_MAX_1024},
> > {"SYNOLOGY", "iSCSI Storage", NULL, BLIST_MAX_1024},
> > {"QUANTUM", "XP34301", "1071", BLIST_NOTQ},
> >
> This doesn't apply to all versions of QEMU (upstream qemu works fine),
> so at the very least it needs to be restricted to a specific version.
Well, the "0.8." version shown above definitely didn't work.
>
> At the same time I'd _rather_ like to figure out why things break in the
> first place.
> QEMU CD-ROM claims to support SPC-3, for which VPD pages are mandatory.
> So why is this error generated?
Not sure why the error is generated, but if it is, we can't keep sending
the commands, the guest won't boot. I guess the real problem is do we
want to keep finding the problem devices one-by-one, or should we put in
some logic to e.g. avoid re-reading the VPD pages on rescan if the
initial scan didn't work. Seems like a lot of trouble, though, for a
broken device, and that's what the blacklist is for.
>
> Either way, this patch is wrong.
If we can identify which versions work, we can update it. Otherwise
I think we have to be conservative.
>
> NACK.
>
> Cheers,
>
> Hannes
--
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: kernel BUG in drivers/scsi/53c700.c:1129
I'm not sure if this is the problem, but the tagging changes to scsi_tcq.h may have altered the 53c700 driver's assumptions. In one case it sets sdev->current_cmnd and then some of the tagging calls would return it if the tag was SCSI_NO_TAG. NCR_700_queuecommand_lck() does: if ((hostdata->tag_negotiated & (1simple_tags) { slot->tag = SCp->request->tag; CDEBUG(KERN_DEBUG, SCp, "sending out tag %d, slot %p\n", slot->tag, slot); } else { slot->tag = SCSI_NO_TAG; /* must populate current_cmnd for scsi_host_find_tag to work */ SCp->device->current_cmnd = SCp; } -Ewan On Fri, 2016-06-10 at 22:25 +0200, Helge Deller wrote: > On 10.06.2016 00:23, James Bottomley wrote: > > On Thu, 2016-06-09 at 21:36 +0200, Helge Deller wrote: > >> Hi James, > >> > >> I just tried Debian kernel 4.6.1-1 on my historic 715/64 machine, and it > >> ran into this BUG() in the LASI scsi driver: > >> > >> scsi 0:0:6:0: no saved request for untagged cmd > >> > >> Any idea? > >> > >> Helge > >> > >> [0.00] Linux version 4.6.0-1-parisc > >> (debian-ker...@lists.debian.org) (gcc version 5.4.0 20160603 (Debian > >> 5.4.0-3) ) #1 Debian 4.6.1-1 (2016-06-06) > >> ... > >> [0.00] Determining PDC firmware type: Snake. > >> [0.00] model 60a0 0481 773c7d2c > >> 0004 0072 0072 > >> [0.00] vers 000c > >> [0.00] model 9000/715 > >> [0.00] Total Memory: 160 MB > >> ... > >> [ 43.18] SCSI subsystem initialized > >> [ 45.076000] 53c700: Version 2.8 By james.bottom...@hansenpartnership.com > >> [ 45.156000] scsi0: 53c710 rev 2 > >> [ 46.204000] scsi host0: LASI SCSI 53c700 > >> [ 58.268000] scsi 0:0:6:0: no saved request for untagged cmd > >> [ 58.336000] [ cut here ] > >> [ 58.392000] kernel BUG at > >> /build/linux-XAODSw/linux-4.6.1/drivers/scsi/53c700.c:1129! > >> [ 58.484000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.6.0-1-parisc #1 > >> Debian 4.6.1-1 > >> [ 58.58] task: 1083cdd8 ti: 107b1000 task.ti: 107b1000 > >> [ 58.644000] > >> [ 58.66] YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI > >> [ 58.716000] PSW: 01001110 Not tainted > >> [ 58.788000] r00-03 0004ff0e 0084e388 00a8ea8c 107ba400 > >> [ 58.848000] r04-07 15b88cc0 0006 > >> [ 58.912000] r08-11 198ca400 00ff 0020 > >> [ 58.976000] r12-15 00ff 1003 10801020 0001 > >> [ 59.036000] r16-19 107b1240 0002 00ff 0001 > >> [ 59.10] r20-23 0001 000e 0084f388 > >> [ 59.164000] r24-27 107ba500 0083b540 1021a7a4 107b5020 > >> [ 59.224000] r28-31 0040 107ba480 104d52bc > >> [ 59.288000] sr00-03 000d > >> [ 59.352000] sr04-07 > >> [ 59.416000] > >> [ 59.432000] IASQ: IAOQ: 00a8ea8c 00a8ea90 > >> [ 59.50] IIR: 03ffe01fISR: 000d IOR: 00a8ea90 > >> [ 59.564000] CPU:0 CR30: 107b1000 CR31: f00e > >> [ 59.632000] ORIG_R28: 10855e80 > >> [ 59.668000] IAOQ[0]: process_script_interrupt+0x13ec/0x16a4 [53c700] > >> [ 59.748000] IAOQ[1]: process_script_interrupt+0x13f0/0x16a4 [53c700] > >> [ 59.824000] RP(r2): process_script_interrupt+0x13ec/0x16a4 [53c700] > >> [ 59.90] Backtrace: > >> [ 59.928000] [<00a8ea8c>] process_script_interrupt+0x13ec/0x16a4 > >> [53c700] > >> [ 60.012000] > >> [ 60.028000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.6.0-1-parisc #1 > >> Debian 4.6.1-1 > >> [ 60.12] Backtrace: > >> [ 60.148000] [<1015bffc>] show_stack+0x3c/0x50 > >> [ 60.204000] [<10425d40>] dump_stack+0x28/0x38 > >> [ 60.256000] [<1015c180>] die_if_kernel+0x134/0x20c > >> [ 60.316000] [<1015cd34>] handle_interruption+0x804/0x828 > >> [ 60.38] > >> [ 60.40] Kernel panic - not syncing: Fatal exception in interrupt > >> [ 60.40] ---[ end Kernel panic - not syncing: Fatal exception in > >> interrupt > > > > It looks like either an unsolicited reselection or possibly a spurious > > interrupt left over from something. Can you define NCR_700_DEBUG in > > drivers/scsi/53c700.h and see what it says? > > I enabled NCR_700_DEBUG and NCR_700_TAG_DEBUG. > This is from vanilla kernel 4.6: > > [8.70] Uniform Multi-Platform E-IDE driver > [8.76] ide-gd driver 1.18 > [8.79] ide-cd driver 5.00 > [8.86] script, patching MessageLocation at 31 to 0x09f684b0 > [8.93] script, patching StatusAddress at 281 to 0x09f684d0 > [9.00] script, patching ReceiveMsgAddress at 15 to 0x09f684c0 > [9.08] script, patching ReceiveMsgAddress at 19 to 0x09f684c0 > [9.15] script, patching ReceiveMsgAddress at 41 to 0x09f684c0 > [9.23] script, patching Rece
[PATCH] [SCSI] Fix incorrect comment in scsi_logging.h
From: "Ewan D. Milne" The scsi_logging_level word contains 10 3-bit fields, not 8 nibbles. Fix the incorrect comment. Signed-off-by: Ewan D. Milne --- drivers/scsi/scsi_logging.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/scsi_logging.h b/drivers/scsi/scsi_logging.h index 1f65139..bd6f58a 100644 --- a/drivers/scsi/scsi_logging.h +++ b/drivers/scsi/scsi_logging.h @@ -5,8 +5,8 @@ /* * This defines the scsi logging feature. It is a means by which the user * can select how much information they get about various goings on, and it - * can be really useful for fault tracing. The logging word is divided into - * 8 nibbles, each of which describes a loglevel. The division of things is + * can be really useful for fault tracing. The logging word contains 10 + * fields, each of which describes a loglevel. The division of things is * somewhat arbitrary, and the division of the word could be changed if it * were really needed for any reason. The numbers below are the only place * where these are specified. For a first go-around, 3 bits is more than -- 1.7.11.7 -- 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 v3 4/6] [SCSI] Generate uevents for certain Unit Attention codes
From: "Ewan D. Milne"
Generate a uevent on the scsi_target object when the following
Unit Attention ASC/ASCQ code is received:
3F/0E REPORTED LUNS DATA HAS CHANGED
Generate a uevent on the scsi_device object when the following
Unit Attention ASC/ASCQ codes are received:
2A/01 MODE PARAMETERS CHANGED
2A/09 CAPACITY DATA HAS CHANGED
38/07 THIN PROVISIONING SOFT THRESHOLD REACHED
All uevent generation is aggregated and rate-limited so that any
individual event is delivered no more than once every 2 seconds.
Log kernel messages when the following Unit Attention ASC/ASCQ
codes are received:
2A/xx PARAMETERS CHANGED
3F/03 INQUIRY DATA HAS CHANGED
3F/xx TARGET OPERATING CONDITIONS HAVE CHANGED
Also changed the kernel log messages on existing Unit Attention
codes, to remove text that indicates that the conditions were not
handled in any way (since they are now handled in some way) and
reflect the wording used in SPC-4.
Also log a kernel message when the a scsi device reports a Unit
Attention queue overflow, indicating that status has been lost.
These changes are only enabled when the kernel config option
CONFIG_SCSI_ENHANCED_UA is set. Otherwise, the behavior is the
same as before, including the existing kernel log messages.
However, the detection of these conditions was moved to be earlier
in scsi_check_sense(), because the code was not always reached.
Added a new exported function scsi_report_sense() to allow drivers
to report sense data that is not associated with a SCSI command.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_error.c | 187 -
drivers/scsi/scsi_lib.c| 17 -
drivers/scsi/scsi_priv.h | 2 +
drivers/scsi/scsi_scan.c | 5 ++
drivers/scsi/scsi_sysfs.c | 3 +
include/scsi/scsi_cmnd.h | 4 +
include/scsi/scsi_device.h | 22 ++
include/scsi/scsi_eh.h | 5 ++
8 files changed, 223 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index d0f71e5..d0b5a26 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -223,6 +223,86 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host
*shost,
#endif
/**
+ * scsi_report_sense - Examine scsi sense information and log messages for
+ *certain conditions, also issue uevents if so configured.
+ * @sshdr: sshdr to be examined
+ */
+void scsi_report_sense(struct scsi_device *sdev, struct scsi_sense_hdr *sshdr)
+{
+#ifdef CONFIG_SCSI_ENHANCED_UA
+ if (sshdr->ua_queue_overflow)
+ sdev_printk(KERN_WARNING, sdev,
+ "Unit Attention queue overflow");
+#endif
+
+ if (sshdr->sense_key == UNIT_ATTENTION) {
+ if (sshdr->asc == 0x3f && sshdr->ascq == 0x03)
+ sdev_printk(KERN_WARNING, sdev,
+ "Inquiry data has changed");
+ else if (sshdr->asc == 0x3f && sshdr->ascq == 0x0e) {
+#ifdef CONFIG_SCSI_ENHANCED_UA
+ struct scsi_target *starget = scsi_target(sdev);
+ if (atomic_xchg(&starget->lun_change_reported, 1) == 0)
+ schedule_delayed_work(&starget->ua_dwork, 2*HZ);
+ sdev_printk(KERN_WARNING, sdev,
+ "Reported LUNs data has changed");
+#else
+ sdev_printk(KERN_WARNING, sdev,
+ "Warning! Received an indication that the "
+ "LUN assignments on this target have "
+ "changed. The Linux SCSI layer does not "
+ "automatically remap LUN assignments.\n");
+#endif
+ } else if (sshdr->asc == 0x3f)
+#ifdef CONFIG_SCSI_ENHANCED_UA
+ sdev_printk(KERN_WARNING, sdev,
+ "Target operating conditions have changed");
+#else
+ sdev_printk(KERN_WARNING, sdev,
+ "Warning! Received an indication that the "
+ "operating parameters on this target have "
+ "changed. The Linux SCSI layer does not "
+ "automatically adjust these parameters.\n");
+#endif
+
+ if (sshdr->asc == 0x38 && sshdr->ascq == 0x07) {
+#ifdef CONFIG_SCSI_ENHANCED_UA
+ if (atomic_xchg(&sdev->soft_threshold_reached, 1) == 0)
+ schedule_delayed_work(&sdev->ua_dwork, 2 * HZ);
+ sdev_printk(KERN_WARNING, sdev,
+ "Thin provisioning soft thres
[PATCH v3 1/6] [SCSI] Add a kernel config option for enhanced Unit Attention support
From: "Ewan D. Milne" Added CONFIG_SCSI_ENHANCED_UA kernel config option to enable changes in the SCSI mid-layer which detect and report certain Unit Attention conditions reported by devices. These changes are primarily useful when storage arrays that can be reconfigured are being used, so the config option would normally not be used unless it was needed. Signed-off-by: Ewan D. Milne --- drivers/scsi/Kconfig | 12 1 file changed, 12 insertions(+) diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index e955978..5d1e614 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -280,6 +280,18 @@ config SCSI_WAIT_SCAN # disabling it, whereas people who accidentally switch it off may wonder why # their mkinitrd gets into trouble. +config SCSI_ENHANCED_UA + bool "Enhanced SCSI Unit Attention handling" + depends on SCSI + help +Certain SCSI devices report changes via a UNIT ATTENTION code. +(For example, the addition or removal of LUNs from a target, or +changing the number of logical blocks on a LUN.) This option +enables reporting of these changes via udev events, so that the +device can be rescanned to find out what has changed. This is +primarily useful when storage arrays that can be reconfigured +are attached to the system, otherwise you can say N here. + menu "SCSI Transports" depends on SCSI -- 1.7.11.7 -- 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 v3 0/6] [SCSI] Enhanced sense and Unit Attention handling
From: "Ewan D. Milne" This patch set adds changes to the SCSI mid-layer, sysfs and scsi_debug to provide enhanced support for Unit Attention conditions, as well as detection of a unit attention queue overflow condition and the ability for drivers to report sense data outside of normal command completion. There was some discussion about this a couple of years ago on the linux-scsi mailing list: http://marc.info/?l=linux-scsi&m=129702506514742&w=2 Although one approach is to send all SCSI sense data to a userspace daemon for processing, this patch set does not take that approach due to the difficulty in reliably delivering all of the data. An interesting UA condition might not be delivered due to a flood of media errors, for example. The mechanism used is to flag when certain UA ASC/ASCQ codes are received that report asynchronous changes to the storage device configuration. An appropriate uevent is then generated for the scsi_device or scsi_target object. An aggregation mechanism is used to avoid generating uevents at too high a rate, and to coalesce multiple UAs reported by LUNs on the same target for a REPORTED LUNS DATA HAS CHANGED sense code. The changes are enabled by a new kernel config option CONFIG_SCSI_ENHANCED_UA. If this config option is not used, no new uevents are generated. There are some changes to kernel logging messages if CONFIG_SCSI_ENHANCED_UA is enabled, because the existing messages explicitly stated that the kernel did not do anything with the information. Note that checkpatch is reporting errors on patch 5/6 relating to macros in scsi_sysfs.c -- I believe these errors are incorrect and have sent a message to the checkpatch maintainer. The macros were derived from existing ones already in the file. Changes made since earlier v2 version: - Remove patch 1/8 "Generate uevent on sd capacity change" - Remove patch 8/8 "Streamline detection of FM/EOM/ILI status" - Changed scsi_debug to not generate UA on INQUIRY or REPORT_LUNS - Changed scsi_debug to only report UA queue overflow condition if dsense=1, as descriptor format sense data is needed Changes made since earlier RFC version: - Remove patch 1/9 "Detect overflow of sense data buffer" Some scsi_debug changes in this patch were moved to patch 7/8 - Corrected Kconfig help text - Change name of "sdev_evt_thread" to "sdev_evt_work" - Change name of "starget_evt_thread" to "starget_evt_work" - Pull code out of scsi_check_sense() that handles UAs into an exported function so that drivers can report conditions received asynchronously Thanks to everyone for the comments on this patch series. Ewan D. Milne (6): [SCSI] Add a kernel config option for enhanced Unit Attention support [SCSI] Rename scsi_evt_xxx to sdev_evt_xxx and scsi_event to sdev_event [SCSI] Add support for scsi_target events [SCSI] Generate uevents for certain Unit Attention codes [SCSI] Add sysfs support for enhanced Unit Attention handling [SCSI] Add sense and Unit Attention generation to scsi_debug drivers/scsi/Kconfig | 12 +++ drivers/scsi/scsi_debug.c | 138 + drivers/scsi/scsi_error.c | 187 - drivers/scsi/scsi_lib.c| 176 ++ drivers/scsi/scsi_priv.h | 10 ++- drivers/scsi/scsi_scan.c | 54 +++-- drivers/scsi/scsi_sysfs.c | 146 --- include/scsi/scsi_cmnd.h | 4 + include/scsi/scsi_device.h | 62 ++- include/scsi/scsi_eh.h | 5 ++ 10 files changed, 716 insertions(+), 78 deletions(-) -- 1.7.11.7 -- 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 v3 3/6] [SCSI] Add support for scsi_target events
From: "Ewan D. Milne"
Added capability to generate uevents on scsi_target objects.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_lib.c| 135 +
drivers/scsi/scsi_priv.h | 3 +
drivers/scsi/scsi_scan.c | 17 ++
include/scsi/scsi_device.h | 34
4 files changed, 189 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 53f074d..c55eea1 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2390,6 +2390,141 @@ scsi_target_resume(struct scsi_target *starget)
}
EXPORT_SYMBOL(scsi_target_resume);
+#ifdef CONFIG_SCSI_ENHANCED_UA
+/**
+ * starget_evt_emit - emit a single SCSI target uevent
+ * @starget: associated SCSI target
+ * @evt: event to emit
+ *
+ * Send a single uevent (starget_event) to the associated scsi_target.
+ */
+static void starget_evt_emit(struct scsi_target *starget,
+struct starget_event *evt)
+{
+ int idx = 0;
+ char *envp[3];
+
+ switch (evt->evt_type) {
+ case STARGET_EVT_LUN_CHANGE_REPORTED:
+ envp[idx++] = "STARGET_LUN_CHANGE_REPORTED=1";
+ break;
+ default:
+ /* do nothing */
+ break;
+ }
+
+ envp[idx++] = NULL;
+
+ kobject_uevent_env(&starget->dev.kobj, KOBJ_CHANGE, envp);
+}
+
+/**
+ * starget_evt_work - send a uevent for each scsi event
+ * @work: work struct for scsi_target
+ *
+ * Dispatch queued events to their associated scsi_target kobjects
+ * as uevents.
+ */
+void starget_evt_work(struct work_struct *work)
+{
+ struct scsi_target *starget;
+ LIST_HEAD(event_list);
+
+ starget = container_of(work, struct scsi_target, event_work);
+
+ while (1) {
+ struct starget_event *evt;
+ struct list_head *this, *tmp;
+ unsigned long flags;
+
+ spin_lock_irqsave(&starget->list_lock, flags);
+ list_splice_init(&starget->event_list, &event_list);
+ spin_unlock_irqrestore(&starget->list_lock, flags);
+
+ if (list_empty(&event_list))
+ break;
+
+ list_for_each_safe(this, tmp, &event_list) {
+ evt = list_entry(this, struct starget_event, node);
+ list_del(&evt->node);
+ starget_evt_emit(starget, evt);
+ kfree(evt);
+ }
+ }
+}
+
+/**
+ * starget_evt_send - send asserted event to uevent thread
+ * @starget: scsi_target event occurred on
+ * @evt: event to send
+ *
+ * Assert scsi target event asynchronously.
+ */
+void starget_evt_send(struct scsi_target *starget, struct starget_event *evt)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&starget->list_lock, flags);
+ list_add_tail(&evt->node, &starget->event_list);
+ schedule_work(&starget->event_work);
+ spin_unlock_irqrestore(&starget->list_lock, flags);
+}
+EXPORT_SYMBOL_GPL(starget_evt_send);
+
+/**
+ * starget_evt_alloc - allocate a new scsi_target event
+ * @evt_type: type of event to allocate
+ * @gfpflags: GFP flags for allocation
+ *
+ * Allocates and returns a new starget_event.
+ */
+struct starget_event *starget_evt_alloc(enum scsi_target_event evt_type,
+ gfp_t gfpflags)
+{
+ struct starget_event *evt = kzalloc(sizeof(struct starget_event),
+ gfpflags);
+ if (!evt)
+ return NULL;
+
+ evt->evt_type = evt_type;
+ INIT_LIST_HEAD(&evt->node);
+
+ /* evt_type-specific initialization, if any */
+ switch (evt_type) {
+ case STARGET_EVT_LUN_CHANGE_REPORTED:
+ default:
+ /* do nothing */
+ break;
+ }
+
+ return evt;
+}
+EXPORT_SYMBOL_GPL(starget_evt_alloc);
+
+/**
+ * starget_evt_send_simple - send asserted event to uevent thread
+ * @starget: scsi_target event occurred on
+ * @evt_type: type of event to send
+ * @gfpflags: GFP flags for allocation
+ *
+ * Assert scsi target event asynchronously, given an event type.
+ */
+void starget_evt_send_simple(struct scsi_target *starget,
+enum scsi_target_event evt_type, gfp_t gfpflags)
+{
+ struct starget_event *evt = starget_evt_alloc(evt_type, gfpflags);
+ if (!evt) {
+ starget_printk(KERN_ERR, starget, "event %d eaten due to OOM\n",
+ evt_type);
+ return;
+ }
+
+ starget_evt_send(starget, evt);
+}
+EXPORT_SYMBOL_GPL(starget_evt_send_simple);
+
+#endif
+
/**
* scsi_internal_device_block - internal function to put a device temporarily
into the SDEV_BLOCK
[PATCH v3 6/6] [SCSI] Add sense and Unit Attention generation to scsi_debug
From: "Ewan D. Milne"
Added capability to scsi_debug to generate sense and Unit Attention
conditions to exercise the enhanced sense and Unit Attention handling.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_debug.c | 138 ++
1 file changed, 138 insertions(+)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 182d5a5..1d8c2bb 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -230,6 +230,7 @@ struct sdebug_dev_info {
char reset;
char stopped;
char used;
+ char sense_pending;
};
struct sdebug_host_info {
@@ -3050,10 +3051,27 @@ static ssize_t sdebug_max_luns_store(struct
device_driver * ddp,
const char * buf, size_t count)
{
int n;
+#ifdef CONFIG_SCSI_ENHANCED_UA
+ struct sdebug_host_info *sdbg_host;
+ struct sdebug_dev_info *devip;
+#endif
if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n >= 0)) {
scsi_debug_max_luns = n;
sdebug_max_tgts_luns();
+
+#ifdef CONFIG_SCSI_ENHANCED_UA
+ spin_lock(&sdebug_host_list_lock);
+ list_for_each_entry(sdbg_host, &sdebug_host_list, host_list) {
+ list_for_each_entry(devip, &sdbg_host->dev_info_list,
+ dev_list) {
+ mk_sense_buffer(devip, UNIT_ATTENTION,
+ 0x3f, 0x0e);
+ devip->sense_pending = 1;
+ }
+ }
+ spin_unlock(&sdebug_host_list_lock);
+#endif
return count;
}
return -EINVAL;
@@ -3100,12 +3118,28 @@ static ssize_t sdebug_virtual_gb_store(struct
device_driver * ddp,
const char * buf, size_t count)
{
int n;
+#ifdef CONFIG_SCSI_ENHANCED_UA
+ struct sdebug_host_info *sdbg_host;
+ struct sdebug_dev_info *devip;
+#endif
if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n >= 0)) {
scsi_debug_virtual_gb = n;
sdebug_capacity = get_sdebug_capacity();
+#ifdef CONFIG_SCSI_ENHANCED_UA
+ spin_lock(&sdebug_host_list_lock);
+ list_for_each_entry(sdbg_host, &sdebug_host_list, host_list) {
+ list_for_each_entry(devip, &sdbg_host->dev_info_list,
+ dev_list) {
+ mk_sense_buffer(devip, UNIT_ATTENTION,
+ 0x2a, 0x09);
+ devip->sense_pending = 1;
+ }
+ }
+ spin_unlock(&sdebug_host_list_lock);
+#endif
return count;
}
return -EINVAL;
@@ -3206,6 +3240,89 @@ static ssize_t sdebug_map_show(struct device_driver
*ddp, char *buf)
DRIVER_ATTR(map, S_IRUGO, sdebug_map_show, NULL);
+#ifdef CONFIG_SCSI_ENHANCED_UA
+static ssize_t sdebug_ua_overflow_store(struct device_driver *ddp,
+ const char *buf, size_t count)
+{
+ int n;
+ struct sdebug_host_info *sdbg_host;
+ struct sdebug_dev_info *devip;
+
+ if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n >= 0) &&
+scsi_debug_dsense) {
+ spin_lock(&sdebug_host_list_lock);
+ list_for_each_entry(sdbg_host, &sdebug_host_list, host_list) {
+ list_for_each_entry(devip, &sdbg_host->dev_info_list,
+ dev_list) {
+ mk_sense_buffer(devip, UNIT_ATTENTION,
+ 0, 0);
+ devip->sense_buff[7] = 8;
+ devip->sense_buff[8] = 0x02;
+ devip->sense_buff[9] = 0x06;
+ devip->sense_buff[12] = 0x01;
+ devip->sense_pending = 1;
+ }
+ }
+ spin_unlock(&sdebug_host_list_lock);
+ return count;
+ }
+ return -EINVAL;
+}
+DRIVER_ATTR(ua_overflow, S_IWUSR, NULL, sdebug_ua_overflow_store);
+
+static ssize_t sdebug_soft_threshold_reached_store(struct device_driver *ddp,
+ const char *buf,
+ size_t count)
+{
+ int n;
+ struct sdebug_host_info *sdbg_host;
+ struct sdebug_dev_info *devip;
+
+ if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n >= 0)) {
+ spin
[PATCH v3 2/6] [SCSI] Rename scsi_evt_xxx to sdev_evt_xxx and scsi_event to sdev_event
From: "Ewan D. Milne"
The names of the struct and some of the functions for scsi_device
events are too generic and do not match the comments in the source.
Changed all of the names to begin with sdev_ in order to avoid
naming issues and confusion with scsi_target events to be added.
Also changed name of sdev_evt_thread() to sdev_evt_work().
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_lib.c| 24
drivers/scsi/scsi_priv.h | 1 +
drivers/scsi/scsi_scan.c | 3 +--
drivers/scsi/scsi_sysfs.c | 4 ++--
include/scsi/scsi_device.h | 6 +++---
5 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6dfb978..53f074d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2175,9 +2175,9 @@ EXPORT_SYMBOL(scsi_device_set_state);
* @sdev: associated SCSI device
* @evt: event to emit
*
- * Send a single uevent (scsi_event) to the associated scsi_device.
+ * Send a single uevent (sdev_event) to the associated scsi_device.
*/
-static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt)
+static void sdev_evt_emit(struct scsi_device *sdev, struct sdev_event *evt)
{
int idx = 0;
char *envp[3];
@@ -2198,13 +2198,13 @@ static void scsi_evt_emit(struct scsi_device *sdev,
struct scsi_event *evt)
}
/**
- * sdev_evt_thread - send a uevent for each scsi event
+ * sdev_evt_work - send a uevent for each scsi event
* @work: work struct for scsi_device
*
* Dispatch queued events to their associated scsi_device kobjects
* as uevents.
*/
-void scsi_evt_thread(struct work_struct *work)
+void sdev_evt_work(struct work_struct *work)
{
struct scsi_device *sdev;
LIST_HEAD(event_list);
@@ -2212,7 +2212,7 @@ void scsi_evt_thread(struct work_struct *work)
sdev = container_of(work, struct scsi_device, event_work);
while (1) {
- struct scsi_event *evt;
+ struct sdev_event *evt;
struct list_head *this, *tmp;
unsigned long flags;
@@ -2224,9 +2224,9 @@ void scsi_evt_thread(struct work_struct *work)
break;
list_for_each_safe(this, tmp, &event_list) {
- evt = list_entry(this, struct scsi_event, node);
+ evt = list_entry(this, struct sdev_event, node);
list_del(&evt->node);
- scsi_evt_emit(sdev, evt);
+ sdev_evt_emit(sdev, evt);
kfree(evt);
}
}
@@ -2239,7 +2239,7 @@ void scsi_evt_thread(struct work_struct *work)
*
* Assert scsi device event asynchronously.
*/
-void sdev_evt_send(struct scsi_device *sdev, struct scsi_event *evt)
+void sdev_evt_send(struct scsi_device *sdev, struct sdev_event *evt)
{
unsigned long flags;
@@ -2265,12 +2265,12 @@ EXPORT_SYMBOL_GPL(sdev_evt_send);
* @evt_type: type of event to allocate
* @gfpflags: GFP flags for allocation
*
- * Allocates and returns a new scsi_event.
+ * Allocates and returns a new sdev_event.
*/
-struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
+struct sdev_event *sdev_evt_alloc(enum scsi_device_event evt_type,
gfp_t gfpflags)
{
- struct scsi_event *evt = kzalloc(sizeof(struct scsi_event), gfpflags);
+ struct sdev_event *evt = kzalloc(sizeof(struct sdev_event), gfpflags);
if (!evt)
return NULL;
@@ -2300,7 +2300,7 @@ EXPORT_SYMBOL_GPL(sdev_evt_alloc);
void sdev_evt_send_simple(struct scsi_device *sdev,
enum scsi_device_event evt_type, gfp_t gfpflags)
{
- struct scsi_event *evt = sdev_evt_alloc(evt_type, gfpflags);
+ struct sdev_event *evt = sdev_evt_alloc(evt_type, gfpflags);
if (!evt) {
sdev_printk(KERN_ERR, sdev, "event %d eaten due to OOM\n",
evt_type);
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 07ce3f5..7f00813 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -90,6 +90,7 @@ extern void scsi_exit_queue(void);
struct request_queue;
struct request;
extern struct kmem_cache *scsi_sdb_cache;
+extern void sdev_evt_work(struct work_struct *work);
/* scsi_proc.c */
#ifdef CONFIG_SCSI_PROC_FS
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 2e5fe58..47348ed 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -244,7 +244,6 @@ static struct scsi_device *scsi_alloc_sdev(struct
scsi_target *starget,
struct scsi_device *sdev;
int display_failure_msg = 1, ret;
struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
- extern void scsi_evt_thread(struct work_struct *work);
extern void scsi_requeue_
[PATCH v3 5/6] [SCSI] Add sysfs support for enhanced Unit Attention handling
From: "Ewan D. Milne"
Added support for additional scsi_device events in sysfs, as
well as support for scsi_target events. Also added "rescan"
node in scsi_target sysfs to permit targets to be rescanned
from udev rules in a more straightforward way.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_priv.h | 4 +-
drivers/scsi/scsi_scan.c | 29 +-
drivers/scsi/scsi_sysfs.c | 139 ++
3 files changed, 135 insertions(+), 37 deletions(-)
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index b237c8e..fa3366a97 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -136,7 +136,9 @@ extern int scsi_sysfs_add_host(struct Scsi_Host *);
extern int scsi_sysfs_register(void);
extern void scsi_sysfs_unregister(void);
extern void scsi_sysfs_device_initialize(struct scsi_device *);
-extern int scsi_sysfs_target_initialize(struct scsi_device *);
+extern void scsi_sysfs_target_initialize(struct scsi_target *,
+struct Scsi_Host *,
+struct device *parent);
extern struct scsi_transport_template blank_transport_template;
extern void __scsi_remove_device(struct scsi_device *);
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 1ad4287..1bbbc43 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -346,25 +346,6 @@ static void scsi_target_destroy(struct scsi_target
*starget)
put_device(dev);
}
-static void scsi_target_dev_release(struct device *dev)
-{
- struct device *parent = dev->parent;
- struct scsi_target *starget = to_scsi_target(dev);
-
- kfree(starget);
- put_device(parent);
-}
-
-static struct device_type scsi_target_type = {
- .name = "scsi_target",
- .release = scsi_target_dev_release,
-};
-
-int scsi_is_target_device(const struct device *dev)
-{
- return dev->type == &scsi_target_type;
-}
-EXPORT_SYMBOL(scsi_is_target_device);
static struct scsi_target *__scsi_find_target(struct device *parent,
int channel, uint id)
@@ -416,15 +397,11 @@ static struct scsi_target *scsi_alloc_target(struct
device *parent,
printk(KERN_ERR "%s: allocation failure\n", __func__);
return NULL;
}
- dev = &starget->dev;
- device_initialize(dev);
- starget->reap_ref = 1;
- dev->parent = get_device(parent);
- dev_set_name(dev, "target%d:%d:%d", shost->host_no, channel, id);
- dev->bus = &scsi_bus_type;
- dev->type = &scsi_target_type;
starget->id = id;
starget->channel = channel;
+ scsi_sysfs_target_initialize(starget, shost, parent);
+ dev = &starget->dev;
+ starget->reap_ref = 1;
starget->can_queue = 0;
INIT_LIST_HEAD(&starget->siblings);
INIT_LIST_HEAD(&starget->devices);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index ac0e96f..2a4f2c8 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -502,7 +502,7 @@ sdev_store_##field (struct device *dev, struct
device_attribute *attr, \
{ \
int ret;\
struct scsi_device *sdev; \
- ret = scsi_sdev_check_buf_bit(buf); \
+ ret = scsi_sysfs_check_buf_bit(buf);\
if (ret >= 0) { \
sdev = to_scsi_device(dev); \
sdev->field = ret; \
@@ -513,10 +513,10 @@ sdev_store_##field (struct device *dev, struct
device_attribute *attr,\
static DEVICE_ATTR(field, S_IRUGO | S_IWUSR, sdev_show_##field,
sdev_store_##field);
/*
- * scsi_sdev_check_buf_bit: return 0 if buf is "0", return 1 if buf is "1",
+ * scsi_sysfs_check_buf_bit: return 0 if buf is "0", return 1 if buf is "1",
* else return -EINVAL.
*/
-static int scsi_sdev_check_buf_bit(const char *buf)
+static int scsi_sysfs_check_buf_bit(const char *buf)
{
if ((buf[1] == '\0') || ((buf[1] == '\n') && (buf[2] == '\0'))) {
if (buf[0] == '1')
@@ -650,7 +650,8 @@ show_queue_type_field(struct device *dev, struct
device_attribute *attr,
static DEVICE_ATTR(queue_type, S_IRUGO, show_queue_type_field, NULL);
static ssize_t
-show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
char *buf)
+show_iostat_counterbits(struct device *dev, struct device_attribute
[PATCH v4 04/10] scsi: Change to use list_for_each_entry_safe
From: "Ewan D. Milne"
scsi_device_dev_release_usercontext() should be using
"list_for_each_entry_safe" instead of "list_for_each_safe".
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_sysfs.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 7394a77..34f7580 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -335,7 +335,7 @@ static void scsi_device_dev_release_usercontext(struct
work_struct *work)
struct scsi_device *sdev;
struct device *parent;
struct scsi_target *starget;
- struct list_head *this, *tmp;
+ struct scsi_event *evt, *next;
unsigned long flags;
sdev = container_of(work, struct scsi_device, ew.work);
@@ -352,10 +352,7 @@ static void scsi_device_dev_release_usercontext(struct
work_struct *work)
cancel_work_sync(&sdev->event_work);
- list_for_each_safe(this, tmp, &sdev->event_list) {
- struct scsi_event *evt;
-
- evt = list_entry(this, struct scsi_event, node);
+ list_for_each_entry_safe(evt, next, &sdev->event_list, node) {
list_del(&evt->node);
kfree(evt);
}
--
1.7.11.7
--
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 v4 03/10] scsi: Add missing newline to scsi_sysfs.c
From: "Ewan D. Milne"
show_iostat_counterbits() is obviously missing a newline in
the function declaration.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_sysfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 04c2a27..7394a77 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -647,7 +647,8 @@ show_queue_type_field(struct device *dev, struct
device_attribute *attr,
static DEVICE_ATTR(queue_type, S_IRUGO, show_queue_type_field, NULL);
static ssize_t
-show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
char *buf)
+show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
+ char *buf)
{
return snprintf(buf, 20, "%d\n", (int)sizeof(atomic_t) * 8);
}
--
1.7.11.7
--
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 v4 02/10] scsi: Correct size of envp[]
From: "Ewan D. Milne"
The envp[] array in scsi_evt_emit() only needs to have 2 entries.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_lib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f6499db..6585049 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2180,7 +2180,7 @@ EXPORT_SYMBOL(scsi_device_set_state);
static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt)
{
int idx = 0;
- char *envp[3];
+ char *envp[2];
switch (evt->evt_type) {
case SDEV_EVT_MEDIA_CHANGE:
--
1.7.11.7
--
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 v4 00/10] Enhanced Unit Attention handling
From: "Ewan D. Milne" This patch set adds changes to the SCSI mid-layer, sysfs and scsi_debug to provide enhanced support for Unit Attention conditions. There was some discussion about this a couple of years ago on the linux-scsi mailing list: http://marc.info/?l=linux-scsi&m=129702506514742&w=2 Although one approach is to send all SCSI sense data to a userspace daemon for processing, this patch set does not take that approach due to the difficulty in reliably delivering all of the data. An interesting UA condition might not be delivered due to a flood of media errors, for example. The mechanism used is to flag when certain UA ASC/ASCQ codes are received that report asynchronous changes to the storage device configuration. An appropriate uevent is then generated for the scsi_device or scsi_target object. The expecting_cc_ua flag is used for REPORTED LUNS DATA HAS CHANGED unit attention conditions to avoid generating duplicate events on multiple LUNs. Changes made since earlier v3 version: - Put fixes to existing code in separate individual patches - Removed UA queue overflow condition reporting - Removed delayed_work aggregation mechanism for events - Eliminated separate scsi_device and scsi_target mechanisms - Changed to use a single environment variable for uevents - Clear expecting_cc_ua on successful commands - Added use of expecting_cc_ua for REPORTED LUNS DATA HAS CHANGED Changes made since earlier v2 version: - Remove patch 1/8 "Generate uevent on sd capacity change" - Remove patch 8/8 "Streamline detection of FM/EOM/ILI status" - Changed scsi_debug to not generate UA on INQUIRY or REPORT_LUNS - Changed scsi_debug to only report UA queue overflow condition if dsense=1, as descriptor format sense data is needed Changes made since earlier RFC version: - Remove patch 1/9 "Detect overflow of sense data buffer" Some scsi_debug changes in this patch were moved to patch 7/8 - Corrected Kconfig help text - Change name of "sdev_evt_thread" to "sdev_evt_work" - Change name of "starget_evt_thread" to "starget_evt_work" - Pull code out of scsi_check_sense() that handles UAs into an exported function so that drivers can report conditions received asynchronously Thanks to everyone for the comments on this patch series. Ewan D. Milne (10): scsi: Fix incorrect function name in comment scsi: Correct size of envp[] scsi: Add missing newline to scsi_sysfs.c scsi: Change to use list_for_each_entry_safe scsi: Rename scsi_evt_thread() to scsi_evt_work() scsi: Move schedule_work() call to be outside lock scsi: Clear expecting_cc_ua on successful commands scsi: Generate uevents on certain unit attention codes scsi_debug: Add optional unit attention reporting scsi: Added scsi_target rescan capability to sysfs drivers/scsi/scsi_debug.c | 131 + drivers/scsi/scsi_error.c | 125 ++ drivers/scsi/scsi_lib.c| 52 +++--- drivers/scsi/scsi_priv.h | 5 +- drivers/scsi/scsi_scan.c | 33 ++-- drivers/scsi/scsi_sysfs.c | 81 +--- include/scsi/scsi_device.h | 11 +++- 7 files changed, 372 insertions(+), 66 deletions(-) -- 1.7.11.7 -- 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 v4 01/10] scsi: Fix incorrect function name in comment
From: "Ewan D. Milne" The function name is "scsi_evt_emit", not "sdev_evt_emit". Signed-off-by: Ewan D. Milne --- drivers/scsi/scsi_lib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 6dfb978..f6499db 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2171,7 +2171,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state) EXPORT_SYMBOL(scsi_device_set_state); /** - * sdev_evt_emit - emit a single SCSI device uevent + * scsi_evt_emit - emit a single SCSI device uevent * @sdev: associated SCSI device * @evt: event to emit * -- 1.7.11.7 -- 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 v4 07/10] scsi: Clear expecting_cc_ua on successful commands
From: "Ewan D. Milne" If a device does not report a unit attention as expected, clear the expecting_cc_ua flag so that we will not suppress a future unit attention condition that is *not* expected. INQUIRY and REPORT LUNS commands should not do this, however, because they do not report pending unit attention conditions. Signed-off-by: Ewan D. Milne --- drivers/scsi/scsi_error.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index d0f71e5..96707a6 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -1540,6 +1540,13 @@ int scsi_decide_disposition(struct scsi_cmnd *scmd) */ return ADD_TO_MLQUEUE; case GOOD: + /* +* Reset expecting_cc_ua for all commands except INQUIRY and +* REPORT LUNS, because if we didn't get a UA on this command +* as expected, then we don't want to suppress a subsequent one. +*/ + if (scmd->cmnd[0] != INQUIRY && scmd->cmnd[0] != REPORT_LUNS) + scmd->device->expecting_cc_ua = 0; scsi_handle_queue_ramp_up(scmd->device); case COMMAND_TERMINATED: return SUCCESS; -- 1.7.11.7 -- 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 v4 10/10] scsi: Added scsi_target rescan capability to sysfs
From: "Ewan D. Milne"
Add a "rescan" attribute to sysfs for scsi_target objects,
to permit them to be scanned for LUN changes (e.g. from udev).
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_priv.h | 4 +++-
drivers/scsi/scsi_scan.c | 30 +++
drivers/scsi/scsi_sysfs.c | 61 +++
3 files changed, 67 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index ed80f21..7c33799 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -131,7 +131,9 @@ extern int scsi_sysfs_add_host(struct Scsi_Host *);
extern int scsi_sysfs_register(void);
extern void scsi_sysfs_unregister(void);
extern void scsi_sysfs_device_initialize(struct scsi_device *);
-extern int scsi_sysfs_target_initialize(struct scsi_device *);
+extern void scsi_sysfs_target_initialize(struct scsi_target *,
+struct Scsi_Host *,
+struct device *parent);
extern struct scsi_transport_template blank_transport_template;
extern void __scsi_remove_device(struct scsi_device *);
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 0adfecb..243c8b4 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -343,26 +343,6 @@ static void scsi_target_destroy(struct scsi_target
*starget)
put_device(dev);
}
-static void scsi_target_dev_release(struct device *dev)
-{
- struct device *parent = dev->parent;
- struct scsi_target *starget = to_scsi_target(dev);
-
- kfree(starget);
- put_device(parent);
-}
-
-static struct device_type scsi_target_type = {
- .name = "scsi_target",
- .release = scsi_target_dev_release,
-};
-
-int scsi_is_target_device(const struct device *dev)
-{
- return dev->type == &scsi_target_type;
-}
-EXPORT_SYMBOL(scsi_is_target_device);
-
static struct scsi_target *__scsi_find_target(struct device *parent,
int channel, uint id)
{
@@ -413,15 +393,11 @@ static struct scsi_target *scsi_alloc_target(struct
device *parent,
printk(KERN_ERR "%s: allocation failure\n", __func__);
return NULL;
}
- dev = &starget->dev;
- device_initialize(dev);
- starget->reap_ref = 1;
- dev->parent = get_device(parent);
- dev_set_name(dev, "target%d:%d:%d", shost->host_no, channel, id);
- dev->bus = &scsi_bus_type;
- dev->type = &scsi_target_type;
starget->id = id;
starget->channel = channel;
+ scsi_sysfs_target_initialize(starget, shost, parent);
+ dev = &starget->dev;
+ starget->reap_ref = 1;
starget->can_queue = 0;
INIT_LIST_HEAD(&starget->siblings);
INIT_LIST_HEAD(&starget->devices);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index d5d86b2..212b43a 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1129,6 +1129,67 @@ int scsi_is_sdev_device(const struct device *dev)
}
EXPORT_SYMBOL(scsi_is_sdev_device);
+static ssize_t
+starget_store_rescan_field(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct scsi_target *starget = to_scsi_target(dev);
+
+ scsi_scan_target(starget->dev.parent, starget->channel, starget->id,
+SCAN_WILD_CARD, 1);
+ return count;
+}
+/* DEVICE_ATTR(rescan) clashes with dev_attr_rescan for sdev */
+struct device_attribute dev_attr_trescan =
+ __ATTR(rescan, S_IWUSR, NULL, starget_store_rescan_field);
+
+static struct attribute *scsi_target_attrs[] = {
+ &dev_attr_trescan.attr,
+ NULL
+};
+
+static struct attribute_group scsi_target_attr_group = {
+ .attrs =scsi_target_attrs,
+};
+
+static const struct attribute_group *scsi_target_attr_groups[] = {
+ &scsi_target_attr_group,
+ NULL
+};
+
+static void scsi_target_dev_release(struct device *dev)
+{
+ struct device *parent = dev->parent;
+ struct scsi_target *starget = to_scsi_target(dev);
+
+ kfree(starget);
+ put_device(parent);
+}
+
+static struct device_type scsi_target_type = {
+ .name = "scsi_target",
+ .release = scsi_target_dev_release,
+ .groups = scsi_target_attr_groups,
+};
+
+int scsi_is_target_device(const struct device *dev)
+{
+ return dev->type == &scsi_target_type;
+}
+EXPORT_SYMBOL(scsi_is_target_device);
+
+void scsi_sysfs_target_initialize(struct scsi_target *starget,
+ struct Scsi_Host *shost,
+ struct device *parent)
+{
+ device_initialize(&starget->dev);
+ starget->dev.
[PATCH v4 09/10] scsi_debug: Add optional unit attention reporting
From: "Ewan D. Milne"
Added "report_ua" module parameter to control reporting
of unit attention conditions when the number of LUNs is
changed, or the virtual_gb size of the device is changed.
Also added capability to generate unit attention conditions:
38 07 - THIN PROVISIONING SOFT THRESHOLD REACHED
2A 01 - MODE PARAMETERS CHANGED
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_debug.c | 131 ++
1 file changed, 131 insertions(+)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 182d5a5..738b197 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -111,6 +111,7 @@ static const char * scsi_debug_version_date = "20100324";
#define DEF_PTYPE 0
#define DEF_SCSI_LEVEL 5/* INQUIRY, byte2 [5->SPC-3] */
#define DEF_SECTOR_SIZE 512
+#define DEF_REPORT_UA 0
#define DEF_UNMAP_ALIGNMENT 0
#define DEF_UNMAP_GRANULARITY 1
#define DEF_UNMAP_MAX_BLOCKS 0x
@@ -182,6 +183,7 @@ static int scsi_debug_physblk_exp = DEF_PHYSBLK_EXP;
static int scsi_debug_ptype = DEF_PTYPE; /* SCSI peripheral type (0==disk) */
static int scsi_debug_scsi_level = DEF_SCSI_LEVEL;
static int scsi_debug_sector_size = DEF_SECTOR_SIZE;
+static int scsi_debug_report_ua = DEF_REPORT_UA;
static int scsi_debug_virtual_gb = DEF_VIRTUAL_GB;
static int scsi_debug_vpd_use_hostno = DEF_VPD_USE_HOSTNO;
static unsigned int scsi_debug_lbpu = DEF_LBPU;
@@ -230,6 +232,8 @@ struct sdebug_dev_info {
char reset;
char stopped;
char used;
+ char luns_changed;
+ char sense_pending;
};
struct sdebug_host_info {
@@ -268,6 +272,7 @@ static int num_host_resets = 0;
static int dix_writes;
static int dix_reads;
static int dif_errors;
+static int luns_changed;
static DEFINE_SPINLOCK(queued_arr_lock);
static DEFINE_RWLOCK(atomic_rw);
@@ -2756,6 +2761,7 @@ module_param_named(physblk_exp, scsi_debug_physblk_exp,
int, S_IRUGO);
module_param_named(ptype, scsi_debug_ptype, int, S_IRUGO | S_IWUSR);
module_param_named(scsi_level, scsi_debug_scsi_level, int, S_IRUGO);
module_param_named(sector_size, scsi_debug_sector_size, int, S_IRUGO);
+module_param_named(report_ua, scsi_debug_report_ua, int, S_IRUGO | S_IWUSR);
module_param_named(unmap_alignment, scsi_debug_unmap_alignment, int, S_IRUGO);
module_param_named(unmap_granularity, scsi_debug_unmap_granularity, int,
S_IRUGO);
module_param_named(unmap_max_blocks, scsi_debug_unmap_max_blocks, int,
S_IRUGO);
@@ -2798,6 +2804,7 @@ MODULE_PARM_DESC(physblk_exp, "physical block exponent
(def=0)");
MODULE_PARM_DESC(ptype, "SCSI peripheral type(def=0[disk])");
MODULE_PARM_DESC(scsi_level, "SCSI level to simulate(def=5[SPC-3])");
MODULE_PARM_DESC(sector_size, "logical block size in bytes (def=512)");
+MODULE_PARM_DESC(report_ua, "report unit attentions when reconfigured
(def=0)");
MODULE_PARM_DESC(unmap_alignment, "lowest aligned thin provisioning lba
(def=0)");
MODULE_PARM_DESC(unmap_granularity, "thin provisioning granularity in blocks
(def=1)");
MODULE_PARM_DESC(unmap_max_blocks, "max # of blocks can be unmapped in one cmd
(def=0x)");
@@ -3050,10 +3057,37 @@ static ssize_t sdebug_max_luns_store(struct
device_driver * ddp,
const char * buf, size_t count)
{
int n;
+ struct sdebug_host_info *sdbg_host;
+ struct sdebug_dev_info *devip;
if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n >= 0)) {
scsi_debug_max_luns = n;
sdebug_max_tgts_luns();
+
+ if (!scsi_debug_report_ua)
+ return count;
+
+ /*
+* SPC-3 behavior is to report a UNIT ATTENTION with ASC/ASCQ
+* REPORTED LUNS DATA HAS CHANGED on every LUN on the target,
+* until a REPORT LUNS command is received. SPC-4 behavior is
+* to report it only once. NOTE: scsi_debug_scsi_level does
+* not use the same values as struct scsi_device->scsi_level.
+*/
+ if (scsi_debug_scsi_level == 5) { /* SPC-3 */
+ spin_lock(&sdebug_host_list_lock);
+ list_for_each_entry(sdbg_host, &sdebug_host_list,
+ host_list) {
+ list_for_each_entry(devip,
+ &sdbg_host->dev_info_list,
+ dev_list) {
+ devip->luns_changed = 1;
+ }
+ }
+ spin_unlock(&sdebug_host_list_lock);
+ } else if (scsi_debug_scsi_level > 5)
+
[PATCH v4 06/10] scsi: Move schedule_work() call to be outside lock
From: "Ewan D. Milne" The call to schedule_work() in sdev_evt_send() should not be made while the sdev->list_lock is held. Signed-off-by: Ewan D. Milne --- drivers/scsi/scsi_lib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 97699a5..f871ecf 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2255,8 +2255,8 @@ void sdev_evt_send(struct scsi_device *sdev, struct scsi_event *evt) spin_lock_irqsave(&sdev->list_lock, flags); list_add_tail(&evt->node, &sdev->event_list); - schedule_work(&sdev->event_work); spin_unlock_irqrestore(&sdev->list_lock, flags); + schedule_work(&sdev->event_work); } EXPORT_SYMBOL_GPL(sdev_evt_send); -- 1.7.11.7 -- 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 v4 05/10] scsi: Rename scsi_evt_thread() to scsi_evt_work()
From: "Ewan D. Milne"
The scsi_evt_thread() function is not actually a thread, it is
a work function. So it should be named scsi_evt_work() instead.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_lib.c | 4 ++--
drivers/scsi/scsi_priv.h | 1 +
drivers/scsi/scsi_scan.c | 3 +--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6585049..97699a5 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2198,13 +2198,13 @@ static void scsi_evt_emit(struct scsi_device *sdev,
struct scsi_event *evt)
}
/**
- * sdev_evt_thread - send a uevent for each scsi event
+ * scsi_evt_work - send a uevent for each scsi event
* @work: work struct for scsi_device
*
* Dispatch queued events to their associated scsi_device kobjects
* as uevents.
*/
-void scsi_evt_thread(struct work_struct *work)
+void scsi_evt_work(struct work_struct *work)
{
struct scsi_device *sdev;
LIST_HEAD(event_list);
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 07ce3f5..ed80f21 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -90,6 +90,7 @@ extern void scsi_exit_queue(void);
struct request_queue;
struct request;
extern struct kmem_cache *scsi_sdb_cache;
+extern void scsi_evt_work(struct work_struct *work);
/* scsi_proc.c */
#ifdef CONFIG_SCSI_PROC_FS
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 2e5fe58..0adfecb 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -244,7 +244,6 @@ static struct scsi_device *scsi_alloc_sdev(struct
scsi_target *starget,
struct scsi_device *sdev;
int display_failure_msg = 1, ret;
struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
- extern void scsi_evt_thread(struct work_struct *work);
extern void scsi_requeue_run_queue(struct work_struct *work);
sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size,
@@ -267,7 +266,7 @@ static struct scsi_device *scsi_alloc_sdev(struct
scsi_target *starget,
INIT_LIST_HEAD(&sdev->starved_entry);
INIT_LIST_HEAD(&sdev->event_list);
spin_lock_init(&sdev->list_lock);
- INIT_WORK(&sdev->event_work, scsi_evt_thread);
+ INIT_WORK(&sdev->event_work, scsi_evt_work);
INIT_WORK(&sdev->requeue_work, scsi_requeue_run_queue);
sdev->sdev_gendev.parent = get_device(&starget->dev);
--
1.7.11.7
--
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 v4 08/10] scsi: Generate uevents on certain unit attention codes
From: "Ewan D. Milne"
Generate a uevent when the following Unit Attention ASC/ASCQ
codes are received:
2A/01 MODE PARAMETERS CHANGED
2A/09 CAPACITY DATA HAS CHANGED
38/07 THIN PROVISIONING SOFT THRESHOLD REACHED
3F/03 INQUIRY DATA HAS CHANGED
3F/0E REPORTED LUNS DATA HAS CHANGED
Log kernel messages when the following Unit Attention ASC/ASCQ
codes are received that are not as specific as those above:
2A/xx PARAMETERS CHANGED
3F/xx TARGET OPERATING CONDITIONS HAVE CHANGED
Added logic to set expecting_cc_ua for other LUNs on SPC-3 devices
after REPORTED LUNS DATA HAS CHANGED is received, so that duplicate
uevents are not generated, and clear expecting_cc_ua when a
REPORT LUNS command completes, in accordance with the SPC-3
specification regarding reporting of the 3F 0E ASC/ASCQ UA.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_error.c | 118 +
drivers/scsi/scsi_lib.c| 42 ++--
drivers/scsi/scsi_sysfs.c | 10
include/scsi/scsi_device.h | 11 -
4 files changed, 156 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 96707a6..227041a 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -222,6 +222,86 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host
*shost,
}
#endif
+ /**
+ * scsi_report_lun_change - Set flag on all *other* devices on the same target
+ * to indicate that a UNIT ATTENTION is expected.
+ * Only do this for SPC-3 devices, however.
+ * @sdev: Device reporting the UNIT ATTENTION
+ */
+static void scsi_report_lun_change(struct scsi_device *sdev)
+{
+ struct Scsi_Host *shost = sdev->host;
+ struct scsi_device *tmp_sdev;
+
+ if (sdev->scsi_level == SCSI_SPC_3)
+ shost_for_each_device(tmp_sdev, shost) {
+ if (tmp_sdev->channel == sdev->channel &&
+ tmp_sdev->id == sdev->id &&
+ tmp_sdev != sdev)
+ tmp_sdev->expecting_cc_ua = 1;
+ }
+}
+
+/**
+ * scsi_report_sense - Examine scsi sense information and log messages for
+ *certain conditions, also issue uevents for some of them.
+ * @sshdr: sshdr to be examined
+ */
+static void scsi_report_sense(struct scsi_device *sdev,
+ struct scsi_sense_hdr *sshdr)
+{
+ enum scsi_device_event evt_type = SDEV_EVT_MAXBITS; /* i.e. none */
+ unsigned long flags;
+
+ if (sshdr->sense_key == UNIT_ATTENTION) {
+ if (sshdr->asc == 0x3f && sshdr->ascq == 0x03) {
+ evt_type = SDEV_EVT_INQUIRY_CHANGE_REPORTED;
+ sdev_printk(KERN_WARNING, sdev,
+ "Inquiry data has changed");
+ } else if (sshdr->asc == 0x3f && sshdr->ascq == 0x0e) {
+ evt_type = SDEV_EVT_LUN_CHANGE_REPORTED;
+ scsi_report_lun_change(sdev);
+ sdev_printk(KERN_WARNING, sdev,
+ "Warning! Received an indication that the "
+ "LUN assignments on this target have "
+ "changed. The Linux SCSI layer does not "
+ "automatically remap LUN assignments.\n");
+ } else if (sshdr->asc == 0x3f)
+ sdev_printk(KERN_WARNING, sdev,
+ "Warning! Received an indication that the "
+ "operating parameters on this target have "
+ "changed. The Linux SCSI layer does not "
+ "automatically adjust these parameters.\n");
+
+ if (sshdr->asc == 0x38 && sshdr->ascq == 0x07) {
+ evt_type = SDEV_EVT_SOFT_THRESHOLD_REACHED_REPORTED;
+ sdev_printk(KERN_WARNING, sdev,
+ "Warning! Received an indication that the "
+ "LUN reached a thin provisioning soft "
+ "threshold.\n");
+ }
+
+ if (sshdr->asc == 0x2a && sshdr->ascq == 0x01) {
+ evt_type = SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED;
+ sdev_printk(KERN_WARNING, sdev,
+ "Mode parameters changed");
+ } else if (sshdr->asc == 0x2a && sshdr->ascq == 0x09) {
+ evt_type = SDEV_
[PATCH v5] scsi: Generate uevents on certain unit attention codes
From: "Ewan D. Milne"
Generate a uevent when the following Unit Attention ASC/ASCQ
codes are received:
2A/01 MODE PARAMETERS CHANGED
2A/09 CAPACITY DATA HAS CHANGED
38/07 THIN PROVISIONING SOFT THRESHOLD REACHED
3F/03 INQUIRY DATA HAS CHANGED
3F/0E REPORTED LUNS DATA HAS CHANGED
Log kernel messages when the following Unit Attention ASC/ASCQ
codes are received that are not as specific as those above:
2A/xx PARAMETERS CHANGED
3F/xx TARGET OPERATING CONDITIONS HAVE CHANGED
Added logic to set expecting_lun_change for other LUNs on the target
after REPORTED LUNS DATA HAS CHANGED is received, so that duplicate
uevents are not generated, and clear expecting_lun_change when a
REPORT LUNS command completes, in accordance with the SPC-3
specification regarding reporting of the 3F 0E ASC/ASCQ UA.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_error.c | 106 -
drivers/scsi/scsi_lib.c| 26 ++-
drivers/scsi/scsi_sysfs.c | 10 +
include/scsi/scsi_device.h | 13 +-
4 files changed, 133 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index d0f71e5..598afd9 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -222,6 +222,80 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host
*shost,
}
#endif
+ /**
+ * scsi_report_lun_change - Set flag on all *other* devices on the same target
+ * to indicate that a UNIT ATTENTION is expected.
+ * @sdev: Device reporting the UNIT ATTENTION
+ */
+static void scsi_report_lun_change(struct scsi_device *sdev)
+{
+ /*
+* Prior to SPC-4, every device will report a 3F 0E unit attention once
+* until a REPORT LUNS command is received. SPC-4 targets only report
+* the unit attention once on a single device.
+*/
+ if (sdev->scsi_level <= SCSI_SPC_3)
+ sdev->sdev_target->expecting_lun_change = 1;
+}
+
+/**
+ * scsi_report_sense - Examine scsi sense information and log messages for
+ *certain conditions, also issue uevents for some of them.
+ * @sshdr: sshdr to be examined
+ */
+static void scsi_report_sense(struct scsi_device *sdev,
+ struct scsi_sense_hdr *sshdr)
+{
+ enum scsi_device_event evt_type = SDEV_EVT_MAXBITS; /* i.e. none */
+ unsigned long flags;
+
+ if (sshdr->sense_key == UNIT_ATTENTION) {
+ if (sshdr->asc == 0x3f && sshdr->ascq == 0x03) {
+ evt_type = SDEV_EVT_INQUIRY_CHANGE_REPORTED;
+ sdev_printk(KERN_WARNING, sdev,
+ "Inquiry data has changed");
+ } else if (sshdr->asc == 0x3f && sshdr->ascq == 0x0e) {
+ evt_type = SDEV_EVT_LUN_CHANGE_REPORTED;
+ scsi_report_lun_change(sdev);
+ sdev_printk(KERN_WARNING, sdev,
+ "Warning! Received an indication that the "
+ "LUN assignments on this target have "
+ "changed. The Linux SCSI layer does not "
+ "automatically remap LUN assignments.\n");
+ } else if (sshdr->asc == 0x3f)
+ sdev_printk(KERN_WARNING, sdev,
+ "Warning! Received an indication that the "
+ "operating parameters on this target have "
+ "changed. The Linux SCSI layer does not "
+ "automatically adjust these parameters.\n");
+
+ if (sshdr->asc == 0x38 && sshdr->ascq == 0x07) {
+ evt_type = SDEV_EVT_SOFT_THRESHOLD_REACHED_REPORTED;
+ sdev_printk(KERN_WARNING, sdev,
+ "Warning! Received an indication that the "
+ "LUN reached a thin provisioning soft "
+ "threshold.\n");
+ }
+
+ if (sshdr->asc == 0x2a && sshdr->ascq == 0x01) {
+ evt_type = SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED;
+ sdev_printk(KERN_WARNING, sdev,
+ "Mode parameters changed");
+ } else if (sshdr->asc == 0x2a && sshdr->ascq == 0x09) {
+ evt_type = SDEV_EVT_CAPACITY_CHANGE_REPORTED;
+ sdev_printk(KERN_WARNING, sdev,
+ "Capacity data has changed");
+ } else if (sshdr-&
[PATCH 0/5] Patches to clean up SCSI code
From: "Ewan D. Milne" This is a set of 5 patches to clean up the SCSI code. They consist of various things found during development and review of some changes for Unit Attention handling (which was submitted as a separate patch). Ewan D. Milne (5): scsi: Fix incorrect function name in comment scsi: Correct size of envp[] scsi: Add missing newline to scsi_sysfs.c scsi: Change to use list_for_each_entry_safe scsi: Rename scsi_evt_thread() to scsi_evt_work() drivers/scsi/scsi_lib.c | 8 drivers/scsi/scsi_priv.h | 1 + drivers/scsi/scsi_scan.c | 3 +-- drivers/scsi/scsi_sysfs.c | 10 -- 4 files changed, 10 insertions(+), 12 deletions(-) -- 1.7.11.7 -- 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 1/5] scsi: Fix incorrect function name in comment
From: "Ewan D. Milne" The function name is "scsi_evt_emit", not "sdev_evt_emit". Signed-off-by: Ewan D. Milne --- drivers/scsi/scsi_lib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 6dfb978..f6499db 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2171,7 +2171,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state) EXPORT_SYMBOL(scsi_device_set_state); /** - * sdev_evt_emit - emit a single SCSI device uevent + * scsi_evt_emit - emit a single SCSI device uevent * @sdev: associated SCSI device * @evt: event to emit * -- 1.7.11.7 -- 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 4/5] scsi: Change to use list_for_each_entry_safe
From: "Ewan D. Milne"
scsi_device_dev_release_usercontext() should be using
"list_for_each_entry_safe" instead of "list_for_each_safe".
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_sysfs.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 7394a77..34f7580 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -335,7 +335,7 @@ static void scsi_device_dev_release_usercontext(struct
work_struct *work)
struct scsi_device *sdev;
struct device *parent;
struct scsi_target *starget;
- struct list_head *this, *tmp;
+ struct scsi_event *evt, *next;
unsigned long flags;
sdev = container_of(work, struct scsi_device, ew.work);
@@ -352,10 +352,7 @@ static void scsi_device_dev_release_usercontext(struct
work_struct *work)
cancel_work_sync(&sdev->event_work);
- list_for_each_safe(this, tmp, &sdev->event_list) {
- struct scsi_event *evt;
-
- evt = list_entry(this, struct scsi_event, node);
+ list_for_each_entry_safe(evt, next, &sdev->event_list, node) {
list_del(&evt->node);
kfree(evt);
}
--
1.7.11.7
--
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 3/5] scsi: Add missing newline to scsi_sysfs.c
From: "Ewan D. Milne"
show_iostat_counterbits() is obviously missing a newline in
the function declaration.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_sysfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 04c2a27..7394a77 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -647,7 +647,8 @@ show_queue_type_field(struct device *dev, struct
device_attribute *attr,
static DEVICE_ATTR(queue_type, S_IRUGO, show_queue_type_field, NULL);
static ssize_t
-show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
char *buf)
+show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
+ char *buf)
{
return snprintf(buf, 20, "%d\n", (int)sizeof(atomic_t) * 8);
}
--
1.7.11.7
--
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 5/5] scsi: Rename scsi_evt_thread() to scsi_evt_work()
From: "Ewan D. Milne"
The scsi_evt_thread() function is not actually a thread, it is
a work function. So it should be named scsi_evt_work() instead.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_lib.c | 4 ++--
drivers/scsi/scsi_priv.h | 1 +
drivers/scsi/scsi_scan.c | 3 +--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6585049..97699a5 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2198,13 +2198,13 @@ static void scsi_evt_emit(struct scsi_device *sdev,
struct scsi_event *evt)
}
/**
- * sdev_evt_thread - send a uevent for each scsi event
+ * scsi_evt_work - send a uevent for each scsi event
* @work: work struct for scsi_device
*
* Dispatch queued events to their associated scsi_device kobjects
* as uevents.
*/
-void scsi_evt_thread(struct work_struct *work)
+void scsi_evt_work(struct work_struct *work)
{
struct scsi_device *sdev;
LIST_HEAD(event_list);
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 07ce3f5..ed80f21 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -90,6 +90,7 @@ extern void scsi_exit_queue(void);
struct request_queue;
struct request;
extern struct kmem_cache *scsi_sdb_cache;
+extern void scsi_evt_work(struct work_struct *work);
/* scsi_proc.c */
#ifdef CONFIG_SCSI_PROC_FS
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 2e5fe58..0adfecb 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -244,7 +244,6 @@ static struct scsi_device *scsi_alloc_sdev(struct
scsi_target *starget,
struct scsi_device *sdev;
int display_failure_msg = 1, ret;
struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
- extern void scsi_evt_thread(struct work_struct *work);
extern void scsi_requeue_run_queue(struct work_struct *work);
sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size,
@@ -267,7 +266,7 @@ static struct scsi_device *scsi_alloc_sdev(struct
scsi_target *starget,
INIT_LIST_HEAD(&sdev->starved_entry);
INIT_LIST_HEAD(&sdev->event_list);
spin_lock_init(&sdev->list_lock);
- INIT_WORK(&sdev->event_work, scsi_evt_thread);
+ INIT_WORK(&sdev->event_work, scsi_evt_work);
INIT_WORK(&sdev->requeue_work, scsi_requeue_run_queue);
sdev->sdev_gendev.parent = get_device(&starget->dev);
--
1.7.11.7
--
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 2/5] scsi: Correct size of envp[]
From: "Ewan D. Milne"
The envp[] array in scsi_evt_emit() only needs to have 2 entries.
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_lib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f6499db..6585049 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2180,7 +2180,7 @@ EXPORT_SYMBOL(scsi_device_set_state);
static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt)
{
int idx = 0;
- char *envp[3];
+ char *envp[2];
switch (evt->evt_type) {
case SDEV_EVT_MEDIA_CHANGE:
--
1.7.11.7
--
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
[TRIVIAL PATCH 2/2] scsi: Remove unused variable "flags" from scsi_report_sense()
From: "Ewan D. Milne"
Signed-off-by: Ewan D. Milne
---
drivers/scsi/scsi_error.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index cf6fd20..7f95236 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -248,7 +248,6 @@ static void scsi_report_sense(struct scsi_device *sdev,
struct scsi_sense_hdr *sshdr)
{
enum scsi_device_event evt_type = SDEV_EVT_MAXBITS; /* i.e. none */
- unsigned long flags;
if (sshdr->sense_key == UNIT_ATTENTION) {
if (sshdr->asc == 0x3f && sshdr->ascq == 0x03) {
--
1.7.11.7
--
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
[TRIVIAL PATCH 1/2] scsi: Add missing @sdev parameter description to scsi_report_sense()
From: "Ewan D. Milne" Signed-off-by: Ewan D. Milne --- drivers/scsi/scsi_error.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 598afd9..cf6fd20 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -241,6 +241,7 @@ static void scsi_report_lun_change(struct scsi_device *sdev) /** * scsi_report_sense - Examine scsi sense information and log messages for *certain conditions, also issue uevents for some of them. + * @sdev: Device reporting sense information * @sshdr: sshdr to be examined */ static void scsi_report_sense(struct scsi_device *sdev, -- 1.7.11.7 -- 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
[TRIVIAL PATCH 0/2] Fix warnings in kernel build
From: "Ewan D. Milne" The patch to add uevents for certain Unit Attention codes had a couple of problems which were found by a kbuild test robot. Sorry about that, I will be more careful next time. Ewan D. Milne (2): scsi: Add missing @sdev parameter description to scsi_report_sense() scsi: Remove unused variable "flags" from scsi_report_sense() drivers/scsi/scsi_error.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.7.11.7 -- 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
