On Thu, 2020-08-06 at 18:07 +0800, Can Guo wrote:
> Hi Bean,
>
> On 2020-08-06 17:50, Bean Huo wrote:
> > >
> > > Please check Stanley's recent change to ufshcd_abort, you may
> > > want to rebase your change on his and do goto cleanup here.
> > > @Stanley correct me if I am wrong.
> > >
> > > But even if you do a goto cleanup here, we still lost the
> > > chances to dump host infos/regs like it does in the old code.
> > > If a cmd was completed but without a notifying intr, this is
> > > kind of a problem that we/host should look into, because it's
> > > pasted at least 30 sec since the cmd was sent, so those dumps
> > > are necessary to debug the problem. How about moving blow prints
> > > in front of this part?
> > >
> > > Thanks,
> > >
> > > Can Guo.
> > >
> > > > }
> > > >
> > > > /* Print Transfer Request of aborted task */
> >
> > Hi Can
> >
> > Thanks, do you mean that change to like this:
> >
> >
> > Author: Bean Huo <beanhuo@xxxxxxxxxx>
> > Date: Thu Aug 6 11:34:45 2020 +0200
> >
> > scsi: ufs: no need to send one Abort Task TM in case the task
> > in
> > was cleared
> >
> > If the bit corresponds to a task in the Doorbell register has
> > been
> > cleared, no need to poll the status of the task on the device
> > side
> > and to send an Abort Task TM.
> > This patch also deletes dispensable dev_err() in case of the
> > task
> > already completed.
> >
> > Signed-off-by: Bean Huo <beanhuo@xxxxxxxxxx>
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 307622284239..f7c91ce9e294 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -6425,23 +6425,9 @@ static int ufshcd_abort(struct scsi_cmnd
> > *cmd)
> > return ufshcd_eh_host_reset_handler(cmd);
> >
> > ufshcd_hold(hba, false);
> > - reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> > /* If command is already aborted/completed, return SUCCESS
> > */
> > - if (!(test_bit(tag, &hba->outstanding_reqs))) {
> > - dev_err(hba->dev,
> > - "%s: cmd at tag %d already completed,
> > outstanding=0x%lx, doorbell=0x%x\n",
> > - __func__, tag, hba->outstanding_reqs, reg);
> > + if (!(test_bit(tag, &hba->outstanding_reqs)))
> > goto out;
> > - }
> > -
> > - if (!(reg & (1 << tag))) {
> > - dev_err(hba->dev,
> > - "%s: cmd was completed, but without a notifying
> > intr,
> > tag = %d",
> > - __func__, tag);
> > - }
> > -
> > - /* Print Transfer Request of aborted task */
> > - dev_err(hba->dev, "%s: Device abort task at tag %d\n",
> > __func__, tag);
> >
> > /*
> > * Print detailed info about aborted request.
> > @@ -6462,6 +6448,17 @@ static int ufshcd_abort(struct scsi_cmnd
> > *cmd)
> > }
> > hba->req_abort_count++;
> >
> > + reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> > + if (!(reg & (1 << tag))) {
> > + dev_err(hba->dev,
> > + "%s: cmd was completed, but without a notifying
> > intr,
> > tag = %d",
> > + __func__, tag);
> > + goto cleanup;
> > + }
> > +
> > + /* Print Transfer Request of aborted task */
> > + dev_err(hba->dev, "%s: Device abort task at tag %d\n",
> > __func__, tag);
> > +
>
> The rest looks good but let below two lines stay where they were.
>
> /* Print Transfer Request of aborted task */
> dev_err(hba->dev, "%s: Device abort task at tag %d\n",
> __func__, tag);
>
>
> Thanks,
>
> Can Guo.
>
Hi Can
I will change it in the next version.
Hi Stanly
would you mind I take your patch into my next version patchset? Since
we both will add a new same goto label. I will keep your patch
authorship.
Thanks,
Bean
