RE: [PATCH v3] scsi: ufs: Cleanup completed request without interrupt notification

From: Avri Altman <Avri.Altman@wdc.com>
To: Stanley Chu <stanley.chu@mediatek.com>,
	Bart Van Assche <bvanassche@acm.org>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
	"alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
	"jejb@linux.ibm.com" <jejb@linux.ibm.com>,
	"beanhuo@micron.com" <beanhuo@micron.com>,
	"asutoshd@codeaurora.org" <asutoshd@codeaurora.org>,
	"cang@codeaurora.org" <cang@codeaurora.org>,
	"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
	"linux-mediatek@lists.infradead.org" 
	<linux-mediatek@lists.infradead.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kuohong.wang@mediatek.com" <kuohong.wang@mediatek.com>,
	"peter.wang@mediatek.com" <peter.wang@mediatek.com>,
	"chun-hung.wu@mediatek.com" <chun-hung.wu@mediatek.com>,
	"andy.teng@mediatek.com" <andy.teng@mediatek.com>,
	"chaotian.jing@mediatek.com" <chaotian.jing@mediatek.com>,
	"cc.chou@mediatek.com" <cc.chou@mediatek.com>
Subject: RE: [PATCH v3] scsi: ufs: Cleanup completed request without interrupt notification
Date: Mon, 13 Jul 2020 08:10:29 +0000	[thread overview]
Message-ID: <SN6PR04MB46409838AE9D4BD63797E26DFC600@SN6PR04MB4640.namprd04.prod.outlook.com> (raw)
In-Reply-To: <1594607245.22878.8.camel@mtkswgap22>

> 
> Hi Bart and Avri,
> 
> On Sun, 2020-07-12 at 18:39 -0700, Bart Van Assche wrote:
> > On 2020-07-06 06:21, Stanley Chu wrote:
> > > If somehow no interrupt notification is raised for a completed request
> > > and its doorbell bit is cleared by host, UFS driver needs to cleanup
> > > its outstanding bit in ufshcd_abort().
> >
> > How is it possible that no interrupt notification is raised for a completed
> > request? Is this the result of a hardware shortcoming or rather the result
> > of how the UFS driver works? In the latter case, is this patch perhaps a
> > workaround? If so, has it been considered to fix the root cause instead of
> > implementing a workaround?
> 
> Actually this fail is triggered by "error injection" to produce a
> command timeout event for checking if anything can be improved or fixed.
> 
> I agree that "no interrupt notification" may be something wrong in
> hardware and the root cause shall be fixed in the highest priority.
> However from this injection, we found ufshcd_abort() indeed has a defect
> flow for a corner case, so we are looking for the solution to fix the
> "hole".
> 
> What would you think if Linux driver shall consider this case? If this
> is not necessary, I would drop this patch : )
Artificially injecting errors is a very common validation mechanism,
Provided that you are not breaking anything of the upper-layers,
Which I don't think you are doing.

Can you refer please to my last comment?

> 
> Thanks a lot,
> Stanley Chu
> 
> >
> > In section 7.2.3 of the UFS specification I found the following about how
> > to process request completions: "Software determines if new TRs have
> > completed since step #2, by repeating one of the two methods described in
> > step #2. If new TRs have completed, software repeats the sequence from
> step
> > #3." Is such a loop perhaps missing from the Linux UFS driver?
Could not find that citation.
What version of the spec are you using?

Thanks,
Avri
> >
> > Thanks,
> >
> > Bart.