Re: [PATCH v7 5/5] scsi: ufs-qcom-ice: add Inline Crypto Engine (ICE) support for UFS
Yaniv, On Thu, 2015-01-15 at 16:32 +0200, Yaniv Gardi wrote: > From: Yaniv Gardi > > In-order to enhance storage encryption performance, > an Inline Cryptographic Engine is introduced to UFS. > This patch adds in-line encryption capabilities to the UFS > driver. > > Signed-off-by: Yaniv Gardi This patch became commit 8805ccd069b7 ("ufs-qcom-ice: add Inline Crypto Engine (ICE) support for UFS") in today's linux-next (ie, next-20150123). I noticed because a script I use to check linux-next spotted a problem with it. > --- > drivers/scsi/ufs/Kconfig| 12 + > drivers/scsi/ufs/Makefile | 1 + > drivers/scsi/ufs/ufs-qcom-ice.c | 520 > > drivers/scsi/ufs/ufs-qcom-ice.h | 113 + > drivers/scsi/ufs/ufs-qcom.c | 56 - > drivers/scsi/ufs/ufs-qcom.h | 25 ++ > 6 files changed, 726 insertions(+), 1 deletion(-) > create mode 100644 drivers/scsi/ufs/ufs-qcom-ice.c > create mode 100644 drivers/scsi/ufs/ufs-qcom-ice.h > > diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig > index 8a1f4b3..ecf34ed 100644 > --- a/drivers/scsi/ufs/Kconfig > +++ b/drivers/scsi/ufs/Kconfig > @@ -83,3 +83,15 @@ config SCSI_UFS_QCOM > > Select this if you have UFS controller on QCOM chipset. > If unsure, say N. > + > +config SCSI_UFS_QCOM_ICE > + bool "QCOM specific hooks to Inline Crypto Engine for UFS driver" > + depends on SCSI_UFS_QCOM && CRYPTO_DEV_QCOM_ICE There's currently no Kconfig symbol CRYPTO_DEV_QCOM_ICE in linux-next. So SCSI_UFS_QCOM_ICE can not be set and these "in-line encryption capabilities" can not be enabled. > + help > + This selects the QCOM specific additions to support Inline Crypto > + Engine (ICE). > + ICE accelerates the crypto operations and maintains the high UFS > + performance. > + > + Select this if you have ICE supported for UFS on QCOM chipset. > + If unsure, say N. > diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile > index 8303bcc..31adca5 100644 > --- a/drivers/scsi/ufs/Makefile > +++ b/drivers/scsi/ufs/Makefile > @@ -1,5 +1,6 @@ > # UFSHCD makefile > obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o > +obj-$(CONFIG_SCSI_UFS_QCOM_ICE) += ufs-qcom-ice.o > obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o > obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o > obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o > diff --git a/drivers/scsi/ufs/ufs-qcom-ice.c b/drivers/scsi/ufs/ufs-qcom-ice.c > new file mode 100644 > index 000..9202b73 > --- /dev/null > +++ b/drivers/scsi/ufs/ufs-qcom-ice.c > @@ -0,0 +1,520 @@ > +/* Copyright (c) 2014-2015, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include This header is not included in linux-next so manually building ufs-qcom-ice.o isn't possible either. I assume a series to add CRYPTO_DEV_QCOM_ICE and crypto/ice.h (and whatever else is needed to build this) is queued somewhere. Is that correct? Paul Bolle -- 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 0/17] Clear up bidi command confusion
There is some confusion in the SCSI core and in SCSI LLDs around the meaning of sc_data_direction and whether or not this member can have the value DMA_BIDIRECTIONAL. Clear up this confusion. The patches in this series are: 0001-Clear-up-sc_data_direction-documentation.patch 0002-Clean-up-scsi_ioctl_reset.patch 0003-sg-Remove-an-unused-variable.patch 0004-dpt_i2o-Fix-bidi-command-test.patch 0005-aachba-Fix-bidi-command-test.patch 0006-eata-Remove-dead-code.patch 0007-sbp2-Fix-bidi-command-test.patch 0008-ibmvscsi-Fix-bidi-command-test.patch 0009-cciss-Remove-dead-code.patch 0010-hpsa-Remove-dead-code.patch 0011-3w-9xxx-Remove-dead-code.patch 0012-ncr53c8xx-Fix-bidi-command-support.patch 0013-qla1280-Fix-bidi-command-support.patch 0014-53c700-Remove-dead-code.patch 0015-mvumi-Remove-dead-code.patch 0016-sym_glue-Fix-bidi-command-test.patch 0017-IB-srp-Detect-bidi-commands-properly.patch -- 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 01/17] Clear up sc_data_direction documentation
Signed-off-by: Bart Van Assche --- Documentation/DMA-API-HOWTO.txt | 6 +++--- Documentation/scsi/scsi_mid_low_api.txt | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Documentation/DMA-API-HOWTO.txt b/Documentation/DMA-API-HOWTO.txt index 0f7afb2..3999ec3 100644 --- a/Documentation/DMA-API-HOWTO.txt +++ b/Documentation/DMA-API-HOWTO.txt @@ -507,9 +507,9 @@ Only streaming mappings specify a direction, consistent mappings implicitly have a direction attribute setting of DMA_BIDIRECTIONAL. -The SCSI subsystem tells you the direction to use in the -'sc_data_direction' member of the SCSI command your driver is -working on. +Except for bidi commands, the SCSI subsystem tells you the direction to +use in the 'sc_data_direction' member of the SCSI command your driver +is working on. For Networking drivers, it's a rather simple affair. For transmit packets, map/unmap them with the DMA_TO_DEVICE direction diff --git a/Documentation/scsi/scsi_mid_low_api.txt b/Documentation/scsi/scsi_mid_low_api.txt index 731bc4f..39cbbca 100644 --- a/Documentation/scsi/scsi_mid_low_api.txt +++ b/Documentation/scsi/scsi_mid_low_api.txt @@ -1259,8 +1259,9 @@ be at least one instance of struct scsi_cmnd available for each SCSI device. Members of interest: cmnd - array containing SCSI command cmnd_len - length (in bytes) of SCSI command -sc_data_direction - direction of data transfer in data phase. See -"enum dma_data_direction" in include/linux/dma-mapping.h +sc_data_direction - except for bidi commands, direction of data transfer +in data phase. See "enum dma_data_direction" in +include/linux/dma-mapping.h request_bufflen - number of data bytes to transfer (0 if no data phase) use_sg - ==0 -> no scatter gather list, hence transfer data to/from request_buffer -- 2.1.2 -- 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 02/17] Clean up scsi_ioctl_reset()
Since SG_SCSI_RESET causes a task management function to be sent and since LLDs only check the sc_data_direction field inside the queueucommand callback function, the sc_data_direction member does not have to be set here. Hence remove the sc_data_direction assignment. Signed-off-by: Bart Van Assche --- drivers/scsi/scsi_error.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 8afb016..9ee4830 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -2346,8 +2346,6 @@ scsi_ioctl_reset(struct scsi_device *dev, int __user *arg) scmd->cmd_len = 0; - scmd->sc_data_direction = DMA_BIDIRECTIONAL; - spin_lock_irqsave(shost->host_lock, flags); shost->tmf_in_progress = 1; spin_unlock_irqrestore(shost->host_lock, flags); -- 2.1.2 -- 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 03/17] sg: Remove an unused variable
Signed-off-by: Bart Van Assche --- drivers/scsi/sg.c | 17 + 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index b14f64c..a668c88 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -763,7 +763,7 @@ static int sg_common_write(Sg_fd * sfp, Sg_request * srp, unsigned char *cmnd, int timeout, int blocking) { - int k, data_dir, at_head; + int k, at_head; Sg_device *sdp = sfp->parentdp; sg_io_hdr_t *hp = &srp->header; @@ -793,21 +793,6 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp, return -ENODEV; } - switch (hp->dxfer_direction) { - case SG_DXFER_TO_FROM_DEV: - case SG_DXFER_FROM_DEV: - data_dir = DMA_FROM_DEVICE; - break; - case SG_DXFER_TO_DEV: - data_dir = DMA_TO_DEVICE; - break; - case SG_DXFER_UNKNOWN: - data_dir = DMA_BIDIRECTIONAL; - break; - default: - data_dir = DMA_NONE; - break; - } hp->duration = jiffies_to_msecs(jiffies); if (hp->interface_id != '\0' && /* v3 (or later) interface */ (SG_FLAG_Q_AT_TAIL & hp->flags)) -- 2.1.2 -- 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 04/17] dpt_i2o: Fix bidi command test
Signed-off-by: Bart Van Assche Cc: Adaptec OEM Raid Solutions --- drivers/scsi/dpt_i2o.c | 21 ++--- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c index 0bf9769..9bab86c 100644 --- a/drivers/scsi/dpt_i2o.c +++ b/drivers/scsi/dpt_i2o.c @@ -2240,26 +2240,17 @@ static s32 adpt_scsi_to_i2o(adpt_hba* pHba, struct scsi_cmnd* cmd, struct adpt_d * Note: Do not have to verify index is less than 0 since * cmd->cmnd[0] is an unsigned char */ - switch(cmd->sc_data_direction){ - case DMA_FROM_DEVICE: - scsidir =0x4000; // DATA IN (iop<--dev) - break; - case DMA_TO_DEVICE: - direction=0x0400; // SGL OUT - scsidir =0x8000; // DATA OUT (iop-->dev) - break; - case DMA_NONE: - break; - case DMA_BIDIRECTIONAL: - scsidir =0x4000; // DATA IN (iop<--dev) - // Assume In - and continue; - break; - default: + if (scsi_bidi_cmnd(cmd)) { printk(KERN_WARNING"%s: scsi opcode 0x%x not supported.\n", pHba->name, cmd->cmnd[0]); cmd->result = (DID_OK <<16) | (INITIATOR_ERROR << 8); cmd->scsi_done(cmd); return 0; + } else if (cmd->sc_data_direction == DMA_FROM_DEVICE) { + scsidir =0x4000; // DATA IN (iop<--dev) + } else if (cmd->sc_data_direction == DMA_TO_DEVICE) { + direction=0x0400; // SGL OUT + scsidir =0x8000; // DATA OUT (iop-->dev) } } // msg[0] is set later -- 2.1.2 -- 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 05/17] aachba: Fix bidi command test
Signed-off-by: Bart Van Assche --- drivers/scsi/aacraid/aachba.c | 17 + 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c index b32e77d..5ade84a 100644 --- a/drivers/scsi/aacraid/aachba.c +++ b/drivers/scsi/aacraid/aachba.c @@ -1228,21 +1228,14 @@ static struct aac_srb * aac_scsi_common(struct fib * fib, struct scsi_cmnd * cmd u32 timeout; aac_fib_init(fib); - switch(cmd->sc_data_direction){ - case DMA_TO_DEVICE: - flag = SRB_DataOut; - break; - case DMA_BIDIRECTIONAL: + if (scsi_bidi_cmnd(cmd)) flag = SRB_DataIn | SRB_DataOut; - break; - case DMA_FROM_DEVICE: + else if (cmd->sc_data_direction == DMA_TO_DEVICE) + flag = SRB_DataOut; + else if (cmd->sc_data_direction == DMA_FROM_DEVICE) flag = SRB_DataIn; - break; - case DMA_NONE: - default:/* shuts up some versions of gcc */ + else flag = SRB_NoDataXfer; - break; - } srbcmd = (struct aac_srb*) fib_data(fib); srbcmd->function = cpu_to_le32(SRBF_ExecuteScsi); -- 2.1.2 -- 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 06/17] eata: Remove dead code
The SCSI core never sets sc_data_direction to DMA_BIDIRECTIONAL. Hence remove the code that tests for this value. Signed-off-by: Bart Van Assche Cc: Dario Ballabio --- drivers/scsi/eata.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c index 227dd2c..c1fa5ba 100644 --- a/drivers/scsi/eata.c +++ b/drivers/scsi/eata.c @@ -1726,9 +1726,8 @@ static void scsi_to_dev_dir(unsigned int i, struct hostdata *ha) return; } - if (SCpnt->sc_data_direction != DMA_BIDIRECTIONAL) - panic("%s: qcomm, invalid SCpnt->sc_data_direction.\n", - ha->board_name); + panic("%s: qcomm, invalid SCpnt->sc_data_direction %d.\n", + ha->board_name, SCpnt->sc_data_direction); for (k = 0; k < ARRAY_SIZE(data_out_cmds); k++) if (SCpnt->cmnd[0] == data_out_cmds[k]) { -- 2.1.2 -- 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 08/17] ibmvscsi: Fix bidi command test
Signed-off-by: Bart Van Assche Cc: Brian King Cc: Nathan Fontenot --- drivers/scsi/ibmvscsi/ibmvscsi.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index acea5d6..cf26b33 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -765,16 +765,18 @@ static int map_data_for_srp_cmd(struct scsi_cmnd *cmd, struct srp_event_struct *evt_struct, struct srp_cmd *srp_cmd, struct device *dev) { + if (scsi_bidi_cmnd(cmd)) { + sdev_printk(KERN_ERR, cmd->device, + "Bidirectional commands are not yet supported\n"); + return 0; + } + switch (cmd->sc_data_direction) { case DMA_FROM_DEVICE: case DMA_TO_DEVICE: break; case DMA_NONE: return 1; - case DMA_BIDIRECTIONAL: - sdev_printk(KERN_ERR, cmd->device, - "Can't map DMA_BIDIRECTIONAL to read/write\n"); - return 0; default: sdev_printk(KERN_ERR, cmd->device, "Unknown data direction 0x%02x; can't map!\n", -- 2.1.2 -- 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 09/17] cciss: Remove dead code
The code that could set sc_data_direction to DMA_BIDIRECTIONAL has been removed eight years ago. Hence remove the code that handles this case. See also patch "unify SCSI_IOCTL_SEND_COMMAND implementations" (commit 21b2f0c803ad). Signed-off-by: Bart Van Assche Cc: Don Brace --- drivers/block/cciss_scsi.c | 15 --- 1 file changed, 15 deletions(-) diff --git a/drivers/block/cciss_scsi.c b/drivers/block/cciss_scsi.c index ecd845c..8f60a64 100644 --- a/drivers/block/cciss_scsi.c +++ b/drivers/block/cciss_scsi.c @@ -1478,21 +1478,6 @@ cciss_scsi_queue_command_lck(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmn case DMA_NONE: c->Request.Type.Direction = XFER_NONE; break; - case DMA_BIDIRECTIONAL: - // This can happen if a buggy application does a scsi passthru - // and sets both inlen and outlen to non-zero. ( see - // ../scsi/scsi_ioctl.c:scsi_ioctl_send_command() ) - - c->Request.Type.Direction = XFER_RSVD; - // This is technically wrong, and cciss controllers should - // reject it with CMD_INVALID, which is the most correct - // response, but non-fibre backends appear to let it - // slide by, and give the same results as if this field - // were set correctly. Either way is acceptable for - // our purposes here. - - break; - default: dev_warn(&h->pdev->dev, "unknown data direction: %d\n", cmd->sc_data_direction); -- 2.1.2 -- 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 10/17] hpsa: Remove dead code
The SCSI core never sets sc_data_direction to DMA_BIDIRECTIONAL. Hence remove the code that tests for this value. Signed-off-by: Bart Van Assche Cc: Don Brace --- drivers/scsi/hpsa.c | 18 -- 1 file changed, 18 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 6bb4611..9d16a06 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3969,24 +3969,6 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) c->Request.type_attr_dir = TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_NONE); break; - case DMA_BIDIRECTIONAL: - /* This can happen if a buggy application does a scsi passthru -* and sets both inlen and outlen to non-zero. ( see -* ../scsi/scsi_ioctl.c:scsi_ioctl_send_command() ) -*/ - - c->Request.type_attr_dir = - TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_RSVD); - /* This is technically wrong, and hpsa controllers should -* reject it with CMD_INVALID, which is the most correct -* response, but non-fibre backends appear to let it -* slide by, and give the same results as if this field -* were set correctly. Either way is acceptable for -* our purposes here. -*/ - - break; - default: dev_err(&h->pdev->dev, "unknown data direction: %d\n", cmd->sc_data_direction); -- 2.1.2 -- 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 11/17] 3w-9xxx: Remove dead code
The SCSI core never sets sc_data_direction to DMA_BIDIRECTIONAL. Hence remove the code that tests for this value. Signed-off-by: Bart Van Assche Cc: Adam Radford --- drivers/scsi/3w-9xxx.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c index cd4129f..e83e3b7 100644 --- a/drivers/scsi/3w-9xxx.c +++ b/drivers/scsi/3w-9xxx.c @@ -1854,8 +1854,7 @@ static int twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id, if (scsi_sg_count(srb)) { if ((scsi_sg_count(srb) == 1) && (scsi_bufflen(srb) < TW_MIN_SGL_LENGTH)) { - if (srb->sc_data_direction == DMA_TO_DEVICE || - srb->sc_data_direction == DMA_BIDIRECTIONAL) + if (srb->sc_data_direction == DMA_TO_DEVICE) scsi_sg_copy_to_buffer(srb, tw_dev->generic_buffer_virt[request_id], TW_SECTOR_SIZE); @@ -1927,8 +1926,7 @@ static void twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re struct scsi_cmnd *cmd = tw_dev->srb[request_id]; if (scsi_bufflen(cmd) < TW_MIN_SGL_LENGTH && - (cmd->sc_data_direction == DMA_FROM_DEVICE || -cmd->sc_data_direction == DMA_BIDIRECTIONAL)) { + cmd->sc_data_direction == DMA_FROM_DEVICE) { if (scsi_sg_count(cmd) == 1) { void *buf = tw_dev->generic_buffer_virt[request_id]; -- 2.1.2 -- 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 14/17] 53c700: Remove dead code
The SCSI core never sets sc_data_direction to DMA_BIDIRECTIONAL. Hence remove the code that tests for this value. Signed-off-by: Bart Van Assche --- drivers/scsi/53c700.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c index 82abfce..78f4b57 100644 --- a/drivers/scsi/53c700.c +++ b/drivers/scsi/53c700.c @@ -578,8 +578,7 @@ STATIC inline void NCR_700_unmap(struct NCR_700_Host_Parameters *hostdata, struct scsi_cmnd *SCp, struct NCR_700_command_slot *slot) { - if(SCp->sc_data_direction != DMA_NONE && - SCp->sc_data_direction != DMA_BIDIRECTIONAL) + if (SCp->sc_data_direction != DMA_NONE) scsi_dma_unmap(SCp); } @@ -1846,7 +1845,6 @@ NCR_700_queuecommand_lck(struct scsi_cmnd *SCp, void (*done)(struct scsi_cmnd *) default: /* OK, get it from the command */ switch(SCp->sc_data_direction) { - case DMA_BIDIRECTIONAL: default: printk(KERN_ERR "53c700: Unknown command for data direction "); scsi_print_command(SCp); -- 2.1.2 -- 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 15/17] mvumi: Remove dead code
The SCSI core never sets sc_data_direction to DMA_BIDIRECTIONAL. Hence remove the code that tests for this value. Signed-off-by: Bart Van Assche --- drivers/scsi/mvumi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c index 3e6b866..eae3cb2 100644 --- a/drivers/scsi/mvumi.c +++ b/drivers/scsi/mvumi.c @@ -2081,7 +2081,6 @@ static unsigned char mvumi_build_frame(struct mvumi_hba *mhba, case DMA_TO_DEVICE: pframe->cmd_flag |= CMD_FLAG_DATA_OUT; break; - case DMA_BIDIRECTIONAL: default: dev_warn(&mhba->pdev->dev, "unexpected data direction[%d] " "cmd[0x%x]\n", scmd->sc_data_direction, scmd->cmnd[0]); -- 2.1.2 -- 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 16/17] sym_glue: Fix bidi command test
Signed-off-by: Bart Van Assche --- drivers/scsi/sym53c8xx_2/sym_glue.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c index 5d00e51..c33c697 100644 --- a/drivers/scsi/sym53c8xx_2/sym_glue.c +++ b/drivers/scsi/sym53c8xx_2/sym_glue.c @@ -374,14 +374,16 @@ int sym_setup_data_and_start(struct sym_hcb *np, struct scsi_cmnd *cmd, struct s cp->segments = 0; } + if (scsi_bidi_cmnd(cmd)) { + scmd_printk(KERN_INFO, cmd, "got bidi command"); + sym_set_cam_status(cmd, DID_ERROR); + goto out_abort; + } + /* * Set the data pointer. */ switch (dir) { - case DMA_BIDIRECTIONAL: - scmd_printk(KERN_INFO, cmd, "got DMA_BIDIRECTIONAL command"); - sym_set_cam_status(cmd, DID_ERROR); - goto out_abort; case DMA_TO_DEVICE: goalp = SCRIPTA_BA(np, data_out2) + 8; lastp = goalp - 8 - (cp->segments * (2*4)); -- 2.1.2 -- 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 17/17] IB/srp: Detect bidi commands properly
The SCSI core never sets sc_data_direction to DMA_BIDIRECTIONAL. This allows to simplify srp_unmap_data(). Fix the bidi test in srp_map_data(). Signed-off-by: Bart Van Assche Cc: Roland Dreier --- drivers/infiniband/ulp/srp/ib_srp.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 0747c05..22b7529 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1063,9 +1063,7 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd, struct ib_device *ibdev = dev->dev; int i, res; - if (!scsi_sglist(scmnd) || - (scmnd->sc_data_direction != DMA_TO_DEVICE && -scmnd->sc_data_direction != DMA_FROM_DEVICE)) + if (!scsi_sglist(scmnd)) return; if (dev->use_fast_reg) { @@ -1524,11 +1522,9 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch, if (!scsi_sglist(scmnd) || scmnd->sc_data_direction == DMA_NONE) return sizeof (struct srp_cmd); - if (scmnd->sc_data_direction != DMA_FROM_DEVICE && - scmnd->sc_data_direction != DMA_TO_DEVICE) { + if (scsi_bidi_cmnd(scmnd)) { shost_printk(KERN_WARNING, target->scsi_host, -PFX "Unhandled data direction %d\n", -scmnd->sc_data_direction); +PFX "Bidi commands are not yet supported\n"); return -EINVAL; } -- 2.1.2 -- 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 07/17] sbp2: Fix bidi command test
The SCSI core never sets sc_data_direction to DMA_BIDIRECTIONAL. Signed-off-by: Bart Van Assche Cc: Stefan Richter --- drivers/firewire/sbp2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index 64ac8f8..ab1b114 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -1467,7 +1467,7 @@ static int sbp2_scsi_queuecommand(struct Scsi_Host *shost, * Bidirectional commands are not yet implemented, and unknown * transfer direction not handled. */ - if (cmd->sc_data_direction == DMA_BIDIRECTIONAL) { + if (scsi_bidi_cmnd(cmd)) { dev_err(lu_dev(lu), "cannot handle bidirectional command\n"); cmd->result = DID_ERROR << 16; cmd->scsi_done(cmd); -- 2.1.2 -- 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 13/17] qla1280: Fix bidi command support
The SCSI core never sets sc_data_direction to DMA_BIDIRECTIONAL. Signed-off-by: Bart Van Assche Cc: Michael Reed --- drivers/scsi/qla1280.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c index c68a66e..ede9563 100644 --- a/drivers/scsi/qla1280.c +++ b/drivers/scsi/qla1280.c @@ -468,13 +468,14 @@ static struct qla_driver_setup driver_setup; static inline uint16_t qla1280_data_direction(struct scsi_cmnd *cmnd) { + if (scsi_bidi_cmnd(cmnd)) + return BIT_5 | BIT_6; + switch(cmnd->sc_data_direction) { case DMA_FROM_DEVICE: return BIT_5; case DMA_TO_DEVICE: return BIT_6; - case DMA_BIDIRECTIONAL: - return BIT_5 | BIT_6; /* * We could BUG() on default here if one of the four cases aren't * met, but then again if we receive something like that from the -- 2.1.2 -- 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 12/17] ncr53c8xx: Fix bidi command support
Signed-off-by: Bart Van Assche Cc: Finn Thain Cc: Michael Schmitz --- drivers/scsi/ncr53c8xx.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/ncr53c8xx.c b/drivers/scsi/ncr53c8xx.c index 5b93ed8..b9c489a 100644 --- a/drivers/scsi/ncr53c8xx.c +++ b/drivers/scsi/ncr53c8xx.c @@ -4286,13 +4286,12 @@ static int ncr_queue_command (struct ncb *np, struct scsi_cmnd *cmd) direction = DMA_NONE; /* - ** If data direction is BIDIRECTIONAL, speculate FROM_DEVICE + ** For bidi commands, speculate FROM_DEVICE ** but prepare alternate pointers for TO_DEVICE in case ** of our speculation will be just wrong. ** SCRIPTS will swap values if needed. */ switch(direction) { - case DMA_BIDIRECTIONAL: case DMA_TO_DEVICE: goalp = NCB_SCRIPT_PHYS (np, data_out2) + 8; if (segments <= MAX_SCATTERL) @@ -4301,7 +4300,7 @@ static int ncr_queue_command (struct ncb *np, struct scsi_cmnd *cmd) lastp = NCB_SCRIPTH_PHYS (np, hdata_out2); lastp -= (segments - MAX_SCATTERL) * 16; } - if (direction != DMA_BIDIRECTIONAL) + if (!scsi_bidi_cmd(cmd)) break; cp->phys.header.wgoalp = cpu_to_scr(goalp); cp->phys.header.wlastp = cpu_to_scr(lastp); @@ -4328,7 +4327,7 @@ static int ncr_queue_command (struct ncb *np, struct scsi_cmnd *cmd) cp->phys.header.lastp = cpu_to_scr(lastp); cp->phys.header.goalp = cpu_to_scr(goalp); - if (direction == DMA_BIDIRECTIONAL) + if (scsi_bidi_cmd(cmd)) cp->phys.header.savep = cpu_to_scr(NCB_SCRIPTH_PHYS (np, data_io)); else -- 2.1.2 -- 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 03/17] sg: Remove an unused variable
On 15-01-23 01:07 PM, Bart Van Assche wrote: Signed-off-by: Bart Van Assche Acked-by: Douglas Gilbert --- drivers/scsi/sg.c | 17 + 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index b14f64c..a668c88 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -763,7 +763,7 @@ static int sg_common_write(Sg_fd * sfp, Sg_request * srp, unsigned char *cmnd, int timeout, int blocking) { - int k, data_dir, at_head; + int k, at_head; Sg_device *sdp = sfp->parentdp; sg_io_hdr_t *hp = &srp->header; @@ -793,21 +793,6 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp, return -ENODEV; } - switch (hp->dxfer_direction) { - case SG_DXFER_TO_FROM_DEV: - case SG_DXFER_FROM_DEV: - data_dir = DMA_FROM_DEVICE; - break; - case SG_DXFER_TO_DEV: - data_dir = DMA_TO_DEVICE; - break; - case SG_DXFER_UNKNOWN: - data_dir = DMA_BIDIRECTIONAL; - break; - default: - data_dir = DMA_NONE; - break; - } hp->duration = jiffies_to_msecs(jiffies); if (hp->interface_id != '\0' && /* v3 (or later) interface */ (SG_FLAG_Q_AT_TAIL & hp->flags)) -- 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 0/17] Clear up bidi command confusion
On Fri, Jan 23, 2015 at 01:05:30PM +0100, Bart Van Assche wrote: > There is some confusion in the SCSI core and in SCSI LLDs around the > meaning of sc_data_direction and whether or not this member can have the > value DMA_BIDIRECTIONAL. Clear up this confusion. The patches in this > series are: I wonder if we should change the code to set DMA_BIDIRECTIONAL for bidi commands. That seems a lot more logical than the current version. Also I don't think all the debug checks for bidi commands that you change should stay at all - driver need to set the QUEUE_FLAG_BIDI to ever see a bidi command. It would also nice to add a host template flag for bidi support instead of having to poke into the block layer request_queue while we're at it. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: module: fix module_refcount() return when running in a module exit routine
On Fri, Jan 23, 2015 at 01:24:15PM +1030, Rusty Russell wrote: > The correct fix is to turn try_module_get() into __module_get(), and > always do the module_put(). Is this really safe? __module_get sais it needs a non-zero refcount to start with, but scsi_device_get is the only thing every incrementing the refcount on the module pointer in the scsi host template, so exactly that case can happen easily. There's not assert actually hardcoding the assumption, so I'm not sure if requirement the comment really just is nice to have or a strong requirement. -- 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 00/48] hpsa driver updates
Any comments on the reviews? Should I just take the patches that didn't have objections for now? -- 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 07/17] sbp2: Fix bidi command test
On Jan 23 Bart Van Assche wrote: > The SCSI core never sets sc_data_direction to DMA_BIDIRECTIONAL. > > Signed-off-by: Bart Van Assche > Cc: Stefan Richter > --- > drivers/firewire/sbp2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c > index 64ac8f8..ab1b114 100644 > --- a/drivers/firewire/sbp2.c > +++ b/drivers/firewire/sbp2.c > @@ -1467,7 +1467,7 @@ static int sbp2_scsi_queuecommand(struct Scsi_Host > *shost, >* Bidirectional commands are not yet implemented, and unknown >* transfer direction not handled. >*/ > - if (cmd->sc_data_direction == DMA_BIDIRECTIONAL) { > + if (scsi_bidi_cmnd(cmd)) { > dev_err(lu_dev(lu), "cannot handle bidirectional command\n"); > cmd->result = DID_ERROR << 16; > cmd->scsi_done(cmd); 8< Subject: [PATCH] firewire: sbp2: remove redundant check for bidi command [Bart van Asche:] SCSI core never sets cmd->sc_data_direction to DMA_BIDIRECTIONAL, scsi_bidi_cmnd(cmd) should be used instead. [Christoph Hellwig:] Bidirectional commands won't ever be queued anyway, unless a LLD or transport driver sets QUEUE_FLAG_BIDI. So, simply remove the respective queuecommand check in the SBP-2 transport driver. Signed-off-by: Stefan Richter --- drivers/firewire/sbp2.c | 11 --- 1 file changed, 11 deletions(-) --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -1463,17 +1463,6 @@ static int sbp2_scsi_queuecommand(struct struct sbp2_command_orb *orb; int generation, retval = SCSI_MLQUEUE_HOST_BUSY; - /* -* Bidirectional commands are not yet implemented, and unknown -* transfer direction not handled. -*/ - if (cmd->sc_data_direction == DMA_BIDIRECTIONAL) { - dev_err(lu_dev(lu), "cannot handle bidirectional command\n"); - cmd->result = DID_ERROR << 16; - cmd->scsi_done(cmd); - return 0; - } - orb = kzalloc(sizeof(*orb), GFP_ATOMIC); if (orb == NULL) return SCSI_MLQUEUE_HOST_BUSY; -- Stefan Richter -=-= ---= =-=== http://arcgraph.de/sr/ -- 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 0/2] hpsa: workqueue + minor fix
I've adapted previously posted changes on top of the latest hpsa series. Tomas -- 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/2] hpsa: remove a needless call - pci_get_drvdata
Signed-off-by: Tomas Henzl --- drivers/scsi/hpsa.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 909562c09f..7fdc4d2a91 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -7069,13 +7069,12 @@ static void hpsa_free_device_info(struct ctlr_info *h) static void hpsa_remove_one(struct pci_dev *pdev) { - struct ctlr_info *h; + struct ctlr_info *h = pci_get_drvdata(pdev); - if (pci_get_drvdata(pdev) == NULL) { + if (!h) { dev_err(&pdev->dev, "unable to remove device\n"); return; } - h = pci_get_drvdata(pdev); /* Get rid of any controller monitoring work items */ h->remove_in_progress = 1; -- 1.9.3 -- 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/2] hpsa: use a driver's own workqueue instead of a shared wq
hpsa driver uses a shared wq, max sleep time in function spent hpsa_wait_for_clear_event_notify_ack may take up to 20sec and that is too much for a shared workqueue. This patch takes the easiest approach and just creates a driver's own workqueue. Signed-off-by: Tomas Henzl --- drivers/scsi/hpsa.c | 39 +-- drivers/scsi/hpsa.h | 1 + 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 15ef65c25d..909562c09f 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6229,6 +6229,12 @@ static int hpsa_wait_for_mode_change_ack(struct ctlr_info *h) goto done; /* delay and try again */ msleep(MODE_CHANGE_WAIT_INTERVAL); + + /* we allow a timeout under certain conditions, +* so we may return here when a removal is in progress +*/ + if (h->remove_in_progress) + return -ENODEV; } return -ENODEV; done: @@ -6790,7 +6796,6 @@ static int hpsa_offline_devices_ready(struct ctlr_info *h) static void hpsa_monitor_ctlr_worker(struct work_struct *work) { - unsigned long flags; struct ctlr_info *h = container_of(to_delayed_work(work), struct ctlr_info, monitor_ctlr_work); detect_controller_lockup(h); @@ -6804,14 +6809,10 @@ static void hpsa_monitor_ctlr_worker(struct work_struct *work) scsi_host_put(h->scsi_host); } - spin_lock_irqsave(&h->lock, flags); - if (h->remove_in_progress) { - spin_unlock_irqrestore(&h->lock, flags); + if (h->remove_in_progress) return; - } - schedule_delayed_work(&h->monitor_ctlr_work, - h->heartbeat_sample_interval); - spin_unlock_irqrestore(&h->lock, flags); + queue_delayed_work(h->monitor_ctrl_wq, &h->monitor_ctlr_work, + h->heartbeat_sample_interval); } static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) @@ -6820,6 +6821,7 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) struct ctlr_info *h; int try_soft_reset = 0; unsigned long flags; + char wq_name[20]; if (number_of_controllers == 0) printk(KERN_INFO DRIVER_NAME "\n"); @@ -6983,10 +6985,20 @@ reinit_after_soft_reset: /* Monitor the controller for firmware lockups */ h->heartbeat_sample_interval = HEARTBEAT_SAMPLE_INTERVAL; INIT_DELAYED_WORK(&h->monitor_ctlr_work, hpsa_monitor_ctlr_worker); - schedule_delayed_work(&h->monitor_ctlr_work, - h->heartbeat_sample_interval); + snprintf(wq_name, sizeof(wq_name), "monitor_%d_hpsa", h->ctlr); + h->monitor_ctrl_wq = alloc_ordered_workqueue(wq_name, 0); + if (!h->monitor_ctrl_wq) { + dev_err(&pdev->dev, "failed to create monitor workqueue\n"); + goto clean5; + } + queue_delayed_work(h->monitor_ctrl_wq, &h->monitor_ctlr_work, + h->heartbeat_sample_interval); return 0; +clean5: + hpsa_unregister_scsi(h); + kfree(h->hba_inquiry_data); + h->access.set_intr_mask(h, HPSA_INTR_OFF); clean4: hpsa_free_sg_chain_blocks(h); hpsa_free_cmd_pool(h); @@ -7058,7 +7070,6 @@ static void hpsa_free_device_info(struct ctlr_info *h) static void hpsa_remove_one(struct pci_dev *pdev) { struct ctlr_info *h; - unsigned long flags; if (pci_get_drvdata(pdev) == NULL) { dev_err(&pdev->dev, "unable to remove device\n"); @@ -7067,10 +7078,10 @@ static void hpsa_remove_one(struct pci_dev *pdev) h = pci_get_drvdata(pdev); /* Get rid of any controller monitoring work items */ - spin_lock_irqsave(&h->lock, flags); h->remove_in_progress = 1; - cancel_delayed_work(&h->monitor_ctlr_work); - spin_unlock_irqrestore(&h->lock, flags); + cancel_delayed_work_sync(&h->monitor_ctlr_work); + destroy_workqueue(h->monitor_ctrl_wq); + hpsa_unregister_scsi(h);/* unhook from SCSI subsystem */ hpsa_shutdown(pdev); destroy_workqueue(h->resubmit_wq); diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 62c50c3207..bff65d0a7b 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -207,6 +207,7 @@ struct ctlr_info { atomic_t firmware_flash_in_progress; u32 __percpu *lockup_detected; struct delayed_work monitor_ctlr_work; + struct workqueue_struct *monitor_ctrl_wq; int remove_in_progress; /* Address of h->q[x] is passed to intr handler to know which queue */ u8 q[MAX_REPLY_QUEUES]; -- 1.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo in
Re: [PATCH 00/48] hpsa driver updates
On Fri, Jan 23, 2015 at 04:58:52PM +, brace wrote: > I made your requested changes. Do you want me to just send those up or the > whole set up again? > > Tomas Henzl asked me to add a workqueue change also... Just resend the whole series, looks like you're using git-end-email already. -- 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: block layer copying user io vectors
On Thu, Jan 22, 2015 at 09:33:08PM +, Andy Falanga (afalanga) wrote: > Please CC me directly. > > I am working in kernel 2.6.32 (CentOS 6). To increase the upper limit > of sg from 4mb to at least 128mb in a single SCSI command. At first I > thought this issue was in sg, but have tracked the issue to the block > layer. 2.6.32 is fairly old, but fortunately for you not too many things should have changed in this area. > > Thinking I could solve this issue by using scatter/gather lists, I > increased the size from 32k to 4mb of each vector. This did work > until I tried to send 8mb. When I do so, I get errno EINVAL. After > some tracing, I tracked the problem into bio_copy_user_iov(). > > This function does something that seems rather strange. On line 859, > a for loop determines the number of pages needed for the copying of the > user data to kernel space. Then the memory is allocated (line > 886 bio_kmalloc()). Then, strangely, on line 895, there is this > conditional: This is because the function can also be used with preallocated pages, a feature only used by the sg and tape drivers. Make sure your user memory is 4k aligned, and you should be able to avoid the copy entirely (1). (1) except that the sg driver disables the direct mapping of user pages when using readv/writev. I can't really see why and it should be fixable by just removign that condition from the if in sg_start_req. Alternatively use the SG_IO ioctl directly on the disk device node, which neither has the read/writev limitation, nor does it use a fixed upper bound preallocated page pool. > > if (map_data) { > nr_pages = 1 << map_data->page_order; > i = map_data->offset / PAGE_SIZE; > } > > This effectively ignores the number of pages counted earlier (in this > case which applies to me), and then apparently disregards whatever > memory may have been allocated earlier. Thinking that this was the > root I tried to correct this by commenting that simple branch from > bio_copy_user_iov, but still had the same result. Can someone > help me understand what is happening in the block layer? -- 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 00/48] hpsa driver updates
I made your requested changes. Do you want me to just send those up or the whole set up again? Tomas Henzl asked me to add a workqueue change also... > -Original Message- > From: Christoph Hellwig [mailto:h...@infradead.org] > Sent: Friday, January 23, 2015 7:22 AM > To: Don Brace > Cc: Scott Teel; Kevin Barnett; james.bottom...@parallels.com; > h...@infradead.org; Justin Lindley; brace; linux-scsi@vger.kernel.org > Subject: Re: [PATCH 00/48] hpsa driver updates > > Any comments on the reviews? Should I just take the patches that > didn't have objections for now? -- 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 00/48] hpsa driver updates
On 01/23/2015 06:01 PM, Christoph Hellwig wrote: > On Fri, Jan 23, 2015 at 04:58:52PM +, brace wrote: >> I made your requested changes. Do you want me to just send those up or the >> whole set up again? >> >> Tomas Henzl asked me to add a workqueue change also... I have rebased the patch on top of your series and reposted an hour ago, you may use the new version. > Just resend the whole series, looks like you're using git-end-email > already. > -- > 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: module: fix module_refcount() return when running in a module exit routine
On Fri, 2015-01-23 at 05:17 -0800, Christoph Hellwig wrote: > On Fri, Jan 23, 2015 at 01:24:15PM +1030, Rusty Russell wrote: > > The correct fix is to turn try_module_get() into __module_get(), and > > always do the module_put(). > > Is this really safe? __module_get sais it needs a non-zero refcount > to start with, but scsi_device_get is the only thing every incrementing > the refcount on the module pointer in the scsi host template, so > exactly that case can happen easily. There's not assert actually > hardcoding the assumption, so I'm not sure if requirement the comment > really just is nice to have or a strong requirement. The comment was just documenting the old status quo: the try_module_get() was expected to fail if called within the host module remove path. If you look at the flow, we use the refcounts on the actual scsi device to measure. If they fail we know we have a problem. The module stuff is really only slaved to our master refcount on the device. It's purpose is to prevent an inopportune rmmod. Our default operating state is zero references on everything, so the user can just do rmmod ... obviously if the device is open or mounted then we hold both the device and the module. To that point, Rusty's patch just keeps the status quo in the new module_refcount() environment, so it's the quick bandaid. I think the use case you're worrying about is what happens if someone tries to use a device after module removal begins executing but before the device has been deleted (say by opening it)? We'll exit the device removal routines and then kill the module, because after the module code gets to ->exit(), nothing re-checks the module refcount, so the host module will get free'd while we're still using the device. The fix for this seems to be to differentiate between special uses of scsi_get_device, which are allowed to get the device in the module exit routines and ordinary uses which aren't. Something like this? (the patch isn't complete, but you get the idea). James --- diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 08c90a7..31ba254 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -965,6 +965,15 @@ int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer, } EXPORT_SYMBOL(scsi_report_opcode); +static int __scsi_device_get_common(struct scsi_device *sdev) +{ + if (sdev->sdev_state == SDEV_DEL) + return -ENXIO; + if (!get_device(&sdev->sdev_gendev)) + return -ENXIO; + return 0; +} + /** * scsi_device_get - get an additional reference to a scsi_device * @sdev: device to get a reference to @@ -975,19 +984,45 @@ EXPORT_SYMBOL(scsi_report_opcode); */ int scsi_device_get(struct scsi_device *sdev) { - if (sdev->sdev_state == SDEV_DEL) - return -ENXIO; - if (!get_device(&sdev->sdev_gendev)) - return -ENXIO; - /* We can fail this if we're doing SCSI operations -* from module exit (like cache flush) */ - try_module_get(sdev->host->hostt->module); + int ret; - return 0; + ret = __scsi_device_get_common(sdev); + if (ret) + return ret; + + ret = try_module_get(sdev->host->hostt->module); + + if (ret) + put_device(&sdev->sdev_gendev); + + return ret; } EXPORT_SYMBOL(scsi_device_get); /** + * scsi_device_get_in_module_exit() - get an additional reference to a scsi_device + * @sdev: device to get a reference to + * + * Functions identically to scsi_device_get() except that it unconditionally + * gets the module reference. This allows it to be called from module exit + * routines where scsi_device_get() will fail. This routine is still paired + * with scsi_device_put(). + */ +int scsi_device_get_in_module_exit(struct scsi_device *sdev) +{ + int ret; + + ret = __scsi_device_get_common(sdev); + if (ret) + return ret; + + __module_get(sdev->host->hostt->module); + + return 0; +} +EXPORT_SYMBOL(scsi_device_get_in_module_exit); + +/** * scsi_device_put - release a reference to a scsi_device * @sdev: device to release a reference on. * diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index ebf35cb6..057604e 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -564,16 +564,22 @@ static int sd_major(int major_idx) } } -static struct scsi_disk *__scsi_disk_get(struct gendisk *disk) +static struct scsi_disk *__scsi_disk_get(struct gendisk *disk, int in_exit) { struct scsi_disk *sdkp = NULL; if (disk->private_data) { + int ret; + sdkp = scsi_disk(disk); - if (scsi_device_get(sdkp->device) == 0) - get_device(&sdkp->dev); + if (in_exit) + ret = scsi_device_get_in_module_exit(sdkp->device); else + ret = scsi_device_get(sdkp->device); +
[PATCH v2 00/48] hpsa driver updates
This patch set is based on Linus's tree. The changes are: - correct sparse warnings - correct memory leaks - correct kdump issues - correct queue depth updates - cleanup function return values - cleanup and enhance messages - cleanup and enhance error handling - minor code refactoring - performance enhancements. - removal of command queueing in driver - elimination of race conditions around aborts - change to internal driver workqueues --- Don Brace (12): hpsa: correct endian sparse warnings hpsa: change how SA controllers are reset hpsa: correct change_queue_depth hpsa: do not queue commands internally in driver hpsa: use workqueue to resubmit failed ioaccel commands hpsa: honor queue depth of physical devices hpsa: count passthru cmds with atomics, not a spin locked int hpsa: slightly optimize SA5_performant_completed hpsa: return failed from device reset/abort handlers hpsa: add in gen9 controller model names hpsa: add in P840ar controller model name hpsa: Use local workqueues instead of system workqueues Fabian Frederick (1): hpsa: Fix -Wunused-but-set-variable warning Robert Elliott (16): hpsa: adjust RAID-1, RAID-1ADM, and RAID-6 names hpsa: rename free_irqs to hpsa_free_irqs hpsa: notice all request_irq errors hpsa: remove 0x from queue depth print which is in decimal hpsa: propagate hard_reset failures in reset_devices mode hpsa: propagate return value from board ID lookup hpsa: downgrade the Waiting for no-op print to dev_info hpsa: report failure to ioremap config table hpsa: rename hpsa_request_irq to hpsa_request_irqs hpsa: pass error from pci_set_consistent_dma_mask from hpsa_message hpsa: report allocation failures while allocating SG chain blocks hpsa: fix memory leak in hpsa_alloc_cmd_pool hpsa: avoid unneccesary calls to resource freeing functions hpsa: optimize cmd_alloc function by remembering last allocation hpsa: shorten the wait for the CISS doorbell mode change ack hpsa: detect and report failures changing controller transport modes Stephen Cameron (13): hpsa: trivial message and comment clean ups hpsa: reserve some commands for use by driver hpsa: get rid of cmd_special_alloc and cmd_special_free hpsa: do not request device rescan on every ioaccel path error hpsa: factor out hpsa_ciss_submit function hpsa: do not check for msi(x) in interrupt_pending hpsa: remove incorrect BUG_ONs checking for raid offload enable hpsa: do not ack controller events on controllers that do not support it hpsa: guard against overflowing raid map array hpsa: check for ctlr lockup after command allocation in main io path hpsa: do not use a void pointer for scsi_cmd field of struct CommandList hpsa: print CDBs instead of kernel virtual addresses for uncommon errors hpsa: do not use function pointers in fast path command submission Tomas Henzl (2): hpsa: fix memory leak in kdump hard reset hpsa: turn off interrupts when kdump starts Webb Scales (4): hpsa: refactor hpsa_find_board_params() to encapsulate legacy test hpsa: fix race between abort handler and main i/o path hpsa: move SG descriptor set-up out of hpsa_scatter_gather() hpsa: refactor duplicated scan completion code into a new routine drivers/scsi/hpsa.c | 1858 --- drivers/scsi/hpsa.h | 61 +- drivers/scsi/hpsa_cmd.h | 334 ++-- 3 files changed, 1203 insertions(+), 1050 deletions(-) -- Signature -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/48] hpsa: correct endian sparse warnings
Correct endiness issues reported by sparse. SA controllers are little endian. This patch ensures endiness correctness. Signed-off-by: Don Brace Reviewed-by: Scott Teel Reviewed-by: Webb Scales --- drivers/scsi/hpsa.c | 275 +++ drivers/scsi/hpsa_cmd.h | 172 +++-- 2 files changed, 198 insertions(+), 249 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 6bb4611..2b9baea 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -50,6 +50,7 @@ #include #include #include +#include #include #include "hpsa_cmd.h" #include "hpsa.h" @@ -229,7 +230,7 @@ static void check_ioctl_unit_attention(struct ctlr_info *h, struct CommandList *c); /* performant mode helper functions */ static void calc_bucket_map(int *bucket, int num_buckets, - int nsgs, int min_blocks, int *bucket_map); + int nsgs, int min_blocks, u32 *bucket_map); static void hpsa_put_ctlr_into_performant_mode(struct ctlr_info *h); static inline u32 next_command(struct ctlr_info *h, u8 q); static int hpsa_find_cfg_addrs(struct pci_dev *pdev, void __iomem *vaddr, @@ -919,7 +920,7 @@ static int hpsa_scsi_add_entry(struct ctlr_info *h, int hostno, /* If this device a non-zero lun of a multi-lun device * byte 4 of the 8-byte LUN addr will contain the logical -* unit no, zero otherise. +* unit no, zero otherwise. */ if (device->scsi3addr[4] == 0) { /* This is not a non-zero lun of a multi-lun device */ @@ -1504,7 +1505,7 @@ static int hpsa_map_sg_chain_block(struct ctlr_info *h, chain_block = h->cmd_sg_list[c->cmdindex]; chain_sg->Ext = cpu_to_le32(HPSA_SG_CHAIN); chain_len = sizeof(*chain_sg) * - (c->Header.SGTotal - h->max_cmd_sg_entries); + (le16_to_cpu(c->Header.SGTotal) - h->max_cmd_sg_entries); chain_sg->Len = cpu_to_le32(chain_len); temp64 = pci_map_single(h->pdev, chain_block, chain_len, PCI_DMA_TODEVICE); @@ -1693,7 +1694,7 @@ static void complete_scsi_command(struct CommandList *cp) scsi_dma_unmap(cmd); /* undo the DMA mappings */ if ((cp->cmd_type == CMD_SCSI) && - (cp->Header.SGTotal > h->max_cmd_sg_entries)) + (le16_to_cpu(cp->Header.SGTotal) > h->max_cmd_sg_entries)) hpsa_unmap_sg_chain_block(h, cp); cmd->result = (DID_OK << 16); /* host byte */ @@ -1726,8 +1727,10 @@ static void complete_scsi_command(struct CommandList *cp) */ if (cp->cmd_type == CMD_IOACCEL1) { struct io_accel1_cmd *c = &h->ioaccel_cmd_pool[cp->cmdindex]; - cp->Header.SGList = cp->Header.SGTotal = scsi_sg_count(cmd); - cp->Request.CDBLen = c->io_flags & IOACCEL1_IOFLAGS_CDBLEN_MASK; + cp->Header.SGList = scsi_sg_count(cmd); + cp->Header.SGTotal = cpu_to_le16(cp->Header.SGList); + cp->Request.CDBLen = le16_to_cpu(c->io_flags) & + IOACCEL1_IOFLAGS_CDBLEN_MASK; cp->Header.tag = c->tag; memcpy(cp->Header.LUN.LunAddrBytes, c->CISS_LUN, 8); memcpy(cp->Request.CDB, c->CDB, cp->Request.CDBLen); @@ -2191,15 +2194,13 @@ static void hpsa_debug_map_buff(struct ctlr_info *h, int rc, le16_to_cpu(map_buff->row_cnt)); dev_info(&h->pdev->dev, "layout_map_count = %u\n", le16_to_cpu(map_buff->layout_map_count)); - dev_info(&h->pdev->dev, "flags = %u\n", + dev_info(&h->pdev->dev, "flags = 0x%x\n", le16_to_cpu(map_buff->flags)); - if (map_buff->flags & RAID_MAP_FLAG_ENCRYPT_ON) - dev_info(&h->pdev->dev, "encrypytion = ON\n"); - else - dev_info(&h->pdev->dev, "encrypytion = OFF\n"); + dev_info(&h->pdev->dev, "encrypytion = %s\n", + le16_to_cpu(map_buff->flags) & + RAID_MAP_FLAG_ENCRYPT_ON ? "ON" : "OFF"); dev_info(&h->pdev->dev, "dekindex = %u\n", le16_to_cpu(map_buff->dekindex)); - map_cnt = le16_to_cpu(map_buff->layout_map_count); for (map = 0; map < map_cnt; map++) { dev_info(&h->pdev->dev, "Map%u:\n", map); @@ -2741,8 +2742,8 @@ static int hpsa_get_pdisk_of_ioaccel2(struct ctlr_info *h, struct scsi_cmnd *scmd; /* scsi command within request being aborted */ struct hpsa_scsi_dev_t *d; /* device of request being aborted */ struct io_accel2_cmd *c2a; /* ioaccel2 command to abort */ - u32 it_nexus; /* 4 byte device handle for the ioaccel2 cmd */ - u32 scsi_nexus; /* 4 byte device handle for the ioaccel2 cmd */ + __le32 it_nexus;/* 4 byte device handle for the ioaccel2 cmd */ + __le32 scsi_nexus; /* 4 byte device handle for t
[PATCH v2 11/48] hpsa: propagate hard_reset failures in reset_devices mode
From: Robert Elliott Return the real reason for kdump_hard_reset failure rather than change them all to -ENODEV. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index fbeef5b..92ac76a 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6416,11 +6416,8 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev) * "performant mode". Or, it might be 640x, which can't reset * due to concerns about shared bbwc between 6402/6404 pair. */ - if (rc) { - if (rc != -ENOTSUPP) /* just try to do the kdump anyhow. */ - rc = -ENODEV; + if (rc) goto out_disable; - } /* Now try to get the controller to respond to a no-op */ dev_warn(&pdev->dev, "Waiting for controller to respond to no-op\n"); -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/48] hpsa: remove 0x from queue depth print which is in decimal
From: Robert Elliott The queue depth printed at startup is in decimal, so shouldn't have a 0x prefix. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 97bb718..fbeef5b 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5931,7 +5931,7 @@ static void print_cfg_table(struct device *dev, struct CfgTable __iomem *tb) readl(&(tb->HostWrite.CoalIntDelay))); dev_info(dev, " Coalesce Interrupt Count = 0x%x\n", readl(&(tb->HostWrite.CoalIntCount))); - dev_info(dev, " Max outstanding commands = 0x%d\n", + dev_info(dev, " Max outstanding commands = %d\n", readl(&(tb->CmdsOutMax))); dev_info(dev, " Bus Types = 0x%x\n", readl(&(tb->BusTypes))); for (i = 0; i < 16; i++) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/48] hpsa: adjust RAID-1, RAID-1ADM, and RAID-6 names
From: Robert Elliott HP now uses RAID-6 rather than RAID-ADG (Advanced Data Guarding) as the marketing name for our implementation of RAID-6. The driver considers RAID-1 and RAID-1+0 to be the same level, and considers RAID-1ADM and RAID-1+0ADM to be the same level. Parenthesis can be used to reflect the optional +0 portion of both those RAID levels. Rename: RAID-ADG to RAID-6 RAID-1(1+0) to RAID-1(+0) RAID-1(ADM) to RAID-1(+0)ADM Also, add another const after the pointer type as suggested by checkpatch.pl so the array is: static const char * const raid_label[] Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 7dfe829..4252b63 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -507,8 +507,8 @@ static inline int is_logical_dev_addr_mode(unsigned char scsi3addr[]) return (scsi3addr[3] & 0xC0) == 0x40; } -static const char *raid_label[] = { "0", "4", "1(1+0)", "5", "5+1", "ADG", - "1(ADM)", "UNKNOWN" +static const char * const raid_label[] = { "0", "4", "1(+0)", "5", "5+1", "6", + "1(+0)ADM", "UNKNOWN" }; #define HPSA_RAID_00 #define HPSA_RAID_41 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/48] hpsa: correct change_queue_depth
We change drive queue depths to match drive reported queue depths. The name of the SML function was changed from scsi_adjust_queue_depth changed to scsi_change_queue_depth. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 371d0a8..7dfe829 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -217,6 +217,7 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *h, struct scsi_cmnd *cmd); static void hpsa_scan_start(struct Scsi_Host *); static int hpsa_scan_finished(struct Scsi_Host *sh, unsigned long elapsed_time); +static int hpsa_change_queue_depth(struct scsi_device *sdev, int qdepth); static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd); static int hpsa_eh_abort_handler(struct scsi_cmnd *scsicmd); @@ -672,7 +673,7 @@ static struct scsi_host_template hpsa_driver_template = { .queuecommand = hpsa_scsi_queue_command, .scan_start = hpsa_scan_start, .scan_finished = hpsa_scan_finished, - .change_queue_depth = scsi_change_queue_depth, + .change_queue_depth = hpsa_change_queue_depth, .this_id= -1, .use_clustering = ENABLE_CLUSTERING, .eh_abort_handler = hpsa_eh_abort_handler, @@ -4007,6 +4008,19 @@ static void hpsa_scan_start(struct Scsi_Host *sh) spin_unlock_irqrestore(&h->scan_lock, flags); } +static int hpsa_change_queue_depth(struct scsi_device *sdev, int qdepth) +{ + struct ctlr_info *h = sdev_to_hba(sdev); + + if (qdepth < 1) + qdepth = 1; + else + if (qdepth > h->nr_cmds) + qdepth = h->nr_cmds; + scsi_change_queue_depth(sdev, qdepth); + return sdev->queue_depth; +} + static int hpsa_scan_finished(struct Scsi_Host *sh, unsigned long elapsed_time) { -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/48] hpsa: downgrade the Waiting for no-op print to dev_info
From: Robert Elliott There is nothing worrisome about the "Waiting for controller to respond to no-op" print, so use dev_info rather than dev_warn. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 85b3d73..ec2503a 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6424,7 +6424,7 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev) goto out_disable; /* Now try to get the controller to respond to a no-op */ - dev_warn(&pdev->dev, "Waiting for controller to respond to no-op\n"); + dev_info(&pdev->dev, "Waiting for controller to respond to no-op\n"); for (i = 0; i < HPSA_POST_RESET_NOOP_RETRIES; i++) { if (hpsa_noop(pdev) == 0) break; -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/48] hpsa: Fix -Wunused-but-set-variable warning
From: Fabian Frederick Remove unused variable in hpsa_free_cmd_pool. Reviewed-by: Scott Teel Signed-off-by: Fabian Frederick Acked-by: Don Brace Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index dc81d88..11d21ef 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6482,11 +6482,11 @@ static void hpsa_free_cmd_pool(struct ctlr_info *h) static void hpsa_irq_affinity_hints(struct ctlr_info *h) { - int i, cpu, rc; + int i, cpu; cpu = cpumask_first(cpu_online_mask); for (i = 0; i < h->msix_vector; i++) { - rc = irq_set_affinity_hint(h->intr[i], get_cpu_mask(cpu)); + irq_set_affinity_hint(h->intr[i], get_cpu_mask(cpu)); cpu = cpumask_next(cpu, cpu_online_mask); } } -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/48] hpsa: fix memory leak in kdump hard reset
From: Tomas Henzl There is a potential memory leak in hpsa_kdump_hard_reset_controller. Reviewed-by: Don Brace Reviewed-by: Scott Teel Signed-off-by: Tomas Henzl Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 2b9baea..d97f455 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5840,7 +5840,7 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev) } rc = write_driver_ver_to_cfgtable(cfgtable); if (rc) - goto unmap_vaddr; + goto unmap_cfgtable; /* If reset via doorbell register is supported, use that. * There are two such methods. Favor the newest method. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 12/48] hpsa: propagate return value from board ID lookup
From: Robert Elliott If the board ID lookup function fails, return the return code rather than return -ENODEV. The only board ID failure reason right now is -ENODEV, so this just provides more informative prints in kdump and adapts to future changes. Tested with error injection while booting with reset_devices on the kernel command line: [ 62.804324] injecting error in inj_hpsa_lookup_board_id: 1 11 [ 62.804423] hpsa :04:00.0: Board ID not found (the pci probe layer does not print an additional message if -ENODEV is the reason) Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 92ac76a..85b3d73 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5812,8 +5812,12 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev) */ rc = hpsa_lookup_board_id(pdev, &board_id); - if (rc < 0 || !ctlr_is_resettable(board_id)) { - dev_warn(&pdev->dev, "Not resetting device.\n"); + if (rc < 0) { + dev_warn(&pdev->dev, "Board ID not found\n"); + return rc; + } + if (!ctlr_is_resettable(board_id)) { + dev_warn(&pdev->dev, "Controller not resettable\n"); return -ENODEV; } @@ -6295,7 +6299,7 @@ static int hpsa_pci_init(struct ctlr_info *h) prod_index = hpsa_lookup_board_id(h->pdev, &h->board_id); if (prod_index < 0) - return -ENODEV; + return prod_index; h->product_name = products[prod_index].product_name; h->access = *(products[prod_index].access); -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/48] hpsa: notice all request_irq errors
From: Robert Elliott In MSI and MSI-X mode, where hpsa asks for more than one interrupt, hpsa_request_irqs forgets if the first request_irq call failed if later ones succeed. It needs to exit the loop on any failure rather than continue, freeing all irqs that were requested until that point. Also, it needs to clear out the q numbers up to MAX_REPLY_QUEUES. The same is true for the general hpsa_free_irqs function. Tested with error injection of -ENOSYS on the 4th call: [9.277691] injecting error in inj_request_irq: 1 4 [9.277780] hpsa :02:00.0: failed to get irq 35 for hpsa1 [ 10.711623] scsi host1: Error handler scsi_eh_1 exiting [ 10.739170] hpsa: probe of :02:00.0 failed with error -38 Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 11d21ef..97bb718 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6508,6 +6508,8 @@ static void hpsa_free_irqs(struct ctlr_info *h) irq_set_affinity_hint(h->intr[i], NULL); free_irq(h->intr[i], &h->q[i]); } + for (; i < MAX_REPLY_QUEUES; i++) + h->q[i] = 0; } static int hpsa_request_irq(struct ctlr_info *h, @@ -6525,10 +6527,25 @@ static int hpsa_request_irq(struct ctlr_info *h, if (h->intr_mode == PERF_MODE_INT && h->msix_vector > 0) { /* If performant mode and MSI-X, use multiple reply queues */ - for (i = 0; i < h->msix_vector; i++) + for (i = 0; i < h->msix_vector; i++) { rc = request_irq(h->intr[i], msixhandler, 0, h->devname, &h->q[i]); + if (rc) { + int j; + + dev_err(&h->pdev->dev, + "failed to get irq %d for %s\n", + h->intr[i], h->devname); + for (j = 0; j < i; j++) { + free_irq(h->intr[j], &h->q[j]); + h->q[j] = 0; + } + for (; j < MAX_REPLY_QUEUES; j++) + h->q[j] = 0; + return rc; + } + } hpsa_irq_affinity_hints(h); } else { /* Use single reply pool */ -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/48] hpsa: rename free_irqs to hpsa_free_irqs
From: Robert Elliott Change the function names to have hpsa prefix. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 43 ++- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 4252b63..dc81d88 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6491,6 +6491,25 @@ static void hpsa_irq_affinity_hints(struct ctlr_info *h) } } +/* clear affinity hints and free MSI-X, MSI, or legacy INTx vectors */ +static void hpsa_free_irqs(struct ctlr_info *h) +{ + int i; + + if (!h->msix_vector || h->intr_mode != PERF_MODE_INT) { + /* Single reply queue, only one irq to free */ + i = h->intr_mode; + irq_set_affinity_hint(h->intr[i], NULL); + free_irq(h->intr[i], &h->q[i]); + return; + } + + for (i = 0; i < h->msix_vector; i++) { + irq_set_affinity_hint(h->intr[i], NULL); + free_irq(h->intr[i], &h->q[i]); + } +} + static int hpsa_request_irq(struct ctlr_info *h, irqreturn_t (*msixhandler)(int, void *), irqreturn_t (*intxhandler)(int, void *)) @@ -6555,27 +6574,9 @@ static int hpsa_kdump_soft_reset(struct ctlr_info *h) return 0; } -static void free_irqs(struct ctlr_info *h) -{ - int i; - - if (!h->msix_vector || h->intr_mode != PERF_MODE_INT) { - /* Single reply queue, only one irq to free */ - i = h->intr_mode; - irq_set_affinity_hint(h->intr[i], NULL); - free_irq(h->intr[i], &h->q[i]); - return; - } - - for (i = 0; i < h->msix_vector; i++) { - irq_set_affinity_hint(h->intr[i], NULL); - free_irq(h->intr[i], &h->q[i]); - } -} - static void hpsa_free_irqs_and_disable_msix(struct ctlr_info *h) { - free_irqs(h); + hpsa_free_irqs(h); #ifdef CONFIG_PCI_MSI if (h->msix_vector) { if (h->pdev->msix_enabled) @@ -6937,7 +6938,7 @@ reinit_after_soft_reset: spin_lock_irqsave(&h->lock, flags); h->access.set_intr_mask(h, HPSA_INTR_OFF); spin_unlock_irqrestore(&h->lock, flags); - free_irqs(h); + hpsa_free_irqs(h); rc = hpsa_request_irq(h, hpsa_msix_discard_completions, hpsa_intx_discard_completions); if (rc) { @@ -6997,7 +6998,7 @@ reinit_after_soft_reset: clean4: hpsa_free_sg_chain_blocks(h); hpsa_free_cmd_pool(h); - free_irqs(h); + hpsa_free_irqs(h); clean2: clean1: if (h->lockup_detected) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/48] hpsa: turn off interrupts when kdump starts
From: Tomas Henzl Sometimes when the card is restarted it may cause - "irq 16: nobody cared (try booting with the "irqpoll" option)" that is likely caused so, that the card, after the hard reset finishes, pulls on the irq. Disabling the ints before or after the hpsa_kdump_hard_reset_controller fixes it. At this point we can't know in which state the card is, so using SA5_INTR_OFF + SA5_REPLY_INTR_MASK_OFFSET defines directly, instead of the function the drivers provides, seems to be apropriate. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index d97f455..9edacff 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6370,6 +6370,7 @@ static void hpsa_hba_inquiry(struct ctlr_info *h) static int hpsa_init_reset_devices(struct pci_dev *pdev) { int rc, i; + void __iomem *vaddr; if (!reset_devices) return 0; @@ -6393,6 +6394,14 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev) pci_set_master(pdev); + vaddr = pci_ioremap_bar(pdev, 0); + if (vaddr == NULL) { + rc = -ENOMEM; + goto out_disable; + } + writel(SA5_INTR_OFF, vaddr + SA5_REPLY_INTR_MASK_OFFSET); + iounmap(vaddr); + /* Reset the controller with a PCI power-cycle or via doorbell */ rc = hpsa_kdump_hard_reset_controller(pdev); -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/48] hpsa: change how SA controllers are reset
Change how SA controllers are reset by changing PCI power levels. The hpsa driver was finding the PCI_PM_CTRL_STATE_MASK offset then reading/writing a bitmask to change the power state. There are kernel functions that do the same operations. Better to use the kernel functions. Signed-off-by: Don Brace Reviewed-by: Scott Teel Reviewed-by: Webb Scales --- drivers/scsi/hpsa.c | 28 ++-- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 9edacff..371d0a8 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5689,30 +5689,22 @@ static int hpsa_controller_hard_reset(struct pci_dev *pdev, * the controller, place the interface device in D3 then to D0, * this causes a secondary PCI reset which will reset the * controller." */ - int pos; - u16 pmcsr; - - pos = pci_find_capability(pdev, PCI_CAP_ID_PM); - if (pos == 0) { - dev_err(&pdev->dev, - "hpsa_reset_controller: " - "PCI PM not supported\n"); - return -ENODEV; - } + + int rc = 0; + dev_info(&pdev->dev, "using PCI PM to reset controller\n"); + /* enter the D3hot power management state */ - pci_read_config_word(pdev, pos + PCI_PM_CTRL, - (__force u16 *)&pmcsr); - pmcsr &= ~PCI_PM_CTRL_STATE_MASK; - pmcsr |= (__force u16) PCI_D3hot; - pci_write_config_word(pdev, pos + PCI_PM_CTRL, pmcsr); + rc = pci_set_power_state(pdev, PCI_D3hot); + if (rc) + return rc; msleep(500); /* enter the D0 power management state */ - pmcsr &= ~PCI_PM_CTRL_STATE_MASK; - pmcsr |= (__force u16) PCI_D0; - pci_write_config_word(pdev, pos + PCI_PM_CTRL, pmcsr); + rc = pci_set_power_state(pdev, PCI_D0); + if (rc) + return rc; /* * The P600 requires a small delay when changing states. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 19/48] hpsa: report allocation failures while allocating SG chain blocks
From: Robert Elliott Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 64d17d1..dc328ce 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -1480,13 +1480,17 @@ static int hpsa_allocate_sg_chain_blocks(struct ctlr_info *h) h->cmd_sg_list = kzalloc(sizeof(*h->cmd_sg_list) * h->nr_cmds, GFP_KERNEL); - if (!h->cmd_sg_list) + if (!h->cmd_sg_list) { + dev_err(&h->pdev->dev, "Failed to allocate SG list\n"); return -ENOMEM; + } for (i = 0; i < h->nr_cmds; i++) { h->cmd_sg_list[i] = kmalloc(sizeof(*h->cmd_sg_list[i]) * h->chainsize, GFP_KERNEL); - if (!h->cmd_sg_list[i]) + if (!h->cmd_sg_list[i]) { + dev_err(&h->pdev->dev, "Failed to allocate cmd SG\n"); goto clean; + } } 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
[PATCH v2 22/48] hpsa: reserve some commands for use by driver
From: Stephen Cameron We need to reserve some commands for device rescans, aborts, and the pass through ioctls, etc. so we cannot give them all to the scsi mid layer. This is in preparation for removing cmd_special_alloc and cmd_special_free so that we can stop queuing commands internally in the driver so that we can remove the locks thta protect the queue that we will no longer have. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |7 +-- drivers/scsi/hpsa.h |2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index bee24b2..99c32a0 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4064,11 +4064,14 @@ static int hpsa_register_scsi(struct ctlr_info *h) sh->max_cmd_len = MAX_COMMAND_SIZE; sh->max_lun = HPSA_MAX_LUN; sh->max_id = HPSA_MAX_LUN; - sh->can_queue = h->nr_cmds; + sh->can_queue = h->nr_cmds - + HPSA_CMDS_RESERVED_FOR_ABORTS - + HPSA_CMDS_RESERVED_FOR_DRIVER - + HPSA_MAX_CONCURRENT_PASSTHRUS; if (h->hba_mode_enabled) sh->cmd_per_lun = 7; else - sh->cmd_per_lun = h->nr_cmds; + sh->cmd_per_lun = sh->can_queue; sh->sg_tablesize = h->maxsgentries; h->scsi_host = sh; sh->hostdata[0] = (unsigned long) h; diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 8e06d9e..5ee6c6a 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -115,6 +115,8 @@ struct ctlr_info { void __iomem *vaddr; unsigned long paddr; int nr_cmds; /* Number of commands allowed on this controller */ +#define HPSA_CMDS_RESERVED_FOR_ABORTS 2 +#define HPSA_CMDS_RESERVED_FOR_DRIVER 1 struct CfgTable __iomem *cfgtable; int interrupts_enabled; int max_commands; -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/48] hpsa: refactor hpsa_find_board_params() to encapsulate legacy test
From: Webb Scales Encapsulate the conditional predicate which tests for legacy controllers in a separate function and rework the code comments. Reviewed-by: Scott Teel Signed-off-by: Webb Scales Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index ec2503a..08d96a9 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3107,7 +3107,8 @@ out: kfree(logdev_list); } -/* hpsa_scatter_gather takes a struct scsi_cmnd, (cmd), and does the pci +/* + * hpsa_scatter_gather takes a struct scsi_cmnd, (cmd), and does the pci * dma mapping and fills in the scatter gather entries of the * hpsa command, cp. */ @@ -3165,7 +3166,7 @@ static int hpsa_scatter_gather(struct ctlr_info *h, sglist_finished: cp->Header.SGList = (u8) use_sg; /* no. SGs contig in this cmd */ - cp->Header.SGTotal = cpu_to_le16(use_sg); /* total sgs in this cmd list */ + cp->Header.SGTotal = cpu_to_le16(use_sg); /* total sgs in cmd list */ return 0; } @@ -6162,6 +6163,15 @@ static void hpsa_get_max_perf_mode_cmds(struct ctlr_info *h) } } +/* If the controller reports that the total max sg entries is greater than 512, + * then we know that chained SG blocks work. (Original smart arrays did not + * support chained SG blocks and would return zero for max sg entries.) + */ +static int hpsa_supports_chained_sg_blocks(struct ctlr_info *h) +{ + return h->maxsgentries > 512; +} + /* Interrogate the hardware for some limits: * max commands, max SG elements without chaining, and with chaining, * SG chain block size, etc. @@ -6172,18 +6182,20 @@ static void hpsa_find_board_params(struct ctlr_info *h) h->nr_cmds = h->max_commands - 4; /* Allow room for some ioctls */ h->maxsgentries = readl(&(h->cfgtable->MaxScatterGatherElements)); h->fw_support = readl(&(h->cfgtable->misc_fw_support)); - /* -* Limit in-command s/g elements to 32 save dma'able memory. -* Howvever spec says if 0, use 31 -*/ - h->max_cmd_sg_entries = 31; - if (h->maxsgentries > 512) { + if (hpsa_supports_chained_sg_blocks(h)) { + /* Limit in-command s/g elements to 32 save dma'able memory. */ h->max_cmd_sg_entries = 32; h->chainsize = h->maxsgentries - h->max_cmd_sg_entries; h->maxsgentries--; /* save one for chain pointer */ } else { - h->chainsize = 0; + /* +* Original smart arrays supported at most 31 s/g entries +* embedded inline in the command (trying to use more +* would lock up the controller) +*/ + h->max_cmd_sg_entries = 31; h->maxsgentries = 31; /* default to traditional values */ + h->chainsize = 0; } /* Find out what task management functions are supported and cache */ -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/48] hpsa: rename hpsa_request_irq to hpsa_request_irqs
From: Robert Elliott Make the function name more descriptive. We use more than one interrupt. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 354e7f8..f29f569 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6526,7 +6526,8 @@ static void hpsa_free_irqs(struct ctlr_info *h) h->q[i] = 0; } -static int hpsa_request_irq(struct ctlr_info *h, +/* returns 0 on success; cleans up and returns -Enn on error */ +static int hpsa_request_irqs(struct ctlr_info *h, irqreturn_t (*msixhandler)(int, void *), irqreturn_t (*intxhandler)(int, void *)) { @@ -6934,7 +6935,7 @@ reinit_after_soft_reset: /* make sure the board interrupts are off */ h->access.set_intr_mask(h, HPSA_INTR_OFF); - if (hpsa_request_irq(h, do_hpsa_intr_msi, do_hpsa_intr_intx)) + if (hpsa_request_irqs(h, do_hpsa_intr_msi, do_hpsa_intr_intx)) goto clean2; dev_info(&pdev->dev, "%s: <0x%x> at IRQ %d%s using DAC\n", h->devname, pdev->device, @@ -6970,11 +6971,11 @@ reinit_after_soft_reset: h->access.set_intr_mask(h, HPSA_INTR_OFF); spin_unlock_irqrestore(&h->lock, flags); hpsa_free_irqs(h); - rc = hpsa_request_irq(h, hpsa_msix_discard_completions, + rc = hpsa_request_irqs(h, hpsa_msix_discard_completions, hpsa_intx_discard_completions); if (rc) { - dev_warn(&h->pdev->dev, "Failed to request_irq after " - "soft reset.\n"); + dev_warn(&h->pdev->dev, + "Failed to request_irq after soft reset.\n"); goto clean4; } -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 20/48] hpsa: fix memory leak in hpsa_alloc_cmd_pool
From: Robert Elliott Partial allocation failure wasn't handled correctly Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index dc328ce..a66a50e 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -211,6 +211,7 @@ static struct CommandList *cmd_special_alloc(struct ctlr_info *h); static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h, void *buff, size_t size, u16 page_code, unsigned char *scsi3addr, int cmd_type); +static void hpsa_free_cmd_pool(struct ctlr_info *h); #define VPD_PAGE (1 << 8) static int hpsa_scsi_queue_command(struct Scsi_Host *h, struct scsi_cmnd *cmd); @@ -6471,9 +6472,12 @@ static int hpsa_allocate_cmd_pool(struct ctlr_info *h) || (h->cmd_pool == NULL) || (h->errinfo_pool == NULL)) { dev_err(&h->pdev->dev, "out of memory in %s", __func__); - return -ENOMEM; + goto clean_up; } return 0; +clean_up: + hpsa_free_cmd_pool(h); + return -ENOMEM; } static void hpsa_free_cmd_pool(struct ctlr_info *h) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 15/48] hpsa: trivial message and comment clean ups
From: Stephen Cameron Cleanup comments to be more specific. Make messages more informational. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 08d96a9..5973018 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5863,8 +5863,8 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev) } else { use_doorbell = misc_fw_support & MISC_FW_DOORBELL_RESET; if (use_doorbell) { - dev_warn(&pdev->dev, "Soft reset not supported. " - "Firmware update is required.\n"); + dev_warn(&pdev->dev, + "Soft reset not supported. Firmware update is required.\n"); rc = -ENOTSUPP; /* try soft reset */ goto unmap_cfgtable; } @@ -5884,8 +5884,7 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev) rc = hpsa_wait_for_board_state(pdev, vaddr, BOARD_READY); if (rc) { dev_warn(&pdev->dev, - "failed waiting for board to become ready " - "after hard reset\n"); + "Failed waiting for board to become ready after hard reset\n"); goto unmap_cfgtable; } @@ -5984,7 +5983,7 @@ static int find_PCI_BAR_index(struct pci_dev *pdev, unsigned long pci_bar_addr) } /* If MSI/MSI-X is supported by the kernel we will try to enable it on - * controllers that are capable. If not, we use IO-APIC mode. + * controllers that are capable. If not, we use legacy INTx mode. */ static void hpsa_interrupt_mode(struct ctlr_info *h) @@ -6003,7 +6002,7 @@ static void hpsa_interrupt_mode(struct ctlr_info *h) (h->board_id == 0x40820E11) || (h->board_id == 0x40830E11)) goto default_int_mode; if (pci_find_capability(h->pdev, PCI_CAP_ID_MSIX)) { - dev_info(&h->pdev->dev, "MSIX\n"); + dev_info(&h->pdev->dev, "MSI-X capable controller\n"); h->msix_vector = MAX_REPLY_QUEUES; if (h->msix_vector > num_online_cpus()) h->msix_vector = num_online_cpus(); @@ -6024,7 +6023,7 @@ static void hpsa_interrupt_mode(struct ctlr_info *h) } single_msi_mode: if (pci_find_capability(h->pdev, PCI_CAP_ID_MSI)) { - dev_info(&h->pdev->dev, "MSI\n"); + dev_info(&h->pdev->dev, "MSI capable controller\n"); if (!pci_enable_msi(h->pdev)) h->msi_vector = 1; else @@ -6209,7 +6208,7 @@ static void hpsa_find_board_params(struct ctlr_info *h) static inline bool hpsa_CISS_signature_present(struct ctlr_info *h) { if (!check_signature(h->cfgtable->Signature, "CISS", 4)) { - dev_warn(&h->pdev->dev, "not a valid CISS config table\n"); + dev_err(&h->pdev->dev, "not a valid CISS config table\n"); return false; } return true; @@ -6301,7 +6300,7 @@ static int hpsa_enter_simple_mode(struct ctlr_info *h) h->transMethod = CFGTBL_Trans_Simple; return 0; error: - dev_warn(&h->pdev->dev, "unable to get board into simple mode\n"); + dev_err(&h->pdev->dev, "failed to enter simple mode\n"); return -ENODEV; } @@ -7282,8 +7281,8 @@ static void hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support) hpsa_wait_for_mode_change_ack(h); register_value = readl(&(h->cfgtable->TransportActive)); if (!(register_value & CFGTBL_Trans_Performant)) { - dev_warn(&h->pdev->dev, "unable to get board into" - " performant mode\n"); + dev_err(&h->pdev->dev, + "performant mode problem - transport not active\n"); return; } /* Change the access methods to the performant access methods */ -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 18/48] hpsa: pass error from pci_set_consistent_dma_mask from hpsa_message
From: Robert Elliott Return the actual error code instead of a generic error code. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index f29f569..64d17d1 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5610,7 +5610,7 @@ static int hpsa_message(struct pci_dev *pdev, unsigned char opcode, err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32)); if (err) { iounmap(vaddr); - return -ENOMEM; + return err; } cmd = pci_alloc_consistent(pdev, cmd_sz, &paddr64); -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 16/48] hpsa: report failure to ioremap config table
From: Robert Elliott Enhance error reporting. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 5973018..354e7f8 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6130,8 +6130,10 @@ static int hpsa_find_cfgtables(struct ctlr_info *h) return rc; h->cfgtable = remap_pci_mem(pci_resource_start(h->pdev, cfg_base_addr_index) + cfg_offset, sizeof(*h->cfgtable)); - if (!h->cfgtable) + if (!h->cfgtable) { + dev_err(&h->pdev->dev, "Failed mapping cfgtable\n"); return -ENOMEM; + } rc = write_driver_ver_to_cfgtable(h->cfgtable); if (rc) return rc; -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 25/48] hpsa: do not request device rescan on every ioaccel path error
From: Stephen Cameron The original reasoning behind doing this was faulty. An error of some sort would be encountered, accelerated i/o would be disabled for that logical drive, the command would be kicked back out to the SCSI midlayer for a retry, and since i/o accelerator mode was disabled, it would get retried down the RAID path. However, something needs to turn ioaccellerator mode back on, and this rescan request was what did that. However, it was racy, and extremely bad for performance to rescan all devices, so, don't do that. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 14 ++ drivers/scsi/hpsa.h |1 - 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 70f07af..94a82e3 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -1637,21 +1637,19 @@ static void process_ioaccel2_completion(struct ctlr_info *h, c2->error_data.serv_response == IOACCEL2_SERV_RESPONSE_FAILURE) { dev->offload_enabled = 0; - h->drv_req_rescan = 1; /* schedule controller for a rescan */ cmd->result = DID_SOFT_ERROR << 16; cmd_free(h, c); cmd->scsi_done(cmd); return; } raid_retry = handle_ioaccel_mode2_error(h, c, cmd, c2); - /* If error found, disable Smart Path, schedule a rescan, -* and force a retry on the standard path. + /* If error found, disable Smart Path, +* force a retry on the standard path. */ if (raid_retry) { dev_warn(&h->pdev->dev, "%s: Retrying on standard path.\n", "HP SSD Smart Path"); dev->offload_enabled = 0; /* Disable Smart Path */ - h->drv_req_rescan = 1;/* schedule controller rescan */ cmd->result = DID_SOFT_ERROR << 16; } cmd_free(h, c); @@ -6478,9 +6476,6 @@ static void hpsa_ack_ctlr_events(struct ctlr_info *h) int i; char *event_type; - /* Clear the driver-requested rescan flag */ - h->drv_req_rescan = 0; - /* Ask the controller to clear the events we're handling. */ if ((h->transMethod & (CFGTBL_Trans_io_accel1 | CFGTBL_Trans_io_accel2)) && @@ -6526,9 +6521,6 @@ static void hpsa_ack_ctlr_events(struct ctlr_info *h) */ static int hpsa_ctlr_needs_rescan(struct ctlr_info *h) { - if (h->drv_req_rescan) - return 1; - if (!(h->fw_support & MISC_FW_EVENT_NOTIFY)) return 0; @@ -6574,7 +6566,6 @@ static void hpsa_monitor_ctlr_worker(struct work_struct *work) if (hpsa_ctlr_needs_rescan(h) || hpsa_offline_devices_ready(h)) { scsi_host_get(h->scsi_host); - h->drv_req_rescan = 0; hpsa_ack_ctlr_events(h); hpsa_scan_start(h->scsi_host); scsi_host_put(h->scsi_host); @@ -6743,7 +6734,6 @@ reinit_after_soft_reset: /* Enable Accelerated IO path at driver layer */ h->acciopath_status = 1; - h->drv_req_rescan = 0; /* Turn the interrupts on so we can service requests */ h->access.set_intr_mask(h, HPSA_INTR_ON); diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index e7d7eaa..06a3e81 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -235,7 +235,6 @@ struct ctlr_info { spinlock_t offline_device_lock; struct list_head offline_device_list; int acciopath_status; - int drv_req_rescan; /* flag for driver to request rescan event */ int raid_offload_debug; }; -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 24/48] hpsa: do not queue commands internally in driver
By not doing maintaining a list of queued commands, we can eliminate some spin locking in the main i/o path and gain significant improvement in IOPS. Remove the queuing code and the code that calls it; remove now-unused interrupt code; remove DIRECT_LOOKUP_BIT. Now that the passthru commands share the same command pool as the main i/o path, and the total size of the pool is less than or equal to the number of commands that will fit in the hardware fifo, there is no need to check to see if we are exceeding the hardware fifo's depth. Reviewed-by: Scott Teel Reviewed-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 306 +-- drivers/scsi/hpsa.h | 17 --- drivers/scsi/hpsa_cmd.h | 10 -- 3 files changed, 38 insertions(+), 295 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 26e3e5b..70f07af 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -196,8 +196,6 @@ static int number_of_controllers; static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id); static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id); static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg); -static void lock_and_start_io(struct ctlr_info *h); -static void start_io(struct ctlr_info *h, unsigned long *flags); #ifdef CONFIG_COMPAT static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, @@ -689,13 +687,6 @@ static struct scsi_host_template hpsa_driver_template = { .no_write_same = 1, }; - -/* Enqueuing and dequeuing functions for cmdlists. */ -static inline void addQ(struct list_head *list, struct CommandList *c) -{ - list_add_tail(&c->list, list); -} - static inline u32 next_command(struct ctlr_info *h, u8 q) { u32 a; @@ -829,8 +820,6 @@ static void dial_up_lockup_detection_on_fw_flash_complete(struct ctlr_info *h, static void enqueue_cmd_and_start_io(struct ctlr_info *h, struct CommandList *c) { - unsigned long flags; - switch (c->cmd_type) { case CMD_IOACCEL1: set_ioaccel1_performant_mode(h, c); @@ -842,18 +831,8 @@ static void enqueue_cmd_and_start_io(struct ctlr_info *h, set_performant_mode(h, c); } dial_down_lockup_detection_during_fw_flash(h, c); - spin_lock_irqsave(&h->lock, flags); - addQ(&h->reqQ, c); - h->Qdepth++; - start_io(h, &flags); - spin_unlock_irqrestore(&h->lock, flags); -} - -static inline void removeQ(struct CommandList *c) -{ - if (WARN_ON(list_empty(&c->list))) - return; - list_del_init(&c->list); + atomic_inc(&h->commands_outstanding); + h->access.submit_command(h, c); } static inline int is_hba_lunid(unsigned char scsi3addr[]) @@ -3449,8 +3428,7 @@ static int hpsa_scsi_ioaccel2_queue_command(struct ctlr_info *h, set_encrypt_ioaccel2(h, c, cp); cp->scsi_nexus = cpu_to_le32(ioaccel_handle); - cp->Tag = cpu_to_le32(c->cmdindex << DIRECT_LOOKUP_SHIFT | - DIRECT_LOOKUP_BIT); + cp->Tag = cpu_to_le32(c->cmdindex << DIRECT_LOOKUP_SHIFT); memcpy(cp->cdb, cdb, sizeof(cp->cdb)); /* fill in sg elements */ @@ -3831,10 +3809,7 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h, dev->scsi3addr); } -/* - * Running in struct Scsi_Host->host_lock less mode using LLD internal - * struct ctlr_info *h->lock w/ spin_lock_irqsave() protection. - */ +/* Running in struct Scsi_Host->host_lock less mode */ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) { struct ctlr_info *h; @@ -3898,8 +3873,7 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) c->Header.ReplyQueue = 0; /* unused in simple mode */ memcpy(&c->Header.LUN.LunAddrBytes[0], &scsi3addr[0], 8); - c->Header.tag = cpu_to_le64((c->cmdindex << DIRECT_LOOKUP_SHIFT) | - DIRECT_LOOKUP_BIT); + c->Header.tag = cpu_to_le64((c->cmdindex << DIRECT_LOOKUP_SHIFT)); /* Fill in the request block... */ @@ -4264,56 +4238,6 @@ static int hpsa_send_abort(struct ctlr_info *h, unsigned char *scsi3addr, return rc; } -/* - * hpsa_find_cmd_in_queue - * - * Used to determine whether a command (find) is still present - * in queue_head. Optionally excludes the last element of queue_head. - * - * This is used to avoid unnecessary aborts. Commands in h->reqQ have - * not yet been submitted, and so can be aborted by the driver without - * sending an abort to the hardware. - * - * Returns pointer to command if found in queue, NULL otherwise. - */ -static struct CommandList *hpsa_find_cmd_in_queue(struct ctlr_info *h, - struct scsi_cmnd *find, struct list_head *queue_head) -{ - unsigned long flags; - struct CommandList *c = NULL; /* ptr into cm
[PATCH v2 23/48] hpsa: get rid of cmd_special_alloc and cmd_special_free
From: Stephen Cameron We have commands reserved for internal use. This is laying the groundwork for removing the internal queue of commands from the driver so that the locks that protect that queue may be removed. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 106 ++- drivers/scsi/hpsa.h |2 - 2 files changed, 31 insertions(+), 77 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 99c32a0..26e3e5b 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -205,9 +205,7 @@ static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, #endif static void cmd_free(struct ctlr_info *h, struct CommandList *c); -static void cmd_special_free(struct ctlr_info *h, struct CommandList *c); static struct CommandList *cmd_alloc(struct ctlr_info *h); -static struct CommandList *cmd_special_alloc(struct ctlr_info *h); static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h, void *buff, size_t size, u16 page_code, unsigned char *scsi3addr, int cmd_type); @@ -2057,10 +2055,10 @@ static int hpsa_scsi_do_inquiry(struct ctlr_info *h, unsigned char *scsi3addr, struct CommandList *c; struct ErrorInfo *ei; - c = cmd_special_alloc(h); + c = cmd_alloc(h); if (c == NULL) {/* trouble... */ - dev_warn(&h->pdev->dev, "cmd_special_alloc returned NULL!\n"); + dev_warn(&h->pdev->dev, "cmd_alloc returned NULL!\n"); return -ENOMEM; } @@ -2076,7 +2074,7 @@ static int hpsa_scsi_do_inquiry(struct ctlr_info *h, unsigned char *scsi3addr, rc = -1; } out: - cmd_special_free(h, c); + cmd_free(h, c); return rc; } @@ -2088,10 +2086,9 @@ static int hpsa_bmic_ctrl_mode_sense(struct ctlr_info *h, struct CommandList *c; struct ErrorInfo *ei; - c = cmd_special_alloc(h); - + c = cmd_alloc(h); if (c == NULL) {/* trouble... */ - dev_warn(&h->pdev->dev, "cmd_special_alloc returned NULL!\n"); + dev_warn(&h->pdev->dev, "cmd_alloc returned NULL!\n"); return -ENOMEM; } @@ -2107,7 +2104,7 @@ static int hpsa_bmic_ctrl_mode_sense(struct ctlr_info *h, rc = -1; } out: - cmd_special_free(h, c); + cmd_free(h, c); return rc; } @@ -2118,10 +2115,10 @@ static int hpsa_send_reset(struct ctlr_info *h, unsigned char *scsi3addr, struct CommandList *c; struct ErrorInfo *ei; - c = cmd_special_alloc(h); + c = cmd_alloc(h); if (c == NULL) {/* trouble... */ - dev_warn(&h->pdev->dev, "cmd_special_alloc returned NULL!\n"); + dev_warn(&h->pdev->dev, "cmd_alloc returned NULL!\n"); return -ENOMEM; } @@ -2137,7 +2134,7 @@ static int hpsa_send_reset(struct ctlr_info *h, unsigned char *scsi3addr, hpsa_scsi_interpret_error(h, c); rc = -1; } - cmd_special_free(h, c); + cmd_free(h, c); return rc; } @@ -2245,26 +2242,26 @@ static int hpsa_get_raid_map(struct ctlr_info *h, struct CommandList *c; struct ErrorInfo *ei; - c = cmd_special_alloc(h); + c = cmd_alloc(h); if (c == NULL) { - dev_warn(&h->pdev->dev, "cmd_special_alloc returned NULL!\n"); + dev_warn(&h->pdev->dev, "cmd_alloc returned NULL!\n"); return -ENOMEM; } if (fill_cmd(c, HPSA_GET_RAID_MAP, h, &this_device->raid_map, sizeof(this_device->raid_map), 0, scsi3addr, TYPE_CMD)) { dev_warn(&h->pdev->dev, "Out of memory in hpsa_get_raid_map()\n"); - cmd_special_free(h, c); + cmd_free(h, c); return -ENOMEM; } hpsa_scsi_do_simple_cmd_with_retry(h, c, PCI_DMA_FROMDEVICE); ei = c->err_info; if (ei->CommandStatus != 0 && ei->CommandStatus != CMD_DATA_UNDERRUN) { hpsa_scsi_interpret_error(h, c); - cmd_special_free(h, c); + cmd_free(h, c); return -1; } - cmd_special_free(h, c); + cmd_free(h, c); /* @todo in the future, dynamically allocate RAID map memory */ if (le32_to_cpu(this_device->raid_map.structure_size) > @@ -2384,9 +2381,9 @@ static int hpsa_scsi_do_report_luns(struct ctlr_info *h, int logical, unsigned char scsi3addr[8]; struct ErrorInfo *ei; - c = cmd_special_alloc(h); + c = cmd_alloc(h); if (c == NULL) {/* trouble... */ - dev_err(&h->pdev->dev, "cmd_special_alloc returned NULL!\n"); + dev_err(&h->pdev->dev, "cmd_alloc returned NULL!\n");
[PATCH v2 21/48] hpsa: avoid unneccesary calls to resource freeing functions
From: Robert Elliott If hpsa_allocate_cmd_pool failed, we were calling two functions unnecessarily: hpsa_free_sg_chain_blocks(h); hpsa_free_cmd_pool(h); This didn't cause any problem, as those functions can tolerate being called when what they free hasn't been allocated (relevant pointers would be NULL) but it is potentially confusing. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index a66a50e..bee24b2 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6948,8 +6948,9 @@ reinit_after_soft_reset: dev_info(&pdev->dev, "%s: <0x%x> at IRQ %d%s using DAC\n", h->devname, pdev->device, h->intr[h->intr_mode], dac ? "" : " not"); - if (hpsa_allocate_cmd_pool(h)) - goto clean4; + rc = hpsa_allocate_cmd_pool(h); + if (rc) + goto clean2_and_free_irqs; if (hpsa_allocate_sg_chain_blocks(h)) goto clean4; init_waitqueue_head(&h->scan_wait_queue); @@ -7038,6 +7039,7 @@ reinit_after_soft_reset: clean4: hpsa_free_sg_chain_blocks(h); hpsa_free_cmd_pool(h); +clean2_and_free_irqs: hpsa_free_irqs(h); clean2: clean1: -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 30/48] hpsa: optimize cmd_alloc function by remembering last allocation
From: Robert Elliott Empirically, this improves performance slightly (~2% max IOPS) by allowing cmd_alloc to remember where it left off searching for free commands between calls instead of always starting its search at command 0. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |7 +-- drivers/scsi/hpsa.h |1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index c95a20c..72abcf3 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4649,9 +4649,10 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h) union u64bit temp64; dma_addr_t cmd_dma_handle, err_dma_handle; int refcount; - unsigned long offset = 0; + unsigned long offset; - /* There is some *extremely* small but non-zero chance that that + /* +* There is some *extremely* small but non-zero chance that that * multiple threads could get in here, and one thread could * be scanning through the list of bits looking for a free * one, but the free ones are always behind him, and other @@ -4662,6 +4663,7 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h) * infrequently as to be indistinguishable from never. */ + offset = h->last_allocation; /* benignly racy */ for (;;) { i = find_next_zero_bit(h->cmd_pool_bits, h->nr_cmds, offset); if (unlikely(i == h->nr_cmds)) { @@ -4679,6 +4681,7 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h) h->cmd_pool_bits + (i / BITS_PER_LONG)); break; /* it's ours now. */ } + h->last_allocation = i; /* benignly racy */ /* Zero out all of commandlist except the last field, refcount */ memset(c, 0, offsetof(struct CommandList, refcount)); diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 679e4d2..981479a 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -133,6 +133,7 @@ struct ctlr_info { struct CfgTable __iomem *cfgtable; int interrupts_enabled; int max_commands; + int last_allocation; atomic_t commands_outstanding; # define PERF_MODE_INT0 # define DOORBELL_INT 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
[PATCH v2 26/48] hpsa: factor out hpsa_ciss_submit function
From: Stephen Cameron Factor out the bottom part of the queuecommand function which is the part that builds commands for submitting down the "normal' RAID stack path of a Smart Array. Need to factor this out to improve how commands that were initially sent down one of the "ioaccellerated" paths but which have some sort of error condition are retried down the "normal" path. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 126 +++ 1 file changed, 67 insertions(+), 59 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 94a82e3..cc3128f 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -2034,7 +2034,7 @@ static int hpsa_scsi_do_inquiry(struct ctlr_info *h, unsigned char *scsi3addr, c = cmd_alloc(h); - if (c == NULL) {/* trouble... */ + if (c == NULL) { dev_warn(&h->pdev->dev, "cmd_alloc returned NULL!\n"); return -ENOMEM; } @@ -3807,68 +3807,14 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h, dev->scsi3addr); } -/* Running in struct Scsi_Host->host_lock less mode */ -static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) +/* Submit commands down the "normal" RAID stack path */ +static int hpsa_ciss_submit(struct ctlr_info *h, + struct CommandList *c, struct scsi_cmnd *cmd, + unsigned char scsi3addr[]) { - struct ctlr_info *h; - struct hpsa_scsi_dev_t *dev; - unsigned char scsi3addr[8]; - struct CommandList *c; - int rc = 0; - - /* Get the ptr to our adapter structure out of cmd->host. */ - h = sdev_to_hba(cmd->device); - dev = cmd->device->hostdata; - if (!dev) { - cmd->result = DID_NO_CONNECT << 16; - cmd->scsi_done(cmd); - return 0; - } - memcpy(scsi3addr, dev->scsi3addr, sizeof(scsi3addr)); - - if (unlikely(lockup_detected(h))) { - cmd->result = DID_ERROR << 16; - cmd->scsi_done(cmd); - return 0; - } - c = cmd_alloc(h); - if (c == NULL) {/* trouble... */ - dev_err(&h->pdev->dev, "cmd_alloc returned NULL!\n"); - return SCSI_MLQUEUE_HOST_BUSY; - } - - /* Fill in the command list header */ - /* save c in case we have to abort it */ cmd->host_scribble = (unsigned char *) c; - c->cmd_type = CMD_SCSI; c->scsi_cmd = cmd; - - /* Call alternate submit routine for I/O accelerated commands. -* Retries always go down the normal I/O path. -*/ - if (likely(cmd->retries == 0 && - cmd->request->cmd_type == REQ_TYPE_FS && - h->acciopath_status)) { - if (dev->offload_enabled) { - rc = hpsa_scsi_ioaccel_raid_map(h, c); - if (rc == 0) - return 0; /* Sent on ioaccel path */ - if (rc < 0) { /* scsi_dma_map failed. */ - cmd_free(h, c); - return SCSI_MLQUEUE_HOST_BUSY; - } - } else if (dev->ioaccel_handle) { - rc = hpsa_scsi_ioaccel_direct_map(h, c); - if (rc == 0) - return 0; /* Sent on direct map path */ - if (rc < 0) { /* scsi_dma_map failed. */ - cmd_free(h, c); - return SCSI_MLQUEUE_HOST_BUSY; - } - } - } - c->Header.ReplyQueue = 0; /* unused in simple mode */ memcpy(&c->Header.LUN.LunAddrBytes[0], &scsi3addr[0], 8); c->Header.tag = cpu_to_le64((c->cmdindex << DIRECT_LOOKUP_SHIFT)); @@ -3927,6 +3873,68 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) return 0; } +/* Running in struct Scsi_Host->host_lock less mode */ +static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) +{ + struct ctlr_info *h; + struct hpsa_scsi_dev_t *dev; + unsigned char scsi3addr[8]; + struct CommandList *c; + int rc = 0; + + /* Get the ptr to our adapter structure out of cmd->host. */ + h = sdev_to_hba(cmd->device); + dev = cmd->device->hostdata; + if (!dev) { + cmd->result = DID_NO_CONNECT << 16; + cmd->scsi_done(cmd); + return 0; + } + memcpy(scsi3addr, dev->scsi3addr, sizeof(scsi3addr)); + + if (unlikely(lockup_detected(h))) { + cmd->result = DID_ERROR << 16; + cmd->scsi_done(cmd); + return 0; + } + c = cmd_alloc(h); + if (c == NULL) {/*
[PATCH v2 29/48] hpsa: fix race between abort handler and main i/o path
From: Webb Scales This means changing the allocator to reference count commands. The reference count is now the authoritative indicator of whether a command is allocated or not. The h->cmd_pool_bits bitmap is now only a heuristic hint to speed up the allocation process, it is no longer the authoritative record of allocated commands. Since we changed the command allocator to use reference counting as the authoritative indicator of whether a command is allocated, fail_all_outstanding_cmds needs to use the reference count not h->cmd_pool_bits for this purpose. Fix hpsa_drain_accel_commands to use the reference count as the authoritative indicator of whether a command is allocated instead of the h->cmd_pool_bits bitmap. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 109 +++ drivers/scsi/hpsa.h |2 + drivers/scsi/hpsa_cmd.h |1 3 files changed, 65 insertions(+), 47 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 60f5734..c95a20c 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4552,6 +4552,7 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) char msg[256]; /* For debug messaging. */ int ml = 0; __le32 tagupper, taglower; + int refcount; /* Find the controller of the command to be aborted */ h = sdev_to_hba(sc->device); @@ -4580,9 +4581,13 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) /* Get SCSI command to be aborted */ abort = (struct CommandList *) sc->host_scribble; if (abort == NULL) { - dev_err(&h->pdev->dev, "%s FAILED, Command to abort is NULL.\n", - msg); - return FAILED; + /* This can happen if the command already completed. */ + return SUCCESS; + } + refcount = atomic_inc_return(&abort->refcount); + if (refcount == 1) { /* Command is done already. */ + cmd_free(h, abort); + return SUCCESS; } hpsa_get_tag(h, abort, &taglower, &tagupper); ml += sprintf(msg+ml, "Tag:0x%08x:%08x ", tagupper, taglower); @@ -4604,6 +4609,7 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) dev_warn(&h->pdev->dev, "FAILED abort on device C%d:B%d:T%d:L%d\n", h->scsi_host->host_no, dev->bus, dev->target, dev->lun); + cmd_free(h, abort); return FAILED; } dev_info(&h->pdev->dev, "%s REQUEST SUCCEEDED.\n", msg); @@ -4615,32 +4621,35 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) */ #define ABORT_COMPLETE_WAIT_SECS 30 for (i = 0; i < ABORT_COMPLETE_WAIT_SECS * 10; i++) { - if (test_bit(abort->cmdindex & (BITS_PER_LONG - 1), - h->cmd_pool_bits + - (abort->cmdindex / BITS_PER_LONG))) - msleep(100); - else + refcount = atomic_read(&abort->refcount); + if (refcount < 2) { + cmd_free(h, abort); return SUCCESS; + } else { + msleep(100); + } } dev_warn(&h->pdev->dev, "%s FAILED. Aborted command has not completed after %d seconds.\n", msg, ABORT_COMPLETE_WAIT_SECS); + cmd_free(h, abort); return FAILED; } - /* * For operations that cannot sleep, a command block is allocated at init, * and managed by cmd_alloc() and cmd_free() using a simple bitmap to track * which ones are free or in use. Lock must be held when calling this. * cmd_free() is the complement. */ + static struct CommandList *cmd_alloc(struct ctlr_info *h) { struct CommandList *c; int i; union u64bit temp64; dma_addr_t cmd_dma_handle, err_dma_handle; - int loopcount; + int refcount; + unsigned long offset = 0; /* There is some *extremely* small but non-zero chance that that * multiple threads could get in here, and one thread could @@ -4653,23 +4662,27 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h) * infrequently as to be indistinguishable from never. */ - loopcount = 0; - do { - i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds); - if (i == h->nr_cmds) - i = 0; - loopcount++; - } while (test_and_set_bit(i & (BITS_PER_LONG - 1), - h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0 && - loopcount < 10); - - /* Thread got starved? We do not expect this to ever happen. */ - if (loopcount >= 10) - return NULL; - - c = h->cmd_pool + i; - memset(c, 0, sizeof(*c)); - c->Header.tag = cpu_to_le64((u64) i <<
[PATCH v2 31/48] hpsa: count passthru cmds with atomics, not a spin locked int
Performance enhancement. Remove spin_locks from the driver. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 39 +-- drivers/scsi/hpsa.h |3 +-- 2 files changed, 6 insertions(+), 36 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 72abcf3..bae3759 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5105,35 +5105,6 @@ static void check_ioctl_unit_attention(struct ctlr_info *h, (void) check_for_unit_attention(h, c); } -static int increment_passthru_count(struct ctlr_info *h) -{ - unsigned long flags; - - spin_lock_irqsave(&h->passthru_count_lock, flags); - if (h->passthru_count >= HPSA_MAX_CONCURRENT_PASSTHRUS) { - spin_unlock_irqrestore(&h->passthru_count_lock, flags); - return -1; - } - h->passthru_count++; - spin_unlock_irqrestore(&h->passthru_count_lock, flags); - return 0; -} - -static void decrement_passthru_count(struct ctlr_info *h) -{ - unsigned long flags; - - spin_lock_irqsave(&h->passthru_count_lock, flags); - if (h->passthru_count <= 0) { - spin_unlock_irqrestore(&h->passthru_count_lock, flags); - /* not expecting to get here. */ - dev_warn(&h->pdev->dev, "Bug detected, passthru_count seems to be incorrect.\n"); - return; - } - h->passthru_count--; - spin_unlock_irqrestore(&h->passthru_count_lock, flags); -} - /* * ioctl */ @@ -5156,16 +5127,16 @@ static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg) case CCISS_GETDRIVVER: return hpsa_getdrivver_ioctl(h, argp); case CCISS_PASSTHRU: - if (increment_passthru_count(h)) + if (atomic_dec_if_positive(&h->passthru_cmds_avail) < 0) return -EAGAIN; rc = hpsa_passthru_ioctl(h, argp); - decrement_passthru_count(h); + atomic_inc(&h->passthru_cmds_avail); return rc; case CCISS_BIG_PASSTHRU: - if (increment_passthru_count(h)) + if (atomic_dec_if_positive(&h->passthru_cmds_avail) < 0) return -EAGAIN; rc = hpsa_big_passthru_ioctl(h, argp); - decrement_passthru_count(h); + atomic_inc(&h->passthru_cmds_avail); return rc; default: return -ENOTTY; @@ -6852,7 +6823,7 @@ reinit_after_soft_reset: spin_lock_init(&h->lock); spin_lock_init(&h->offline_device_lock); spin_lock_init(&h->scan_lock); - spin_lock_init(&h->passthru_count_lock); + atomic_set(&h->passthru_cmds_avail, HPSA_MAX_CONCURRENT_PASSTHRUS); h->resubmit_wq = alloc_workqueue("hpsa", WQ_MEM_RECLAIM, 0); if (!h->resubmit_wq) { diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 981479a..1856445 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -183,8 +183,7 @@ struct ctlr_info { /* cap concurrent passthrus at some reasonable maximum */ #define HPSA_MAX_CONCURRENT_PASSTHRUS (10) - spinlock_t passthru_count_lock; /* protects passthru_count */ - int passthru_count; + atomic_t passthru_cmds_avail; /* * Performant mode completion buffers -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 33/48] hpsa: do not check for msi(x) in interrupt_pending
From: Stephen Cameron No need to check whether interrupt pending for MSI(X) and conversely, no need to check whether MSI(X) interrupts are being used when checking if interrupts are pending. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.h |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index aa6cb0b..239ecea 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -479,9 +479,6 @@ static bool SA5_performant_intr_pending(struct ctlr_info *h) if (!register_value) return false; - if (h->msi_vector || h->msix_vector) - return true; - /* Read outbound doorbell to flush */ register_value = readl(h->vaddr + SA5_OUTDB_STATUS); return register_value & SA5_OUTDB_STATUS_PERF_BIT; -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 28/48] hpsa: honor queue depth of physical devices
When using the ioaccel submission methods, requests destined for RAID volumes are sometimes diverted to physical devices. The OS has no or limited knowledge of these physical devices, so it is up to the driver to avoid pushing the device too hard. It is better to honor the physical device queue limit rather than making the device spew zillions of TASK SET FULL responses. This is so that hpsa based devices support /sys/block/sdNN/device/queue_type of simple, which lets the SCSI midlayer automatically adjust the queue_depth based on TASK SET FULL and GOOD status. Adjust the queue depth for a new device after it is created based on the maximum queue depths of the physical devices that constitute the device. This drops the maximum queue depth from .can_queue of 1024 to something like 174 for single-drive RAID-0, 348 for two-drive RAID-1, etc. It also adjusts for the ratio of data to parity drives. Reviewed-by: Scott Teel Signed-off-by: Webb Scales Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 318 +-- drivers/scsi/hpsa.h | 14 ++ drivers/scsi/hpsa_cmd.h | 148 ++ 3 files changed, 412 insertions(+), 68 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index dcacb29..60f5734 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -247,7 +247,7 @@ static void hpsa_drain_accel_commands(struct ctlr_info *h); static void hpsa_flush_cache(struct ctlr_info *h); static int hpsa_scsi_ioaccel_queue_command(struct ctlr_info *h, struct CommandList *c, u32 ioaccel_handle, u8 *cdb, int cdb_len, - u8 *scsi3addr); + u8 *scsi3addr, struct hpsa_scsi_dev_t *phys_disk); static void hpsa_command_resubmit_worker(struct work_struct *work); static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) @@ -965,12 +965,24 @@ static void hpsa_scsi_update_entry(struct ctlr_info *h, int hostno, /* Raid level changed. */ h->dev[entry]->raid_level = new_entry->raid_level; - /* Raid offload parameters changed. */ + /* Raid offload parameters changed. Careful about the ordering. */ + if (new_entry->offload_config && new_entry->offload_enabled) { + /* +* if drive is newly offload_enabled, we want to copy the +* raid map data first. If previously offload_enabled and +* offload_config were set, raid map data had better be +* the same as it was before. if raid map data is changed +* then it had better be the case that +* h->dev[entry]->offload_enabled is currently 0. +*/ + h->dev[entry]->raid_map = new_entry->raid_map; + h->dev[entry]->ioaccel_handle = new_entry->ioaccel_handle; + wmb(); /* ensure raid map updated prior to ->offload_enabled */ + } h->dev[entry]->offload_config = new_entry->offload_config; - h->dev[entry]->offload_enabled = new_entry->offload_enabled; - h->dev[entry]->ioaccel_handle = new_entry->ioaccel_handle; h->dev[entry]->offload_to_mirror = new_entry->offload_to_mirror; - h->dev[entry]->raid_map = new_entry->raid_map; + h->dev[entry]->offload_enabled = new_entry->offload_enabled; + h->dev[entry]->queue_depth = new_entry->queue_depth; dev_info(&h->pdev->dev, "%s device c%db%dt%dl%d updated.\n", scsi_device_type(new_entry->devtype), hostno, new_entry->bus, @@ -1096,6 +1108,8 @@ static inline int device_updated(struct hpsa_scsi_dev_t *dev1, return 1; if (dev1->offload_enabled != dev2->offload_enabled) return 1; + if (dev1->queue_depth != dev2->queue_depth) + return 1; return 0; } @@ -1241,6 +1255,85 @@ static void hpsa_show_volume_status(struct ctlr_info *h, } } +/* + * Figure the list of physical drive pointers for a logical drive with + * raid offload configured. + */ +static void hpsa_figure_phys_disk_ptrs(struct ctlr_info *h, + struct hpsa_scsi_dev_t *dev[], int ndevices, + struct hpsa_scsi_dev_t *logical_drive) +{ + struct raid_map_data *map = &logical_drive->raid_map; + struct raid_map_disk_data *dd = &map->data[0]; + int i, j; + int total_disks_per_row = le16_to_cpu(map->data_disks_per_row) + + le16_to_cpu(map->metadata_disks_per_row); + int nraid_map_entries = le16_to_cpu(map->row_cnt) * + le16_to_cpu(map->layout_map_count) * + total_disks_per_row; + int nphys_disk = le16_to_cpu(map->layout_map_count) * + total_disks_per_row; + int qdepth; + + if (nraid_map_entries > RAID_MAP_MAX_ENTRIES) + nraid_map_entries = RAID_MAP_MAX_ENTRIES; + + qdepth = 0; + for (i = 0; i <
[PATCH v2 32/48] hpsa: slightly optimize SA5_performant_completed
Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.h |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 1856445..aa6cb0b 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -412,19 +412,19 @@ static unsigned long SA5_performant_completed(struct ctlr_info *h, u8 q) unsigned long register_value = FIFO_EMPTY; /* msi auto clears the interrupt pending bit. */ - if (!(h->msi_vector || h->msix_vector)) { + if (unlikely(!(h->msi_vector || h->msix_vector))) { /* flush the controller write of the reply queue by reading * outbound doorbell status register. */ - register_value = readl(h->vaddr + SA5_OUTDB_STATUS); + (void) readl(h->vaddr + SA5_OUTDB_STATUS); writel(SA5_OUTDB_CLEAR_PERF_BIT, h->vaddr + SA5_OUTDB_CLEAR); /* Do a read in order to flush the write to the controller * (as per spec.) */ - register_value = readl(h->vaddr + SA5_OUTDB_STATUS); + (void) readl(h->vaddr + SA5_OUTDB_STATUS); } - if ((rq->head[rq->current_entry] & 1) == rq->wraparound) { + if u32) rq->head[rq->current_entry]) & 1) == rq->wraparound) { register_value = rq->head[rq->current_entry]; rq->current_entry++; atomic_dec(&h->commands_outstanding); -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 27/48] hpsa: use workqueue to resubmit failed ioaccel commands
Instead of kicking the commands all the way back to the mid layer, use a work queue. This enables having a mechanism for the driver to be able to resubmit the commands down the "normal" raid path without turning off the ioaccel feature entirely whenever an error is encountered on the ioaccel path, and prevent excessive rescanning of devices. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 72 ++- drivers/scsi/hpsa.h |1 + drivers/scsi/hpsa_cmd.h |1 + 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index cc3128f..dcacb29 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -248,6 +248,7 @@ static void hpsa_flush_cache(struct ctlr_info *h); static int hpsa_scsi_ioaccel_queue_command(struct ctlr_info *h, struct CommandList *c, u32 ioaccel_handle, u8 *cdb, int cdb_len, u8 *scsi3addr); +static void hpsa_command_resubmit_worker(struct work_struct *work); static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) { @@ -1619,7 +1620,6 @@ static void process_ioaccel2_completion(struct ctlr_info *h, struct hpsa_scsi_dev_t *dev) { struct io_accel2_cmd *c2 = &h->ioaccel2_cmd_pool[c->cmdindex]; - int raid_retry = 0; /* check for good status */ if (likely(c2->error_data.serv_response == 0 && @@ -1636,24 +1636,22 @@ static void process_ioaccel2_completion(struct ctlr_info *h, if (is_logical_dev_addr_mode(dev->scsi3addr) && c2->error_data.serv_response == IOACCEL2_SERV_RESPONSE_FAILURE) { - dev->offload_enabled = 0; - cmd->result = DID_SOFT_ERROR << 16; - cmd_free(h, c); - cmd->scsi_done(cmd); - return; - } - raid_retry = handle_ioaccel_mode2_error(h, c, cmd, c2); - /* If error found, disable Smart Path, -* force a retry on the standard path. -*/ - if (raid_retry) { - dev_warn(&h->pdev->dev, "%s: Retrying on standard path.\n", - "HP SSD Smart Path"); - dev->offload_enabled = 0; /* Disable Smart Path */ - cmd->result = DID_SOFT_ERROR << 16; + if (c2->error_data.status == + IOACCEL2_STATUS_SR_IOACCEL_DISABLED) + dev->offload_enabled = 0; + goto retry_cmd; } + + if (handle_ioaccel_mode2_error(h, c, cmd, c2)) + goto retry_cmd; + cmd_free(h, c); cmd->scsi_done(cmd); + return; + +retry_cmd: + INIT_WORK(&c->work, hpsa_command_resubmit_worker); + queue_work_on(raw_smp_processor_id(), h->resubmit_wq, &c->work); } static void complete_scsi_command(struct CommandList *cp) @@ -1723,9 +1721,9 @@ static void complete_scsi_command(struct CommandList *cp) if (is_logical_dev_addr_mode(dev->scsi3addr)) { if (ei->CommandStatus == CMD_IOACCEL_DISABLED) dev->offload_enabled = 0; - cmd->result = DID_SOFT_ERROR << 16; - cmd_free(h, cp); - cmd->scsi_done(cmd); + INIT_WORK(&cp->work, hpsa_command_resubmit_worker); + queue_work_on(raw_smp_processor_id(), + h->resubmit_wq, &cp->work); return; } } @@ -3873,6 +3871,31 @@ static int hpsa_ciss_submit(struct ctlr_info *h, return 0; } +static void hpsa_command_resubmit_worker(struct work_struct *work) +{ + struct scsi_cmnd *cmd; + struct hpsa_scsi_dev_t *dev; + struct CommandList *c = + container_of(work, struct CommandList, work); + + cmd = c->scsi_cmd; + dev = cmd->device->hostdata; + if (!dev) { + cmd->result = DID_NO_CONNECT << 16; + cmd->scsi_done(cmd); + return; + } + if (hpsa_ciss_submit(c->h, c, cmd, dev->scsi3addr)) { + /* +* If we get here, it means dma mapping failed. Try +* again via scsi mid layer, which will then get +* SCSI_MLQUEUE_HOST_BUSY. +*/ + cmd->result = DID_IMM_RETRY << 16; + cmd->scsi_done(cmd); + } +} + /* Running in struct Scsi_Host->host_lock less mode */ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) { @@ -6396,6 +6419,7 @@ static void fail_all_outstanding_cmds(struct ctlr_info *h) int i; struct CommandList *c = NULL; + flush_workqueue(h->resubmit_wq); /* ensure all cmds are fully built */ for (i = 0; i < h->nr_cmds; i++) { if (!test_bit(i & (BITS_PER_LONG - 1), h->cmd_pool_
[PATCH v2 35/48] hpsa: do not ack controller events on controllers that do not support it
From: Stephen Cameron Acking controller events on controllers that do not support it can cause such controllers to lock up. Reviewed-by: Scott Teel Signed-off-by: Joe Handzik Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 5abd49d..afd192d 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6674,6 +6674,9 @@ static void hpsa_ack_ctlr_events(struct ctlr_info *h) int i; char *event_type; + if (!(h->fw_support & MISC_FW_EVENT_NOTIFY)) + return; + /* Ask the controller to clear the events we're handling. */ if ((h->transMethod & (CFGTBL_Trans_io_accel1 | CFGTBL_Trans_io_accel2)) && -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 34/48] hpsa: remove incorrect BUG_ONs checking for raid offload enable
From: Stephen Cameron In set_encrypt_ioaccel2() and in hpsa_scsi_ioaccel_raid_map there were BUG_ONs that looked like this: BUG_ON(!(dev->offload_config && dev->offload_enabled)); But, In hpsa_ack_ctlr_events() we have this, /* Stop sending new RAID offload reqs via the IO accelerator */ scsi_block_requests(h->scsi_host); for (i = 0; i < h->ndevices; i++) h->dev[i]->offload_enabled = 0; hpsa_drain_accel_commands(h); So, we set offload_enabled = 0 for all drives, then do this drain_accel_commands, so that means accel commands could still be in flight, ie. perhaps having just been submitted into hpsa_scsi_ioaccel_raid_map concurrent with ->offload_enabled having just been set to zero. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |4 1 file changed, 4 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index bae3759..5abd49d 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3456,8 +3456,6 @@ static void set_encrypt_ioaccel2(struct ctlr_info *h, struct raid_map_data *map = &dev->raid_map; u64 first_block; - BUG_ON(!(dev->offload_config && dev->offload_enabled)); - /* Are we doing encryption on this device */ if (!(le16_to_cpu(map->flags) & RAID_MAP_FLAG_ENCRYPT_ON)) return; @@ -3688,8 +3686,6 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h, #endif int offload_to_mirror; - BUG_ON(!(dev->offload_config && dev->offload_enabled)); - /* check for valid opcode, get LBA and block count */ switch (cmd->cmnd[0]) { case WRITE_6: -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 36/48] hpsa: guard against overflowing raid map array
From: Stephen Cameron In the code that translates logical drive LBAs to physical drive LBAs if we overflow the raid map disk data array we will get the wrong answers. We do not expect that to happen, but best to be on the safe side and guard against it anyway. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index afd192d..03fae8a 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3921,6 +3921,9 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h, return IO_ACCEL_INELIGIBLE; } + if (unlikely(map_index >= RAID_MAP_MAX_ENTRIES)) + return IO_ACCEL_INELIGIBLE; + c->phys_disk = dev->phys_disk[map_index]; disk_handle = dd[map_index].ioaccel_handle; -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 37/48] hpsa: check for ctlr lockup after command allocation in main io path
From: Stephen Cameron Command allocation is the thing that takes the longest in the main i/o path, so check for controller lockup immediately after this to prevent submitting commands to locked up controller as much as possible. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 03fae8a..834ac78 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4097,8 +4097,15 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) dev_err(&h->pdev->dev, "cmd_alloc returned NULL!\n"); return SCSI_MLQUEUE_HOST_BUSY; } + if (unlikely(lockup_detected(h))) { + cmd->result = DID_ERROR << 16; + cmd_free(h, c); + cmd->scsi_done(cmd); + return 0; + } - /* Call alternate submit routine for I/O accelerated commands. + /* +* Call alternate submit routine for I/O accelerated commands. * Retries always go down the normal I/O path. */ if (likely(cmd->retries == 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 v2 40/48] hpsa: print CDBs instead of kernel virtual addresses for uncommon errors
From: Stephen Cameron Printing the address of the command pointer is of little value, change to print the CDB. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 8f2569c..18bcba9 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -1882,9 +1882,8 @@ static void complete_scsi_command(struct CommandList *cp) case CMD_DATA_UNDERRUN: /* let mid layer handle it. */ break; case CMD_DATA_OVERRUN: - dev_warn(&h->pdev->dev, "cp %p has" - " completed with data overrun " - "reported\n", cp); + dev_warn(&h->pdev->dev, + "CDB %16phN data overrun\n", cp->Request.CDB); break; case CMD_INVALID: { /* print_bytes(cp, sizeof(*cp), 1, 0); @@ -1900,34 +1899,38 @@ static void complete_scsi_command(struct CommandList *cp) break; case CMD_PROTOCOL_ERR: cmd->result = DID_ERROR << 16; - dev_warn(&h->pdev->dev, "cp %p has " - "protocol error\n", cp); + dev_warn(&h->pdev->dev, "CDB %16phN : protocol error\n", + cp->Request.CDB); break; case CMD_HARDWARE_ERR: cmd->result = DID_ERROR << 16; - dev_warn(&h->pdev->dev, "cp %p had hardware error\n", cp); + dev_warn(&h->pdev->dev, "CDB %16phN : hardware error\n", + cp->Request.CDB); break; case CMD_CONNECTION_LOST: cmd->result = DID_ERROR << 16; - dev_warn(&h->pdev->dev, "cp %p had connection lost\n", cp); + dev_warn(&h->pdev->dev, "CDB %16phN : connection lost\n", + cp->Request.CDB); break; case CMD_ABORTED: cmd->result = DID_ABORT << 16; - dev_warn(&h->pdev->dev, "cp %p was aborted with status 0x%x\n", - cp, ei->ScsiStatus); + dev_warn(&h->pdev->dev, "CDB %16phN was aborted with status 0x%x\n", + cp->Request.CDB, ei->ScsiStatus); break; case CMD_ABORT_FAILED: cmd->result = DID_ERROR << 16; - dev_warn(&h->pdev->dev, "cp %p reports abort failed\n", cp); + dev_warn(&h->pdev->dev, "CDB %16phN : abort failed\n", + cp->Request.CDB); break; case CMD_UNSOLICITED_ABORT: cmd->result = DID_SOFT_ERROR << 16; /* retry the command */ - dev_warn(&h->pdev->dev, "cp %p aborted due to an unsolicited " - "abort\n", cp); + dev_warn(&h->pdev->dev, "CDB %16phN : unsolicited abort\n", + cp->Request.CDB); break; case CMD_TIMEOUT: cmd->result = DID_TIME_OUT << 16; - dev_warn(&h->pdev->dev, "cp %p timedout\n", cp); + dev_warn(&h->pdev->dev, "CDB %16phN timed out\n", + cp->Request.CDB); break; case CMD_UNABORTABLE: cmd->result = DID_ERROR << 16; -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 42/48] hpsa: move SG descriptor set-up out of hpsa_scatter_gather()
From: Webb Scales Move the code which sets up the SG descriptor out of hpsa_scatter_gather() and into a subroutine where it can be reused (in the next patch). The Ext field is now assigned unconditionally: this makes the refactor much simpler, but more importantly it removes a conditional operation from inside the loop. The case for which the conditional formerly tested is now executed (unconditionally) after the loop is exited. Reviewed-by: Scott Teel Signed-off-by: Webb Scales Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index a23277d..7915dc4 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3236,6 +3236,17 @@ out: kfree(id_phys); } +static void hpsa_set_sg_descriptor(struct SGDescriptor *desc, + struct scatterlist *sg) +{ + u64 addr64 = (u64) sg_dma_address(sg); + unsigned int len = sg_dma_len(sg); + + desc->Addr = cpu_to_le64(addr64); + desc->Len = cpu_to_le32(len); + desc->Ext = 0; +} + /* * hpsa_scatter_gather takes a struct scsi_cmnd, (cmd), and does the pci * dma mapping and fills in the scatter gather entries of the @@ -3245,9 +3256,7 @@ static int hpsa_scatter_gather(struct ctlr_info *h, struct CommandList *cp, struct scsi_cmnd *cmd) { - unsigned int len; struct scatterlist *sg; - u64 addr64; int use_sg, i, sg_index, chained; struct SGDescriptor *curr_sg; @@ -3270,13 +3279,11 @@ static int hpsa_scatter_gather(struct ctlr_info *h, curr_sg = h->cmd_sg_list[cp->cmdindex]; sg_index = 0; } - addr64 = (u64) sg_dma_address(sg); - len = sg_dma_len(sg); - curr_sg->Addr = cpu_to_le64(addr64); - curr_sg->Len = cpu_to_le32(len); - curr_sg->Ext = cpu_to_le32(0); + hpsa_set_sg_descriptor(curr_sg, sg); curr_sg++; } + + /* Back the pointer up to the last entry and mark it as "last". */ (--curr_sg)->Ext = cpu_to_le32(HPSA_SG_LAST); if (use_sg + chained > h->maxSG) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 44/48] hpsa: shorten the wait for the CISS doorbell mode change ack
From: Robert Elliott Shorten the wait for the CISS configuration table doorbell mode change acknowledgment from 300-600 s to 20 s, which is the value specified in the CISS specification that should be honored by all controllers. Wait using interruptible msleep() rather than uninterruptible usleep_range(), which triggers rt_sched timeout errors if the wait is long. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index a92653a..89744a1 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -60,8 +60,11 @@ #define DRIVER_NAME "HP HPSA Driver (v " HPSA_DRIVER_VERSION ")" #define HPSA "hpsa" -/* How long to wait (in milliseconds) for board to go into simple mode */ -#define MAX_CONFIG_WAIT 3 +/* How long to wait for CISS doorbell communication */ +#define CLEAR_EVENT_WAIT_INTERVAL 20 /* ms for each msleep() call */ +#define MODE_CHANGE_WAIT_INTERVAL 10 /* ms for each msleep() call */ +#define MAX_CLEAR_EVENT_WAIT 3 /* times 20 ms = 600 s */ +#define MAX_MODE_CHANGE_WAIT 2000 /* times 10 ms = 20 s */ #define MAX_IOCTL_CONFIG_WAIT 1000 /*define how many times we will try a command because of bus resets */ @@ -6194,14 +6197,14 @@ static void hpsa_wait_for_clear_event_notify_ack(struct ctlr_info *h) u32 doorbell_value; unsigned long flags; /* wait until the clear_event_notify bit 6 is cleared by controller. */ - for (i = 0; i < MAX_CONFIG_WAIT; i++) { + for (i = 0; i < MAX_CLEAR_EVENT_WAIT; i++) { spin_lock_irqsave(&h->lock, flags); doorbell_value = readl(h->vaddr + SA5_DOORBELL); spin_unlock_irqrestore(&h->lock, flags); if (!(doorbell_value & DOORBELL_CLEAR_EVENTS)) break; /* delay and try again */ - msleep(20); + msleep(CLEAR_EVENT_WAIT_INTERVAL); } } @@ -6215,14 +6218,14 @@ static void hpsa_wait_for_mode_change_ack(struct ctlr_info *h) * (e.g.: hot replace a failed 144GB drive in a RAID 5 set right * as we enter this code.) */ - for (i = 0; i < MAX_CONFIG_WAIT; i++) { + for (i = 0; i < MAX_MODE_CHANGE_WAIT; i++) { spin_lock_irqsave(&h->lock, flags); doorbell_value = readl(h->vaddr + SA5_DOORBELL); spin_unlock_irqrestore(&h->lock, flags); if (!(doorbell_value & CFGTBL_ChangeReq)) break; /* delay and try again */ - usleep_range(1, 2); + msleep(MODE_CHANGE_WAIT_INTERVAL); } } -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 43/48] hpsa: refactor duplicated scan completion code into a new routine
From: Webb Scales Hoist the conditional out of do_not_scan_if_controller_locked_up() and place it in the caller (this improves the code structure, making it more consistent with other uses and enabling tail-call optimization); rename the function to hpsa_scan_complete(), and use it at the end of hpsa_scan_start() as well. Reviewed-by: Scott Teel Signed-off-by: Webb Scales Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 40 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 7915dc4..a92653a 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4149,25 +4149,14 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) return hpsa_ciss_submit(h, c, cmd, scsi3addr); } -static int do_not_scan_if_controller_locked_up(struct ctlr_info *h) +static void hpsa_scan_complete(struct ctlr_info *h) { unsigned long flags; - /* -* Don't let rescans be initiated on a controller known -* to be locked up. If the controller locks up *during* -* a rescan, that thread is probably hosed, but at least -* we can prevent new rescan threads from piling up on a -* locked up controller. -*/ - if (unlikely(lockup_detected(h))) { - spin_lock_irqsave(&h->scan_lock, flags); - h->scan_finished = 1; - wake_up_all(&h->scan_wait_queue); - spin_unlock_irqrestore(&h->scan_lock, flags); - return 1; - } - return 0; + spin_lock_irqsave(&h->scan_lock, flags); + h->scan_finished = 1; + wake_up_all(&h->scan_wait_queue); + spin_unlock_irqrestore(&h->scan_lock, flags); } static void hpsa_scan_start(struct Scsi_Host *sh) @@ -4175,8 +4164,14 @@ static void hpsa_scan_start(struct Scsi_Host *sh) struct ctlr_info *h = shost_to_hba(sh); unsigned long flags; - if (do_not_scan_if_controller_locked_up(h)) - return; + /* +* Don't let rescans be initiated on a controller known to be locked +* up. If the controller locks up *during* a rescan, that thread is +* probably hosed, but at least we can prevent new rescan threads from +* piling up on a locked up controller. +*/ + if (unlikely(lockup_detected(h))) + return hpsa_scan_complete(h); /* wait until any scan already in progress is finished. */ while (1) { @@ -4194,15 +4189,12 @@ static void hpsa_scan_start(struct Scsi_Host *sh) h->scan_finished = 0; /* mark scan as in progress */ spin_unlock_irqrestore(&h->scan_lock, flags); - if (do_not_scan_if_controller_locked_up(h)) - return; + if (unlikely(lockup_detected(h))) + return hpsa_scan_complete(h); hpsa_update_scsi_devices(h, h->scsi_host->host_no); - spin_lock_irqsave(&h->scan_lock, flags); - h->scan_finished = 1; /* mark scan as finished. */ - wake_up_all(&h->scan_wait_queue); - spin_unlock_irqrestore(&h->scan_lock, flags); + hpsa_scan_complete(h); } static int hpsa_change_queue_depth(struct scsi_device *sdev, int qdepth) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 45/48] hpsa: detect and report failures changing controller transport modes
From: Robert Elliott Detect failues when attempting to change controller to use simple or performant transport modes (mode change ack) rather than just proceeding ahead after timeouts. Return values are added to: hpsa_put_ctlr_into_performant_mode hpsa_wait_for_mode_change_ack and all their callers check/propagate the result. More consistency in printing errors and whether dev_err is used. Reviewed-by: Scott Teel Signed-off-by: Robert Elliott Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 40 +--- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 89744a1..c1f4a95 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -243,7 +243,7 @@ static int hpsa_lookup_board_id(struct pci_dev *pdev, u32 *board_id); static int hpsa_wait_for_board_state(struct pci_dev *pdev, void __iomem *vaddr, int wait_for_ready); static inline void finish_cmd(struct CommandList *c); -static void hpsa_wait_for_mode_change_ack(struct ctlr_info *h); +static int hpsa_wait_for_mode_change_ack(struct ctlr_info *h); #define BOARD_NOT_READY 0 #define BOARD_READY 1 static void hpsa_drain_accel_commands(struct ctlr_info *h); @@ -6191,7 +6191,7 @@ static inline void hpsa_p600_dma_prefetch_quirk(struct ctlr_info *h) writel(dma_prefetch, h->vaddr + I2O_DMA1_CFG); } -static void hpsa_wait_for_clear_event_notify_ack(struct ctlr_info *h) +static int hpsa_wait_for_clear_event_notify_ack(struct ctlr_info *h) { int i; u32 doorbell_value; @@ -6202,13 +6202,16 @@ static void hpsa_wait_for_clear_event_notify_ack(struct ctlr_info *h) doorbell_value = readl(h->vaddr + SA5_DOORBELL); spin_unlock_irqrestore(&h->lock, flags); if (!(doorbell_value & DOORBELL_CLEAR_EVENTS)) - break; + goto done; /* delay and try again */ msleep(CLEAR_EVENT_WAIT_INTERVAL); } + return -ENODEV; +done: + return 0; } -static void hpsa_wait_for_mode_change_ack(struct ctlr_info *h) +static int hpsa_wait_for_mode_change_ack(struct ctlr_info *h) { int i; u32 doorbell_value; @@ -6223,12 +6226,16 @@ static void hpsa_wait_for_mode_change_ack(struct ctlr_info *h) doorbell_value = readl(h->vaddr + SA5_DOORBELL); spin_unlock_irqrestore(&h->lock, flags); if (!(doorbell_value & CFGTBL_ChangeReq)) - break; + goto done; /* delay and try again */ msleep(MODE_CHANGE_WAIT_INTERVAL); } + return -ENODEV; +done: + return 0; } +/* return -ENODEV or other reason on error, 0 on success */ static int hpsa_enter_simple_mode(struct ctlr_info *h) { u32 trans_support; @@ -6243,7 +6250,8 @@ static int hpsa_enter_simple_mode(struct ctlr_info *h) writel(CFGTBL_Trans_Simple, &(h->cfgtable->HostWrite.TransportRequest)); writel(0, &h->cfgtable->HostWrite.command_pool_addr_hi); writel(CFGTBL_ChangeReq, h->vaddr + SA5_DOORBELL); - hpsa_wait_for_mode_change_ack(h); + if (hpsa_wait_for_mode_change_ack(h)) + goto error; print_cfg_table(&h->pdev->dev, h->cfgtable); if (!(readl(&(h->cfgtable->TransportActive)) & CFGTBL_Trans_Simple)) goto error; @@ -7144,7 +7152,8 @@ static void calc_bucket_map(int bucket[], int num_buckets, } } -static void hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support) +/* return -ENODEV or other reason on error, 0 on success */ +static int hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support) { int i; unsigned long register_value; @@ -7236,12 +7245,16 @@ static void hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support) } } writel(CFGTBL_ChangeReq, h->vaddr + SA5_DOORBELL); - hpsa_wait_for_mode_change_ack(h); + if (hpsa_wait_for_mode_change_ack(h)) { + dev_err(&h->pdev->dev, + "performant mode problem - doorbell timeout\n"); + return -ENODEV; + } register_value = readl(&(h->cfgtable->TransportActive)); if (!(register_value & CFGTBL_Trans_Performant)) { dev_err(&h->pdev->dev, "performant mode problem - transport not active\n"); - return; + return -ENODEV; } /* Change the access methods to the performant access methods */ h->access = access; @@ -7249,7 +7262,7 @@ static void hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support) if (!((trans_support & CFGTBL_Trans_io_accel1) || (trans_support & CFGTBL_Trans_io_accel2))) - return; + return 0; if (trans_
[PATCH v2 41/48] hpsa: do not use function pointers in fast path command submission
From: Stephen Cameron Performance tweak, avoid unnecessary function calls. Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |8 +--- drivers/scsi/hpsa.h |5 + 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 18bcba9..a23277d 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -821,19 +821,21 @@ static void dial_up_lockup_detection_on_fw_flash_complete(struct ctlr_info *h, static void enqueue_cmd_and_start_io(struct ctlr_info *h, struct CommandList *c) { + dial_down_lockup_detection_during_fw_flash(h, c); + atomic_inc(&h->commands_outstanding); switch (c->cmd_type) { case CMD_IOACCEL1: set_ioaccel1_performant_mode(h, c); + writel(c->busaddr, h->vaddr + SA5_REQUEST_PORT_OFFSET); break; case CMD_IOACCEL2: set_ioaccel2_performant_mode(h, c); + writel(c->busaddr, h->vaddr + IOACCEL2_INBOUND_POSTQ_32); break; default: set_performant_mode(h, c); + h->access.submit_command(h, c); } - dial_down_lockup_detection_during_fw_flash(h, c); - atomic_inc(&h->commands_outstanding); - h->access.submit_command(h, c); } static inline int is_hba_lunid(unsigned char scsi3addr[]) diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 239ecea..62c50c3 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -367,10 +367,7 @@ static void SA5_submit_command_no_read(struct ctlr_info *h, static void SA5_submit_command_ioaccel2(struct ctlr_info *h, struct CommandList *c) { - if (c->cmd_type == CMD_IOACCEL2) - writel(c->busaddr, h->vaddr + IOACCEL2_INBOUND_POSTQ_32); - else - writel(c->busaddr, h->vaddr + SA5_REQUEST_PORT_OFFSET); + writel(c->busaddr, h->vaddr + SA5_REQUEST_PORT_OFFSET); } /* -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 47/48] hpsa: add in P840ar controller model name
Add in P840ar model name for gen9 Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 6568da5..15ef65c 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -172,7 +172,7 @@ static struct board_type products[] = { {0x21BE103C, "Smart Array P741m", &SA5_access}, {0x21BF103C, "Smart HBA H240ar", &SA5_access}, {0x21C0103C, "Smart Array P440ar", &SA5_access}, - {0x21C1103C, "Smart Array", &SA5_access}, + {0x21C1103C, "Smart Array P840ar", &SA5_access}, {0x21C2103C, "Smart Array P440", &SA5_access}, {0x21C3103C, "Smart Array P441", &SA5_access}, {0x21C4103C, "Smart Array", &SA5_access}, -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 46/48] hpsa: add in gen9 controller model names
Add in gen9 controller model names Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index c1f4a95..6568da5 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -168,24 +168,24 @@ static struct board_type products[] = { {0x1926103C, "Smart Array P731m", &SA5_access}, {0x1928103C, "Smart Array P230i", &SA5_access}, {0x1929103C, "Smart Array P530", &SA5_access}, - {0x21BD103C, "Smart Array", &SA5_access}, - {0x21BE103C, "Smart Array", &SA5_access}, - {0x21BF103C, "Smart Array", &SA5_access}, - {0x21C0103C, "Smart Array", &SA5_access}, + {0x21BD103C, "Smart Array P244br", &SA5_access}, + {0x21BE103C, "Smart Array P741m", &SA5_access}, + {0x21BF103C, "Smart HBA H240ar", &SA5_access}, + {0x21C0103C, "Smart Array P440ar", &SA5_access}, {0x21C1103C, "Smart Array", &SA5_access}, - {0x21C2103C, "Smart Array", &SA5_access}, - {0x21C3103C, "Smart Array", &SA5_access}, + {0x21C2103C, "Smart Array P440", &SA5_access}, + {0x21C3103C, "Smart Array P441", &SA5_access}, {0x21C4103C, "Smart Array", &SA5_access}, - {0x21C5103C, "Smart Array", &SA5_access}, - {0x21C6103C, "Smart Array", &SA5_access}, - {0x21C7103C, "Smart Array", &SA5_access}, - {0x21C8103C, "Smart Array", &SA5_access}, + {0x21C5103C, "Smart Array P841", &SA5_access}, + {0x21C6103C, "Smart HBA H244br", &SA5_access}, + {0x21C7103C, "Smart HBA H240", &SA5_access}, + {0x21C8103C, "Smart HBA H241", &SA5_access}, {0x21C9103C, "Smart Array", &SA5_access}, - {0x21CA103C, "Smart Array", &SA5_access}, - {0x21CB103C, "Smart Array", &SA5_access}, + {0x21CA103C, "Smart Array P246br", &SA5_access}, + {0x21CB103C, "Smart Array P840", &SA5_access}, {0x21CC103C, "Smart Array", &SA5_access}, {0x21CD103C, "Smart Array", &SA5_access}, - {0x21CE103C, "Smart Array", &SA5_access}, + {0x21CE103C, "Smart HBA", &SA5_access}, {0x00761590, "HP Storage P1224 Array Controller", &SA5_access}, {0x00871590, "HP Storage P1224e Array Controller", &SA5_access}, {0x007D1590, "HP Storage P1228 Array Controller", &SA5_access}, -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 39/48] hpsa: do not use a void pointer for scsi_cmd field of struct CommandList
From: Stephen Cameron There's no reason for it to be a void *, it should be a struct scsi_cmnd * Reviewed-by: Scott Teel Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |6 +++--- drivers/scsi/hpsa_cmd.h |2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index a02ea7f..8f2569c 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -1764,7 +1764,7 @@ static void complete_scsi_command(struct CommandList *cp) unsigned long sense_data_size; ei = cp->err_info; - cmd = (struct scsi_cmnd *) cp->scsi_cmd; + cmd = cp->scsi_cmd; h = cp->h; dev = cmd->device->hostdata; @@ -4466,7 +4466,7 @@ static int hpsa_send_reset_as_abort_ioaccel2(struct ctlr_info *h, unsigned char *psa = &phys_scsi3addr[0]; /* Get a pointer to the hpsa logical device. */ - scmd = (struct scsi_cmnd *) abort->scsi_cmd; + scmd = abort->scsi_cmd; dev = (struct hpsa_scsi_dev_t *)(scmd->device->hostdata); if (dev == NULL) { dev_warn(&h->pdev->dev, @@ -4604,7 +4604,7 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) } hpsa_get_tag(h, abort, &taglower, &tagupper); ml += sprintf(msg+ml, "Tag:0x%08x:%08x ", tagupper, taglower); - as = (struct scsi_cmnd *) abort->scsi_cmd; + as = abort->scsi_cmd; if (as != NULL) ml += sprintf(msg+ml, "Command:0x%x SN:0x%lx ", as->cmnd[0], as->serial_number); diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h index 071b64c..3a621c7 100644 --- a/drivers/scsi/hpsa_cmd.h +++ b/drivers/scsi/hpsa_cmd.h @@ -408,7 +408,7 @@ struct CommandList { intcmd_type; long cmdindex; struct completion *waiting; - void *scsi_cmd; + struct scsi_cmnd *scsi_cmd; struct work_struct work; /* -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 38/48] hpsa: return failed from device reset/abort handlers
Returning failed from the device reset handler will get the device kicked offline, which is fine if the controller is locked up anyhow. Cannot abort a command from a failed controller. Reviewed-by: Scott Teel Reviewed-by: Justin Lindley Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 834ac78..a02ea7f 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4343,6 +4343,10 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd) h = sdev_to_hba(scsicmd->device); if (h == NULL) /* paranoia */ return FAILED; + + if (lockup_detected(h)) + return FAILED; + dev = scsicmd->device->hostdata; if (!dev) { dev_err(&h->pdev->dev, "hpsa_eh_device_reset_handler: " @@ -4566,6 +4570,9 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) "ABORT REQUEST FAILED, Controller lookup failed.\n")) return FAILED; + if (lockup_detected(h)) + return FAILED; + /* Check that controller supports some kind of task abort */ if (!(HPSATMF_PHYS_TASK_ABORT & h->TMFSupportFlags) && !(HPSATMF_LOG_TASK_ABORT & h->TMFSupportFlags)) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 48/48] hpsa: Use local workqueues instead of system workqueues
Suggested-by: Tomas Henzl Reviewed-by: Webb Scales Reviewed-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 68 +-- drivers/scsi/hpsa.h |2 ++ 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 15ef65c..95d581c 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6787,14 +6787,14 @@ static int hpsa_offline_devices_ready(struct ctlr_info *h) return 0; } - -static void hpsa_monitor_ctlr_worker(struct work_struct *work) +static void hpsa_rescan_ctlr_worker(struct work_struct *work) { unsigned long flags; struct ctlr_info *h = container_of(to_delayed_work(work), - struct ctlr_info, monitor_ctlr_work); - detect_controller_lockup(h); - if (lockup_detected(h)) + struct ctlr_info, rescan_ctlr_work); + + + if (h->remove_in_progress) return; if (hpsa_ctlr_needs_rescan(h) || hpsa_offline_devices_ready(h)) { @@ -6803,17 +6803,44 @@ static void hpsa_monitor_ctlr_worker(struct work_struct *work) hpsa_scan_start(h->scsi_host); scsi_host_put(h->scsi_host); } - spin_lock_irqsave(&h->lock, flags); - if (h->remove_in_progress) { - spin_unlock_irqrestore(&h->lock, flags); + if (!h->remove_in_progress) + queue_delayed_work(h->rescan_ctlr_wq, &h->rescan_ctlr_work, + h->heartbeat_sample_interval); + spin_unlock_irqrestore(&h->lock, flags); +} + +static void hpsa_monitor_ctlr_worker(struct work_struct *work) +{ + unsigned long flags; + struct ctlr_info *h = container_of(to_delayed_work(work), + struct ctlr_info, monitor_ctlr_work); + + detect_controller_lockup(h); + if (lockup_detected(h)) return; - } - schedule_delayed_work(&h->monitor_ctlr_work, + + spin_lock_irqsave(&h->lock, flags); + if (!h->remove_in_progress) + schedule_delayed_work(&h->monitor_ctlr_work, h->heartbeat_sample_interval); spin_unlock_irqrestore(&h->lock, flags); } +static struct workqueue_struct *hpsa_create_controller_wq(struct ctlr_info *h, + char *name) +{ + struct workqueue_struct *wq = NULL; + char wq_name[20]; + + snprintf(wq_name, sizeof(wq_name), "%s_%d_hpsa", name, h->ctlr); + wq = alloc_ordered_workqueue(wq_name, 0); + if (!wq) + dev_err(&h->pdev->dev, "failed to create %s workqueue\n", name); + + return wq; +} + static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) { int dac, rc; @@ -6856,12 +6883,18 @@ reinit_after_soft_reset: spin_lock_init(&h->scan_lock); atomic_set(&h->passthru_cmds_avail, HPSA_MAX_CONCURRENT_PASSTHRUS); - h->resubmit_wq = alloc_workqueue("hpsa", WQ_MEM_RECLAIM, 0); + h->rescan_ctlr_wq = hpsa_create_controller_wq(h, "rescan"); + if (!h->rescan_ctlr_wq) { + rc = -ENOMEM; + goto clean1; + } + + h->resubmit_wq = hpsa_create_controller_wq(h, "resubmit"); if (!h->resubmit_wq) { - dev_err(&h->pdev->dev, "Failed to allocate work queue\n"); rc = -ENOMEM; goto clean1; } + /* Allocate and clear per-cpu variable lockup_detected */ h->lockup_detected = alloc_percpu(u32); if (!h->lockup_detected) { @@ -6985,6 +7018,9 @@ reinit_after_soft_reset: INIT_DELAYED_WORK(&h->monitor_ctlr_work, hpsa_monitor_ctlr_worker); schedule_delayed_work(&h->monitor_ctlr_work, h->heartbeat_sample_interval); + INIT_DELAYED_WORK(&h->rescan_ctlr_work, hpsa_rescan_ctlr_worker); + queue_delayed_work(h->rescan_ctlr_wq, &h->rescan_ctlr_work, + h->heartbeat_sample_interval); return 0; clean4: @@ -6996,6 +7032,8 @@ clean2: clean1: if (h->resubmit_wq) destroy_workqueue(h->resubmit_wq); + if (h->rescan_ctlr_wq) + destroy_workqueue(h->rescan_ctlr_wq); if (h->lockup_detected) free_percpu(h->lockup_detected); kfree(h); @@ -7069,11 +7107,13 @@ static void hpsa_remove_one(struct pci_dev *pdev) /* Get rid of any controller monitoring work items */ spin_lock_irqsave(&h->lock, flags); h->remove_in_progress = 1; - cancel_delayed_work(&h->monitor_ctlr_work); spin_unlock_irqrestore(&h->lock, flags); + cancel_delayed_work_sync(&h->monitor_ctlr_work); + cancel_delayed_work_sync(&h->rescan_ctlr_work); + destroy_workqueue(h->rescan_ctlr_wq); + destroy_workqueue(h->resubmit_wq); hpsa
Re: module: fix module_refcount() return when running in a module exit routine
James Bottomley writes: > On Fri, 2015-01-23 at 05:17 -0800, Christoph Hellwig wrote: >> On Fri, Jan 23, 2015 at 01:24:15PM +1030, Rusty Russell wrote: >> > The correct fix is to turn try_module_get() into __module_get(), and >> > always do the module_put(). >> >> Is this really safe? __module_get sais it needs a non-zero refcount >> to start with, but scsi_device_get is the only thing every incrementing >> the refcount on the module pointer in the scsi host template, so >> exactly that case can happen easily. There's not assert actually >> hardcoding the assumption, so I'm not sure if requirement the comment >> really just is nice to have or a strong requirement. Yes, as James says, my patch makes it no worse. The "someone else grabs a reference" can be fixed in two ways. James' alternate path as per below, or by waiting in the exit function. The latter is kind of annoying, which is why we do the whole atomic deregistration for modules... Cheers, Rusty. > The comment was just documenting the old status quo: the > try_module_get() was expected to fail if called within the host module > remove path. If you look at the flow, we use the refcounts on the > actual scsi device to measure. If they fail we know we have a problem. > The module stuff is really only slaved to our master refcount on the > device. It's purpose is to prevent an inopportune rmmod. Our default > operating state is zero references on everything, so the user can just > do rmmod ... obviously if the device is open or mounted then we hold > both the device and the module. > > To that point, Rusty's patch just keeps the status quo in the new > module_refcount() environment, so it's the quick bandaid. > > I think the use case you're worrying about is what happens if someone > tries to use a device after module removal begins executing but before > the device has been deleted (say by opening it)? We'll exit the device > removal routines and then kill the module, because after the module code > gets to ->exit(), nothing re-checks the module refcount, so the host > module will get free'd while we're still using the device. > > The fix for this seems to be to differentiate between special uses of > scsi_get_device, which are allowed to get the device in the module exit > routines and ordinary uses which aren't. Something like this? (the > patch isn't complete, but you get the idea). > > James > > --- > > diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c > index 08c90a7..31ba254 100644 > --- a/drivers/scsi/scsi.c > +++ b/drivers/scsi/scsi.c > @@ -965,6 +965,15 @@ int scsi_report_opcode(struct scsi_device *sdev, > unsigned char *buffer, > } > EXPORT_SYMBOL(scsi_report_opcode); > > +static int __scsi_device_get_common(struct scsi_device *sdev) > +{ > + if (sdev->sdev_state == SDEV_DEL) > + return -ENXIO; > + if (!get_device(&sdev->sdev_gendev)) > + return -ENXIO; > + return 0; > +} > + > /** > * scsi_device_get - get an additional reference to a scsi_device > * @sdev:device to get a reference to > @@ -975,19 +984,45 @@ EXPORT_SYMBOL(scsi_report_opcode); > */ > int scsi_device_get(struct scsi_device *sdev) > { > - if (sdev->sdev_state == SDEV_DEL) > - return -ENXIO; > - if (!get_device(&sdev->sdev_gendev)) > - return -ENXIO; > - /* We can fail this if we're doing SCSI operations > - * from module exit (like cache flush) */ > - try_module_get(sdev->host->hostt->module); > + int ret; > > - return 0; > + ret = __scsi_device_get_common(sdev); > + if (ret) > + return ret; > + > + ret = try_module_get(sdev->host->hostt->module); > + > + if (ret) > + put_device(&sdev->sdev_gendev); > + > + return ret; > } > EXPORT_SYMBOL(scsi_device_get); > > /** > + * scsi_device_get_in_module_exit() - get an additional reference to a > scsi_device > + * @sdev:device to get a reference to > + * > + * Functions identically to scsi_device_get() except that it unconditionally > + * gets the module reference. This allows it to be called from module exit > + * routines where scsi_device_get() will fail. This routine is still paired > + * with scsi_device_put(). > + */ > +int scsi_device_get_in_module_exit(struct scsi_device *sdev) > +{ > + int ret; > + > + ret = __scsi_device_get_common(sdev); > + if (ret) > + return ret; > + > + __module_get(sdev->host->hostt->module); > + > + return 0; > +} > +EXPORT_SYMBOL(scsi_device_get_in_module_exit); > + > +/** > * scsi_device_put - release a reference to a scsi_device > * @sdev:device to release a reference on. > * > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index ebf35cb6..057604e 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -564,16 +564,22 @@ static int sd_major(int major_idx) > } > } > > -static struct scsi_disk *__scsi_disk_get(struct gendisk *disk) > +static struct s