| Message ID | 0a508ff31bbfa9cd73c24713c54a29ac459e3254.camel@HansenPartnership.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [GIT,PULL] SCSI fixes for 5.16-rc1 | expand |
On Fri, Nov 19, 2021 at 10:20 AM James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > > Six fixes, five in drivers (ufs, qla2xxx, iscsi) and one core change to > fix a regression in user space device state setting, which is used by > the iscsi daemons to effect device recovery. Language nit. One of the few correct uses of "to effect" - but perhaps best avoided just because even native speakers get it wrong. And we have a lot of non-native speakers too. It might have been clearer to just say "to start device recovery" or perhaps just "as part of device recovery". Just to avoid confusion with "affect". Which it obviously _also_ does. I kept your wording, but this is just a note that maybe commit messages should strive to generally use fairly basic English language and try to avoid things that are known to trip people up. Linus
The pull request you sent on Fri, 19 Nov 2021 13:20:10 -0500:
> git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ecd510d2ff86953378c540182f14c8890b1f1225
Thank you!
On Fri, 2021-11-19 at 11:29 -0800, Linus Torvalds wrote: > On Fri, Nov 19, 2021 at 10:20 AM James Bottomley > <James.Bottomley@hansenpartnership.com> wrote: > > Six fixes, five in drivers (ufs, qla2xxx, iscsi) and one core > > change to fix a regression in user space device state setting, > > which is used by the iscsi daemons to effect device recovery. > > Language nit. hey your "nit" is that I used an English verb correctly? > One of the few correct uses of "to effect" - but perhaps best avoided > just because even native speakers get it wrong. And we have a lot of > non-native speakers too. Well, OK, we do the difference between effect and affect in high school, but I'm happy to use a more neutral phrasing because I do notice when people get it wrong (I just silently correct internally). > It might have been clearer to just say "to start device recovery" or > perhaps just "as part of device recovery". Just to avoid confusion > with "affect". Which it obviously _also_ does. > > I kept your wording, but this is just a note that maybe commit > messages should strive to generally use fairly basic English language > and try to avoid things that are known to trip people up. I can certainly relate to the need to be clear and unambiguous, but this is the thin end of the wedge: you'll be telling me I can't use the subjunctive mood next just because Americans don't understand what it is ... James
On Fri, Nov 19, 2021 at 12:07 PM James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > > I can certainly relate to the need to be clear and unambiguous, but > this is the thin end of the wedge: you'll be telling me I can't use the > subjunctive mood next just because Americans don't understand what it > is ... Please do strive to keep it to monosyllabic words, with the occasional grunt for emphasis. Linus
On Fri, 2021-11-19 at 12:41 -0800, Linus Torvalds wrote: > On Fri, Nov 19, 2021 at 12:07 PM James Bottomley > <James.Bottomley@hansenpartnership.com> wrote: > > I can certainly relate to the need to be clear and unambiguous, but > > this is the thin end of the wedge: you'll be telling me I can't use > > the subjunctive mood next just because Americans don't understand > > what it is ... > > Please do strive to keep it to monosyllabic words, with the > occasional grunt for emphasis. Ugh. James
diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c index 73a353153d33..10d2655ef676 100644 --- a/drivers/scsi/qla2xxx/qla_mbx.c +++ b/drivers/scsi/qla2xxx/qla_mbx.c @@ -1695,10 +1695,8 @@ qla2x00_get_adapter_id(scsi_qla_host_t *vha, uint16_t *id, uint8_t *al_pa, mcp->in_mb |= MBX_13|MBX_12|MBX_11|MBX_10; if (IS_FWI2_CAPABLE(vha->hw)) mcp->in_mb |= MBX_19|MBX_18|MBX_17|MBX_16; - if (IS_QLA27XX(vha->hw) || IS_QLA28XX(vha->hw)) { - mcp->in_mb |= MBX_15; - mcp->out_mb |= MBX_7|MBX_21|MBX_22|MBX_23; - } + if (IS_QLA27XX(vha->hw) || IS_QLA28XX(vha->hw)) + mcp->in_mb |= MBX_15|MBX_21|MBX_22|MBX_23; mcp->tov = MBX_TOV_SECONDS; mcp->flags = 0; diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 55addd78fde4..7afcec250f9b 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -792,6 +792,7 @@ store_state_field(struct device *dev, struct device_attribute *attr, int i, ret; struct scsi_device *sdev = to_scsi_device(dev); enum scsi_device_state state = 0; + bool rescan_dev = false; for (i = 0; i < ARRAY_SIZE(sdev_states); i++) { const int len = strlen(sdev_states[i].name); @@ -810,20 +811,27 @@ store_state_field(struct device *dev, struct device_attribute *attr, } mutex_lock(&sdev->state_mutex); - ret = scsi_device_set_state(sdev, state); - /* - * If the device state changes to SDEV_RUNNING, we need to - * run the queue to avoid I/O hang, and rescan the device - * to revalidate it. Running the queue first is necessary - * because another thread may be waiting inside - * blk_mq_freeze_queue_wait() and because that call may be - * waiting for pending I/O to finish. - */ - if (ret == 0 && state == SDEV_RUNNING) { + if (sdev->sdev_state == SDEV_RUNNING && state == SDEV_RUNNING) { + ret = count; + } else { + ret = scsi_device_set_state(sdev, state); + if (ret == 0 && state == SDEV_RUNNING) + rescan_dev = true; + } + mutex_unlock(&sdev->state_mutex); + + if (rescan_dev) { + /* + * If the device state changes to SDEV_RUNNING, we need to + * run the queue to avoid I/O hang, and rescan the device + * to revalidate it. Running the queue first is necessary + * because another thread may be waiting inside + * blk_mq_freeze_queue_wait() and because that call may be + * waiting for pending I/O to finish. + */ blk_mq_run_hw_queues(sdev->request_queue, true); scsi_rescan_device(dev); } - mutex_unlock(&sdev->state_mutex); return ret == 0 ? count : -EINVAL; } diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index 78343d3f9385..554b6f784223 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -1899,12 +1899,12 @@ static void session_recovery_timedout(struct work_struct *work) } spin_unlock_irqrestore(&session->lock, flags); - if (session->transport->session_recovery_timedout) - session->transport->session_recovery_timedout(session); - ISCSI_DBG_TRANS_SESSION(session, "Unblocking SCSI target\n"); scsi_target_unblock(&session->dev, SDEV_TRANSPORT_OFFLINE); ISCSI_DBG_TRANS_SESSION(session, "Completed unblocking SCSI target\n"); + + if (session->transport->session_recovery_timedout) + session->transport->session_recovery_timedout(session); } static void __iscsi_unblock_session(struct work_struct *work) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index afd38142b1c0..13c09dbd99b9 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -6453,9 +6453,8 @@ static irqreturn_t ufshcd_tmc_handler(struct ufs_hba *hba) irqreturn_t ret = IRQ_NONE; int tag; - pending = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL); - spin_lock_irqsave(hba->host->host_lock, flags); + pending = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL); issued = hba->outstanding_tasks & ~pending; for_each_set_bit(tag, &issued, hba->nutmrs) { struct request *req = hba->tmf_rqs[tag]; @@ -6616,11 +6615,6 @@ static int __ufshcd_issue_tm_cmd(struct ufs_hba *hba, err = wait_for_completion_io_timeout(&wait, msecs_to_jiffies(TM_CMD_TIMEOUT)); if (!err) { - /* - * Make sure that ufshcd_compl_tm() does not trigger a - * use-after-free. - */ - req->end_io_data = NULL; ufshcd_add_tm_upiu_trace(hba, task_tag, UFS_TM_ERR); dev_err(hba->dev, "%s: task management cmd 0x%.2x timed-out\n", __func__, tm_function); @@ -7116,6 +7110,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) goto release; } + lrbp->cmd = NULL; err = SUCCESS; release:
Six fixes, five in drivers (ufs, qla2xxx, iscsi) and one core change to fix a regression in user space device state setting, which is used by the iscsi daemons to effect device recovery. The patch is available here: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes The short changelog is: Adrian Hunter (2): scsi: ufs: core: Fix another task management completion race scsi: ufs: core: Fix task management completion timeout race Bart Van Assche (1): scsi: ufs: core: Improve SCSI abort handling Ewan D. Milne (1): scsi: qla2xxx: Fix mailbox direction flags in qla2xxx_get_adapter_id() Mike Christie (2): scsi: core: sysfs: Fix hang when device state is set via sysfs scsi: iscsi: Unblock session then wake up error handler And the diffstat: drivers/scsi/qla2xxx/qla_mbx.c | 6 ++---- drivers/scsi/scsi_sysfs.c | 30 +++++++++++++++++++----------- drivers/scsi/scsi_transport_iscsi.c | 6 +++--- drivers/scsi/ufs/ufshcd.c | 9 ++------- 4 files changed, 26 insertions(+), 25 deletions(-) With full diff below. James ---