| Message ID | 20211104181059.4129537-1-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | scsi: ufs: Improve SCSI abort handling | expand |
On Thu, 2021-11-04 at 11:10 -0700, Bart Van Assche wrote: > The following has been observed on a test setup: > > WARNING: CPU: 4 PID: 250 at drivers/scsi/ufs/ufshcd.c:2737 > ufshcd_queuecommand+0x468/0x65c > Call trace: > ufshcd_queuecommand+0x468/0x65c > scsi_send_eh_cmnd+0x224/0x6a0 > scsi_eh_test_devices+0x248/0x418 > scsi_eh_ready_devs+0xc34/0xe58 > scsi_error_handler+0x204/0x80c > kthread+0x150/0x1b4 > ret_from_fork+0x10/0x30 > > That warning is triggered by the following statement: > > WARN_ON(lrbp->cmd); > > Fix this warning by clearing lrbp->cmd from the abort handler. > > Fixes: 7a3e97b0dc4b ("[SCSI] ufshcd: UFS Host controller driver") > Signed-off-by: Bart Van Assche <bvanassche@acm.org> Reviewed-by: Bean Huo <beanhuo@micron.com> > --- > drivers/scsi/ufs/ufshcd.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 3b4a714e2f68..d8a59258b1dc 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -7069,6 +7069,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) > goto release; > } > > + lrbp->cmd = NULL; > err = SUCCESS; > > release:
Hi Bart, On Thu, 2021-11-04 at 11:10 -0700, Bart Van Assche wrote: > The following has been observed on a test setup: > > WARNING: CPU: 4 PID: 250 at drivers/scsi/ufs/ufshcd.c:2737 > ufshcd_queuecommand+0x468/0x65c > Call trace: > ufshcd_queuecommand+0x468/0x65c > scsi_send_eh_cmnd+0x224/0x6a0 > scsi_eh_test_devices+0x248/0x418 > scsi_eh_ready_devs+0xc34/0xe58 > scsi_error_handler+0x204/0x80c > kthread+0x150/0x1b4 > ret_from_fork+0x10/0x30 > > That warning is triggered by the following statement: > > WARN_ON(lrbp->cmd); > > Fix this warning by clearing lrbp->cmd from the abort handler. > > Fixes: 7a3e97b0dc4b ("[SCSI] ufshcd: UFS Host controller driver") > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/scsi/ufs/ufshcd.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 3b4a714e2f68..d8a59258b1dc 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -7069,6 +7069,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) > goto release; > } > > + lrbp->cmd = NULL; > err = SUCCESS; > > release: Thanks for this patch! We are looking at the same issue as well recently. Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
Bart, > That warning is triggered by the following statement: > > WARN_ON(lrbp->cmd); > > Fix this warning by clearing lrbp->cmd from the abort handler. Applied to 5.16/scsi-staging, thanks!
On Thu, 4 Nov 2021 11:10:53 -0700, Bart Van Assche wrote: > The following has been observed on a test setup: > > WARNING: CPU: 4 PID: 250 at drivers/scsi/ufs/ufshcd.c:2737 ufshcd_queuecommand+0x468/0x65c > Call trace: > ufshcd_queuecommand+0x468/0x65c > scsi_send_eh_cmnd+0x224/0x6a0 > scsi_eh_test_devices+0x248/0x418 > scsi_eh_ready_devs+0xc34/0xe58 > scsi_error_handler+0x204/0x80c > kthread+0x150/0x1b4 > ret_from_fork+0x10/0x30 > > [...] Applied to 5.16/scsi-fixes, thanks! [1/1] scsi: ufs: Improve SCSI abort handling https://git.kernel.org/mkp/scsi/c/3ff1f6b6ba6f
On Thu, 2021-11-18 at 23:16 -0500, Martin K. Petersen wrote: > On Thu, 4 Nov 2021 11:10:53 -0700, Bart Van Assche wrote: > > > The following has been observed on a test setup: > > > > WARNING: CPU: 4 PID: 250 at drivers/scsi/ufs/ufshcd.c:2737 > > ufshcd_queuecommand+0x468/0x65c > > Call trace: > > ufshcd_queuecommand+0x468/0x65c > > scsi_send_eh_cmnd+0x224/0x6a0 > > scsi_eh_test_devices+0x248/0x418 > > scsi_eh_ready_devs+0xc34/0xe58 > > scsi_error_handler+0x204/0x80c > > kthread+0x150/0x1b4 > > ret_from_fork+0x10/0x30 > > > > [...] > > Applied to 5.16/scsi-fixes, thanks! > > [1/1] scsi: ufs: Improve SCSI abort handling > https://git.kernel.org/mkp/scsi/c/3ff1f6b6ba6f OK, so now we have a conflict between fixes and queue. My impression is that the patch causing the conflict: https://lore.kernel.org/all/20211203231950.193369-14-bvanassche@acm.org/ Actually supersedes this one, so I can simply drop the entirety of this patch in fixes, is that correct? James
On 12/14/21 8:35 AM, James Bottomley wrote: > On Thu, 2021-11-18 at 23:16 -0500, Martin K. Petersen wrote: >> Applied to 5.16/scsi-fixes, thanks! >> >> [1/1] scsi: ufs: Improve SCSI abort handling >> https://git.kernel.org/mkp/scsi/c/3ff1f6b6ba6f > > OK, so now we have a conflict between fixes and queue. My impression > is that the patch causing the conflict: > > https://lore.kernel.org/all/20211203231950.193369-14-bvanassche@acm.org/ > > Actually supersedes this one, so I can simply drop the entirety of this > patch in fixes, is that correct? Hi James, Commit 1fbaa02dfd05 ("scsi: ufs: Improve SCSI abort handling further") is intended as an improvement for commit 3ff1f6b6ba6f ("scsi: ufs: core: Improve SCSI abort handling"). Since commit 3ff1f6b6ba6f is already in Linus' tree I don't think that it can be dropped? A possible approach is to revert commit 3ff1f6b6ba6f before merging the mkp-scsi/for-next branch. Thanks, Bart.
On Tue, 2021-12-14 at 09:37 -0800, Bart Van Assche wrote: > On 12/14/21 8:35 AM, James Bottomley wrote: > > On Thu, 2021-11-18 at 23:16 -0500, Martin K. Petersen wrote: > > > Applied to 5.16/scsi-fixes, thanks! > > > > > > [1/1] scsi: ufs: Improve SCSI abort handling > > > https://git.kernel.org/mkp/scsi/c/3ff1f6b6ba6f > > > > OK, so now we have a conflict between fixes and queue. My > > impression > > is that the patch causing the conflict: > > > > https://lore.kernel.org/all/20211203231950.193369-14-bvanassche@acm.org/ > > > > Actually supersedes this one, so I can simply drop the entirety of > > this patch in fixes, is that correct? > > Hi James, > > Commit 1fbaa02dfd05 ("scsi: ufs: Improve SCSI abort handling > further") is intended as an improvement for commit 3ff1f6b6ba6f > ("scsi: ufs: core: Improve SCSI abort handling"). Since commit > 3ff1f6b6ba6f is already in Linus' tree I don't think that it can be > dropped? A possible approach is to revert commit 3ff1f6b6ba6f before > merging the mkp-scsi/for-next branch. I meant the effect of the fixes patch can be dropped in the merge commit. So the sole surviving code is from the misc tree. Like what I did at the top of the for-next branch: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git/commit/?h=for-next&id=014adbc9a838772b265834a55cd7b13eb2665d7e James
On 12/14/21 9:43 AM, James Bottomley wrote: > I meant the effect of the fixes patch can be dropped in the merge > commit. So the sole surviving code is from the misc tree. Like what I > did at the top of the for-next branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git/commit/?h=for-next&id=014adbc9a838772b265834a55cd7b13eb2665d7e Thanks for the clarification. The ufshcd_abort() code at the above URL looks good to me. Thanks, Bart.
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 3b4a714e2f68..d8a59258b1dc 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -7069,6 +7069,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) goto release; } + lrbp->cmd = NULL; err = SUCCESS; release:
The following has been observed on a test setup: WARNING: CPU: 4 PID: 250 at drivers/scsi/ufs/ufshcd.c:2737 ufshcd_queuecommand+0x468/0x65c Call trace: ufshcd_queuecommand+0x468/0x65c scsi_send_eh_cmnd+0x224/0x6a0 scsi_eh_test_devices+0x248/0x418 scsi_eh_ready_devs+0xc34/0xe58 scsi_error_handler+0x204/0x80c kthread+0x150/0x1b4 ret_from_fork+0x10/0x30 That warning is triggered by the following statement: WARN_ON(lrbp->cmd); Fix this warning by clearing lrbp->cmd from the abort handler. Fixes: 7a3e97b0dc4b ("[SCSI] ufshcd: UFS Host controller driver") Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/scsi/ufs/ufshcd.c | 1 + 1 file changed, 1 insertion(+)