| Message ID | 20230727194457.3152309-10-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Multiple cleanup patches for the UFS driver | expand |
On Thu, 2023-07-27 at 12:41 -0700, Bart Van Assche wrote: > Unify the MCQ and legacy code paths. This patch reworks code > introduced by > commit ab248643d3d6 ("scsi: ufs: core: Add error handling for MCQ > mode"). > > Cc: Bao D. Nguyen <quic_nguyenb@quicinc.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/ufs/core/ufshcd.c | 46 +++++++++++++++++------------------ > ---- > 1 file changed, 20 insertions(+), 26 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index c0031cf8855c..bf76ea59ba6c 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -6387,6 +6387,22 @@ static bool > ufshcd_is_pwr_mode_restore_needed(struct ufs_hba *hba) > return false; > } > > +static bool ufshcd_abort_one(struct request *rq, void *priv) > +{ > + int *ret = priv; > + u32 tag = rq->tag; > + struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq); > + struct scsi_device *sdev = cmd->device; > + struct Scsi_Host *shost = sdev->host; > + struct ufs_hba *hba = shost_priv(shost); > + > + *ret = ufshcd_try_to_abort_task(hba, tag); > + dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag, > + hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1, > + *ret ? "failed" : "succeeded"); > + return *ret == 0; > +} > + > /** > * ufshcd_abort_all - Abort all pending commands. > * @hba: Host bus adapter pointer. > @@ -6395,34 +6411,12 @@ static bool > ufshcd_is_pwr_mode_restore_needed(struct ufs_hba *hba) > */ > static bool ufshcd_abort_all(struct ufs_hba *hba) > { > - int tag, ret; > + int tag, ret = 0; > > - if (is_mcq_enabled(hba)) { > - struct ufshcd_lrb *lrbp; > - int tag; > + blk_mq_tagset_busy_iter(&hba->host->tag_set, ufshcd_abort_one, > &ret); > + if (ret) > + goto out; > > - for (tag = 0; tag < hba->nutrs; tag++) { > - lrbp = &hba->lrb[tag]; > - if (!ufshcd_cmd_inflight(lrbp->cmd)) > - continue; > - ret = ufshcd_try_to_abort_task(hba, tag); > - dev_err(hba->dev, "Aborting tag %d / CDB %#02x > %s\n", tag, > - hba->lrb[tag].cmd ? hba->lrb[tag].cmd- > >cmnd[0] : -1, > - ret ? "failed" : "succeeded"); > - if (ret) > - goto out; > - } > - } else { > - /* Clear pending transfer requests */ > - for_each_set_bit(tag, &hba->outstanding_reqs, hba- > >nutrs) { > - ret = ufshcd_try_to_abort_task(hba, tag); > - dev_err(hba->dev, "Aborting tag %d / CDB %#02x > %s\n", tag, > - hba->lrb[tag].cmd ? hba->lrb[tag].cmd- > >cmnd[0] : -1, > - ret ? "failed" : "succeeded"); > - if (ret) > - goto out; > - } > - } > /* Clear pending task management requests */ > for_each_set_bit(tag, &hba->outstanding_tasks, hba->nutmrs) { > ret = ufshcd_clear_tm_cmd(hba, tag); Hi Bart, Previous ufshcd_try_to_abort_task retrun fail will break tag iterate and return true to tell caller need reset. But this patch only return last tag ufshcd_try_to_abort_task return value, if some tag abort fail and last tag abort success, will not retrun true to tell caller need reset, am I right? Thanks. Peter
On 11/14/23 22:57, Peter Wang (王信友) wrote: > On Thu, 2023-07-27 at 12:41 -0700, Bart Van Assche wrote: >> Unify the MCQ and legacy code paths. This patch reworks code >> introduced by >> commit ab248643d3d6 ("scsi: ufs: core: Add error handling for MCQ >> mode"). >> >> Cc: Bao D. Nguyen <quic_nguyenb@quicinc.com> >> Signed-off-by: Bart Van Assche <bvanassche@acm.org> >> --- >> drivers/ufs/core/ufshcd.c | 46 +++++++++++++++++------------------ >> ---- >> 1 file changed, 20 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c >> index c0031cf8855c..bf76ea59ba6c 100644 >> --- a/drivers/ufs/core/ufshcd.c >> +++ b/drivers/ufs/core/ufshcd.c >> @@ -6387,6 +6387,22 @@ static bool >> ufshcd_is_pwr_mode_restore_needed(struct ufs_hba *hba) >> return false; >> } >> >> +static bool ufshcd_abort_one(struct request *rq, void *priv) >> +{ >> + int *ret = priv; >> + u32 tag = rq->tag; >> + struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq); >> + struct scsi_device *sdev = cmd->device; >> + struct Scsi_Host *shost = sdev->host; >> + struct ufs_hba *hba = shost_priv(shost); >> + >> + *ret = ufshcd_try_to_abort_task(hba, tag); >> + dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag, >> + hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1, >> + *ret ? "failed" : "succeeded"); >> + return *ret == 0; >> +} >> + >> /** >> * ufshcd_abort_all - Abort all pending commands. >> * @hba: Host bus adapter pointer. >> @@ -6395,34 +6411,12 @@ static bool >> ufshcd_is_pwr_mode_restore_needed(struct ufs_hba *hba) >> */ >> static bool ufshcd_abort_all(struct ufs_hba *hba) >> { >> - int tag, ret; >> + int tag, ret = 0; >> >> - if (is_mcq_enabled(hba)) { >> - struct ufshcd_lrb *lrbp; >> - int tag; >> + blk_mq_tagset_busy_iter(&hba->host->tag_set, ufshcd_abort_one, >> &ret); >> + if (ret) >> + goto out; >> >> - for (tag = 0; tag < hba->nutrs; tag++) { >> - lrbp = &hba->lrb[tag]; >> - if (!ufshcd_cmd_inflight(lrbp->cmd)) >> - continue; >> - ret = ufshcd_try_to_abort_task(hba, tag); >> - dev_err(hba->dev, "Aborting tag %d / CDB %#02x >> %s\n", tag, >> - hba->lrb[tag].cmd ? hba->lrb[tag].cmd- >>> cmnd[0] : -1, >> - ret ? "failed" : "succeeded"); >> - if (ret) >> - goto out; >> - } >> - } else { >> - /* Clear pending transfer requests */ >> - for_each_set_bit(tag, &hba->outstanding_reqs, hba- >>> nutrs) { >> - ret = ufshcd_try_to_abort_task(hba, tag); >> - dev_err(hba->dev, "Aborting tag %d / CDB %#02x >> %s\n", tag, >> - hba->lrb[tag].cmd ? hba->lrb[tag].cmd- >>> cmnd[0] : -1, >> - ret ? "failed" : "succeeded"); >> - if (ret) >> - goto out; >> - } >> - } >> /* Clear pending task management requests */ >> for_each_set_bit(tag, &hba->outstanding_tasks, hba->nutmrs) { >> ret = ufshcd_clear_tm_cmd(hba, tag); > > Hi Bart, > > Previous ufshcd_try_to_abort_task retrun fail will break tag iterate > and return true to tell caller need reset. > But this patch only return last tag ufshcd_try_to_abort_task return > value, if some tag abort fail and last tag abort success, will not > retrun true to tell caller need reset, am I right? Hi Peter, blk_mq_tagset_busy_iter() keeps iterating as long as the callback function that has been passed as the second argument returns true. ufshcd_abort_one() returns false if ufshcd_try_to_abort_task() fails. The behavior that a host reset is triggered if ufshcd_try_to_abort_task() fails has been preserved. Bart.
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index c0031cf8855c..bf76ea59ba6c 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -6387,6 +6387,22 @@ static bool ufshcd_is_pwr_mode_restore_needed(struct ufs_hba *hba) return false; } +static bool ufshcd_abort_one(struct request *rq, void *priv) +{ + int *ret = priv; + u32 tag = rq->tag; + struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq); + struct scsi_device *sdev = cmd->device; + struct Scsi_Host *shost = sdev->host; + struct ufs_hba *hba = shost_priv(shost); + + *ret = ufshcd_try_to_abort_task(hba, tag); + dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag, + hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1, + *ret ? "failed" : "succeeded"); + return *ret == 0; +} + /** * ufshcd_abort_all - Abort all pending commands. * @hba: Host bus adapter pointer. @@ -6395,34 +6411,12 @@ static bool ufshcd_is_pwr_mode_restore_needed(struct ufs_hba *hba) */ static bool ufshcd_abort_all(struct ufs_hba *hba) { - int tag, ret; + int tag, ret = 0; - if (is_mcq_enabled(hba)) { - struct ufshcd_lrb *lrbp; - int tag; + blk_mq_tagset_busy_iter(&hba->host->tag_set, ufshcd_abort_one, &ret); + if (ret) + goto out; - for (tag = 0; tag < hba->nutrs; tag++) { - lrbp = &hba->lrb[tag]; - if (!ufshcd_cmd_inflight(lrbp->cmd)) - continue; - ret = ufshcd_try_to_abort_task(hba, tag); - dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag, - hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1, - ret ? "failed" : "succeeded"); - if (ret) - goto out; - } - } else { - /* Clear pending transfer requests */ - for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) { - ret = ufshcd_try_to_abort_task(hba, tag); - dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag, - hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1, - ret ? "failed" : "succeeded"); - if (ret) - goto out; - } - } /* Clear pending task management requests */ for_each_set_bit(tag, &hba->outstanding_tasks, hba->nutmrs) { ret = ufshcd_clear_tm_cmd(hba, tag);
Unify the MCQ and legacy code paths. This patch reworks code introduced by commit ab248643d3d6 ("scsi: ufs: core: Add error handling for MCQ mode"). Cc: Bao D. Nguyen <quic_nguyenb@quicinc.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/ufs/core/ufshcd.c | 46 +++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 26 deletions(-)