diff options
author | Karan Tilak Kumar <kartilak@cisco.com> | 2023-08-17 21:21:46 +0300 |
---|---|---|
committer | Martin K. Petersen <martin.petersen@oracle.com> | 2023-08-26 00:15:09 +0300 |
commit | 15924b0503630016dee4dbb945a8df4df659070b (patch) | |
tree | a23dcbed97bfb274091c5d1cbabacbd5825504e0 | |
parent | 530e86c745ae3342b1df5e8f38529b9f8a6cac17 (diff) | |
download | linux-15924b0503630016dee4dbb945a8df4df659070b.tar.xz |
scsi: fnic: Replace sgreset tag with max_tag_id
sgreset is issued with a SCSI command pointer. The device reset code
assumes that it was issued on a hardware queue, and calls block multiqueue
layer. However, the assumption is broken, and there is no hardware queue
associated with the sgreset, and this leads to a crash due to a null
pointer exception.
Fix the code to use the max_tag_id as a tag which does not overlap with the
other tags issued by mid layer.
Tested by running FC traffic for a few minutes, and by issuing sgreset on
the device in parallel. Without the fix, the crash is observed right away.
With this fix, no crash is observed.
Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Tested-by: Karan Tilak Kumar <kartilak@cisco.com>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20230817182146.229059-1-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
-rw-r--r-- | drivers/scsi/fnic/fnic.h | 3 | ||||
-rw-r--r-- | drivers/scsi/fnic/fnic_scsi.c | 20 |
2 files changed, 11 insertions, 12 deletions
diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h index d82de34f6fd7..93c68931a593 100644 --- a/drivers/scsi/fnic/fnic.h +++ b/drivers/scsi/fnic/fnic.h @@ -27,7 +27,7 @@ #define DRV_NAME "fnic" #define DRV_DESCRIPTION "Cisco FCoE HBA Driver" -#define DRV_VERSION "1.6.0.54" +#define DRV_VERSION "1.6.0.56" #define PFX DRV_NAME ": " #define DFX DRV_NAME "%d: " @@ -236,6 +236,7 @@ struct fnic { unsigned int wq_count; unsigned int cq_count; + struct mutex sgreset_mutex; struct dentry *fnic_stats_debugfs_host; struct dentry *fnic_stats_debugfs_file; struct dentry *fnic_reset_debugfs_file; diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c index 26dbd347156e..8ce3e3c3a882 100644 --- a/drivers/scsi/fnic/fnic_scsi.c +++ b/drivers/scsi/fnic/fnic_scsi.c @@ -2220,7 +2220,6 @@ int fnic_device_reset(struct scsi_cmnd *sc) struct reset_stats *reset_stats; int tag = rq->tag; DECLARE_COMPLETION_ONSTACK(tm_done); - int tag_gen_flag = 0; /*to track tags allocated by fnic driver*/ bool new_sc = 0; /* Wait for rport to unblock */ @@ -2250,17 +2249,17 @@ int fnic_device_reset(struct scsi_cmnd *sc) } fnic_priv(sc)->flags = FNIC_DEVICE_RESET; - /* Allocate tag if not present */ if (unlikely(tag < 0)) { /* - * Really should fix the midlayer to pass in a proper - * request for ioctls... + * For device reset issued through sg3utils, we let + * only one LUN_RESET to go through and use a special + * tag equal to max_tag_id so that we don't have to allocate + * or free it. It won't interact with tags + * allocated by mid layer. */ - tag = fnic_scsi_host_start_tag(fnic, sc); - if (unlikely(tag == SCSI_NO_TAG)) - goto fnic_device_reset_end; - tag_gen_flag = 1; + mutex_lock(&fnic->sgreset_mutex); + tag = fnic->fnic_max_tag_id; new_sc = 1; } io_lock = fnic_io_lock_hash(fnic, sc); @@ -2432,9 +2431,8 @@ fnic_device_reset_end: (u64)sc->cmnd[4] << 8 | sc->cmnd[5]), fnic_flags_and_state(sc)); - /* free tag if it is allocated */ - if (unlikely(tag_gen_flag)) - fnic_scsi_host_end_tag(fnic, sc); + if (new_sc) + mutex_unlock(&fnic->sgreset_mutex); FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host, "Returning from device reset %s\n", |