On Thu, 2021-04-29 at 19:28 +0200, Hannes Reinecke wrote:
> On 4/29/21 4:34 PM, Ming Lei wrote:
> > On Thu, Apr 29, 2021 at 02:25:17PM +0200, Hannes Reinecke wrote:
> > > fnic_wq_copy_cleanup_handler() is using scsi_host_find_tag() to
> > > map id to a scsi command. However, as per discussion on the
> > > mailinglist
> > > scsi_host_find_tag() might return a non-started request, so we
> > > need
> > > to check the returned command with blk_mq_request_started() to
> > > avoid
> > > the function tripping over a non-initialized command.
> > >
> > > Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
> > > ---
> > > drivers/scsi/fnic/fnic_scsi.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/scsi/fnic/fnic_scsi.c
> > > b/drivers/scsi/fnic/fnic_scsi.c
> > > index 762cc8bd2653..b9fd3d87416b 100644
> > > --- a/drivers/scsi/fnic/fnic_scsi.c
> > > +++ b/drivers/scsi/fnic/fnic_scsi.c
> > > @@ -1466,7 +1466,7 @@ void fnic_wq_copy_cleanup_handler(struct
> > > vnic_wq_copy *wq,
> > > return;
> > >
> > > sc = scsi_host_find_tag(fnic->lport->host, id);
> > > - if (!sc)
> > > + if (!sc || !blk_mq_request_started(sc->request))
> > > return;
> >
> > scsi_host_find_tag() has covered blk_mq_request_started check
> > already, so
> > this patch isn't necessary.
> >
> Right. So drop it, then.
While you are at this, could you please re-consider e73a5e8e8003
("scsi: core: Only return started requests from scsi_host_find_tag()")
in general?
I have come to think that commit is incorrect. It was created as an
attempt to fix the observed fnic crashes, but it was ineffective. The
crashes were eventually fixed by patch 2/3 of this series.
IMO scsi_host_find_tag() should return a request if there is one,
regardless whether or not it's started, and leave the decision to
ignore pending requests to the caller, like it used to be until v5.8.
Regards
Martin
