SCSI fixes for 4.13-rc6
James Bottomley
maintainerSix minor and error leg fixes, plus one major change: the reversion of scsi-mq as the default. We're doing the latter temporarily (with a backport to stable) to give us time to fix all the issues that turned up with this default before trying again.
The patch is available here:
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes
The short changelog is:
Christoph Hellwig (1):
Revert "scsi: default to scsi-mq"
Damien Le Moal (1):
scsi: sd_zbc: Write unlock zone from sd_uninit_cmnd()
Raghava Aditya Renukunta (1):
scsi: aacraid: Fix out of bounds in aac_get_name_resp
Varun Prakash (2):
scsi: cxgb4i: call neigh_event_send() to update MAC address
scsi: csiostor: fail probe if fw does not support FCoE
weiping zhang (1):
scsi: megaraid_sas: fix error handle in megasas_probe_one
And the diffstat:
drivers/scsi/Kconfig | 11 +++++++++++ drivers/scsi/aacraid/aachba.c | 9 +++++++-- drivers/scsi/aacraid/aacraid.h | 2 +- drivers/scsi/csiostor/csio_hw.c | 4 +++- drivers/scsi/csiostor/csio_init.c | 12 ++++++++---- drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 3 +++ drivers/scsi/megaraid/megaraid_sas_base.c | 2 +- drivers/scsi/scsi.c | 4 ++++ drivers/scsi/sd.c | 3 +++ drivers/scsi/sd_zbc.c | 9 +++++---- include/scsi/scsi_cmnd.h | 1 + 11 files changed, 47 insertions(+), 13 deletions(-)
With full diff below.
James
---
drivers/scsi/Kconfig +11 -0
@@ -47,6 +47,17 @@ config SCSI_NETLINK default n depends on NET +config SCSI_MQ_DEFAULT + bool "SCSI: use blk-mq I/O path by default" + depends on SCSI + ---help--- + This option enables the new blk-mq based I/O path for SCSI + devices by default. With the option the scsi_mod.use_blk_mq + module/boot option defaults to Y, without it to N, but it can + still be overridden either way. + + If unsure say N. + config SCSI_PROC_FS bool "legacy /proc/scsi/ support" depends on SCSI && PROC_FS
drivers/scsi/aacraid/aachba.c +7 -2
@@ -549,7 +549,9 @@ static void get_container_name_callback(void *context, struct fib * fibptr)if ((le32_to_cpu (get_name_reply->status) ==CT_OK ) && (get_name_reply->data[0 ] !='\0' )) {char *sp = get_name_reply->data; - sp[sizeof (((struct aac_get_name_resp *)NULL )->data)] ='\0' ; + int data_size = FIELD_SIZEOF(struct aac_get_name_resp, data); + + sp[data_size -1 ] ='\0' ;while (*sp ==' ' ) ++sp;if (*sp) {
@@ -579,12 +581,15 @@ static void get_container_name_callback(void *context, struct fib * fibptr)static int aac_get_container_name (struct scsi_cmnd * scsicmd) {int status; +int data_size;struct aac_get_name *dinfo;struct fib * cmd_fibcontext;struct aac_dev * dev; dev = (struct aac_dev *)scsicmd->device->host->hostdata; + data_size = FIELD_SIZEOF(struct aac_get_name_resp, data); + cmd_fibcontext =aac_fib_alloc_tag (dev, scsicmd);aac_fib_init (cmd_fibcontext);
@@ -593,7 +598,7 @@ static int aac_get_container_name(struct scsi_cmnd * scsicmd) dinfo->command =cpu_to_le32 (VM_ContainerConfig); dinfo->type =cpu_to_le32 (CT_READ_NAME ); dinfo->cid =cpu_to_le32 (scmd_id (scsicmd)); - dinfo->count =cpu_to_le32 (sizeof (((struct aac_get_name_resp *)NULL )->data)); + dinfo->count =cpu_to_le32 (data_size -1 ); status = aac_fib_send(ContainerCommand, cmd_fibcontext,
drivers/scsi/aacraid/aacraid.h +1 -1
@@ -2274,7 +2274,7 @@ struct aac_get_name_resp {__le32 parm3;__le32 parm4;__le32 parm5; -u8 data[16 ]; +u8 data[17 ]; };#define CT_CID_TO_32BITS_UID 165
drivers/scsi/csiostor/csio_hw.c +3 -1
@@ -3845,8 +3845,10 @@ csio_hw_start(struct csio_hw *hw)if (csio_is_hw_ready (hw))return 0 ; -else +else if (csio_match_state (hw, csio_hws_uninit))return -EINVAL ; +else +return -ENODEV ; }int
drivers/scsi/csiostor/csio_init.c +8 -4
@@ -969,10 +969,14 @@ static int csio_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)pci_set_drvdata (pdev, hw); -if (csio_hw_start (hw) !=0 ) { -dev_err (&pdev->dev, -"Failed to start FW, continuing in debug mode.\n" ); -return 0 ; + rv =csio_hw_start (hw); +if (rv) { +if (rv == -EINVAL ) { +dev_err (&pdev->dev, +"Failed to start FW, continuing in debug mode.\n" ); +return 0 ; + } + goto err_lnode_exit; } sprintf(hw->fwrev_str, "%u.%u.%u.%u\n",
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c +3 -0
@@ -1635,6 +1635,9 @@ static int init_act_open(struct cxgbi_sock *csk) goto rel_resource; } +if (!(n->nud_state &NUD_VALID )) +neigh_event_send (n,NULL ); + csk->atid =cxgb4_alloc_atid (lldi->tids, csk);if (csk->atid <0 ) {pr_err ("%s, NO atid available.\n" , ndev->name);
drivers/scsi/megaraid/megaraid_sas_base.c +1 -1
@@ -6228,8 +6228,8 @@ static int megasas_probe_one(struct pci_dev *pdev, fail_start_aen: fail_io_attach: megasas_mgmt_info.count--; - megasas_mgmt_info.instance[megasas_mgmt_info.max_index] =NULL ; megasas_mgmt_info.max_index--; + megasas_mgmt_info.instance[megasas_mgmt_info.max_index] =NULL ; instance->instancet->disable_intr (instance);megasas_destroy_irqs (instance);
drivers/scsi/scsi.c +4 -0
@@ -800,7 +800,11 @@ MODULE_LICENSE("GPL");module_param (scsi_logging_level, int,S_IRUGO |S_IWUSR );MODULE_PARM_DESC (scsi_logging_level,"a bit mask of logging levels" ); +#ifdef CONFIG_SCSI_MQ_DEFAULT bool scsi_use_blk_mq = true; +#else +bool scsi_use_blk_mq = false; +#endif module_param_named (use_blk_mq, scsi_use_blk_mq, bool,S_IWUSR |S_IRUGO );static int __init init_scsi (void )
drivers/scsi/sd.c +3 -0
@@ -1277,6 +1277,9 @@ static void sd_uninit_command(struct scsi_cmnd *SCpnt) {struct request *rq = SCpnt->request; +if (SCpnt->flags &SCMD_ZONE_WRITE_LOCK ) +sd_zbc_write_unlock_zone (SCpnt); +if (rq->rq_flags &RQF_SPECIAL_PAYLOAD )__free_page (rq->special_vec.bv_page);
drivers/scsi/sd_zbc.c +5 -4
@@ -294,6 +294,9 @@ int sd_zbc_write_lock_zone(struct scsi_cmnd *cmd) test_and_set_bit(zno, sdkp->zones_wlock))return BLKPREP_DEFER ; +WARN_ON_ONCE (cmd->flags &SCMD_ZONE_WRITE_LOCK ); + cmd->flags |=SCMD_ZONE_WRITE_LOCK ; +return BLKPREP_OK ; }
@@ -302,9 +305,10 @@ void sd_zbc_write_unlock_zone(struct scsi_cmnd *cmd)struct request *rq = cmd->request;struct scsi_disk *sdkp =scsi_disk (rq->rq_disk); -if (sdkp->zones_wlock) { +if (sdkp->zones_wlock && cmd->flags &SCMD_ZONE_WRITE_LOCK ) {unsigned int zno =sd_zbc_zone_no (sdkp,blk_rq_pos (rq));WARN_ON_ONCE (!test_bit (zno, sdkp->zones_wlock)); + cmd->flags &= ~SCMD_ZONE_WRITE_LOCK ;clear_bit_unlock (zno, sdkp->zones_wlock);smp_mb__after_atomic (); }
@@ -335,9 +339,6 @@ void sd_zbc_complete(struct scsi_cmnd *cmd,case REQ_OP_WRITE_ZEROES :case REQ_OP_WRITE_SAME : -/* Unlock the zone */ -sd_zbc_write_unlock_zone (cmd); -if (result && sshdr->sense_key ==ILLEGAL_REQUEST && sshdr->asc ==0x21 )
include/scsi/scsi_cmnd.h +1 -0
@@ -57,6 +57,7 @@ struct scsi_pointer {/* for scmd->flags */ #define SCMD_TAGGED (1 << 0)#define SCMD_UNCHECKED_ISA_DMA (1 << 1) +#define SCMD_ZONE_WRITE_LOCK (1 << 2)struct scsi_cmnd {struct scsi_request req;
Quoted 8 lines · James Bottomley
Six minor and error leg fixes, plus one major change: the reversion of scsi-mq as the default. We're doing the latter temporarily (with a backport to stable) to give us time to fix all the issues that turned up with this default before trying again.
[ ... ]
Damien Le Moal (1):
scsi:sd_zbc: Write unlock zone fromsd_uninit_cmnd()
[ ... ]
Hello James,
Had you noticed that Damien had asked not to send the "sd_zbc: Write unlock zone from sd_uninit_cmnd()" patch to Linus without my "scsi-mq: Always unprepare before requeuing a request" patch? A quote from an e-mail from Damien (https://www.spinics.net/lists/stable/msg185436.html):
---------------------------------------------------------------------------
Re: [PATCH] Revert "scsi-mq: Always unprepare before requeuing a request"
Quoted 4 lines
Quoted 2 lines
For an unknown reason this patch causes the boot process to hang on PowerPC systems:
OK, dropped it from fixes for now.
It means that commit 70e42fd02c46e2aa9ab07b766d418637e3a51de7 "scsi:sd_zbc: Write unlock zone from sd_uninit_cmnd()" will need to be reverted too as it will not solve the potential deadlock anymore. Bart's patch was needed for it to work.
---------------------------------------------------------------------------
Bart.
Hi Bart,
Quoted 3 lines
Had you noticed that Damien had asked not to send the "
sd_zbc: Write unlock zone fromsd_uninit_cmnd()" patch to Linus without my "scsi-mq:
Always unprepare before requeuing a request" patch?
He did change his mind later in that thread, though.
However, what's more important is that we still need a good version of your patch for 4.13. I took Brian's workaround for ipr but I still think Christoph's concerns need to be addressed for me to put your change back in.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Quoted 4 lines · Martin K. Petersen
However, what's more important is that we still need a good version of your patch for 4.13. I took Brian's workaround for ipr but I still think Christoph's concerns need to be addressed for me to put your change back in.
Hello Martin,
I am not aware of any requests to modify the patch "scsi-mq: Always unprepare before requeuing a request". See also
https://www.spinics.net/lists/linux-scsi/msg111541.html. Are you perhaps referring to another patch?
What I remember is that my patch uncovered a bug in the ipr driver. As you mentioned, a workaround for that bug has already been queued for kernel v4.14
(https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=4.13/scsi-fixes&id=723cd772fde2344a9810eeaf5106787d535ec4a4).
Further improvements for the SCSI core that are the result of the analysis of the behavior of the SCSI subsystem on PowerPC systems are under discussion. See also "[PATCHv2 1/2] scsi: Move scsi_cmd->jiffies_at_alloc initialization to allocation time" (https://marc.info/?l=linux-next&m=150335524812989) and "[PATCH 2/2] scsi: Preserve retry counter through scsi_prep_fn"
(https://marc.info/?l=linux-scsi&m=150335371112485).
Please correct me if I got anything wrong.
Bart.
Quoted 8 lines · Bart Van Assche
Quoted 4 lines · Martin K. Petersen
However, what's more important is that we still need a good version of your patch for 4.13. I took Brian's workaround for ipr but I still think Christoph's concerns need to be addressed for me to put your change back in.
Hello Martin,
Quoted 8 lines
I am not aware of any requests to modify the patch "scsi-mq: Always unprepare before requeuing a request". See also
https://www.spinics.net/lists/linux-scsi/msg111541.html. Are you perhaps referring to another patch?What I remember is that my patch uncovered a bug in the ipr driver. As you mentioned, a workaround for that bug has already been queued for kernel v4.14
(https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=4.13/scsi-fixes&id=723cd772fde2344a9810eeaf5106787d535ec4a4).
I don't completely agree with that statement. The patch "scsi-mq: Always unprepare before requeueing a request" introduces a regression in the scsi stack. It alters the behavior of retries such that the retry counter no longer works and the jiffies_at_alloc use to ensure we don't spend a tremendous amount of time retrying ops gets broken as well.
As far as ipr is concerned, we do have the workaround in place now and I'll also queue up a further improvement to ipr to return a better failure response. However, until we fix the retry counter in scsi, any driver that returns an error response that scsi wants to retry could get us stuck in an eternal retry loop, like we were seeing with ipr.
Quoted 6 lines
Further improvements for the SCSI core that are the result of the analysis of the behavior of the SCSI subsystem on PowerPC systems are under discussion. See also "[PATCHv2 1/2] scsi: Move
scsi_cmd->jiffies_at_allocinitialization to allocation time" (https://marc.info/?l=linux-next&m=150335524812989) and "[PATCH 2/2] scsi: Preserve retry counter throughscsi_prep_fn"
(https://marc.info/?l=linux-scsi&m=150335371112485).
While my patches highlight the problem, I don't think they are the right fix and need to be reworked. It looks like we go through scsi_init_rq at hctx setup time rather than for each new i/o submission. Adding a simple kprobe to scsi_init_rq never triggers when issuing i/o to already configured devices. Therefore, we cannot simply move the initialization of jiffies_at_alloc to scsi_init_rq. I've been working on moving the retry counter and jiffies_at_alloc into struct scsi_request, as that doesn't get reinitialized.
Thanks,
Brian
Signature · 3 lines
Brian King Power Linux I/O IBM Linux Technology Center
Quoted 7 lines · Martin K. Petersen
Hi Bart,
Quoted 3 lines
Had you noticed that Damien had asked not to send the "
sd_zbc: Write unlock zone fromsd_uninit_cmnd()" patch to Linus without my "scsi-mq:
Always unprepare before requeuing a request" patch?
Quoted 6 lines
He did change his mind later in that thread, though.
However, what's more important is that we still need a good version of your patch for 4.13. I took Brian's workaround for ipr but I still think Christoph's concerns need to be addressed for me to put your change back in.
Yes, I did ask for the patch to be kept since it does not make things worse for the SMR/mq case (still have a potential dispatch deadlock) but does cleanup the code in the legacy/sq case.
Since it looks like the jiffies_at_alloc and retry counter initialization fixes are progressing, we hopefully should be able to get Bart's pacth back in soon (this week ?) and remove the mq dispatch deadlock.
Best regards.
Signature · 2 lines
Damien Le Moal, Western Digital Research