On Tue, May 04, 2021 at 09:49:12AM +0200, Martin Wilck wrote:

On Thu, 2021-04-29 at 19:28 +0200, Hannes Reinecke wrote:

quoted

On 4/29/21 4:34 PM, Ming Lei wrote:

quoted

On Thu, Apr 29, 2021 at 02:25:17PM +0200, Hannes Reinecke wrote:

quoted

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@suse.de>
---
  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.

Can you share the cases in which SCSI needs to deal with non in-flight
requests via scsi_host_find_tag()? which is supposed to be used for retrieving
request via one active tag in scsi io completion path.


Thanks,
Ming