diff mbox

[1/1] Using the local variable instead of I/O flag to acquire io_req_lock in fnic_queuecommand() to avoid deadloack

Message ID 1436882937-11741-1-git-send-email-hishah@cisco.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hiral Shah July 14, 2015, 2:08 p.m. UTC 
We added changes in fnic driver patch 1.6.0.16 to acquire
 io_req_lock in fnic_queuecommand() before issuing I/O so that io completion
 is serialized. But when releasing the lock we check for the I/O flag and
 this could be modified if IO abort occurs before I/O completion. In this case
 we wont release the lock and causes deadlock in some scenerios. Using the
 local variable to check the IO lock status will resolve the problem.

Signed-off-by: Hiral Shah <hishah@cisco.com>
Signed-off-by: Sesidhar Baddela <sebaddel@cisco.com>
Signed-off-by: Anil Chintalapati <achintal@cisco.com>
---
 drivers/scsi/fnic/fnic.h      | 2 +-
 drivers/scsi/fnic/fnic_scsi.c | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

Martin K. Petersen July 17, 2015, 1:56 a.m. UTC | #1 
>>>>> "Hiral" == Hiral Shah <hishah@cisco.com> writes:

Hiral> We added changes in fnic driver patch 1.6.0.16 to acquire
Hiral> io_req_lock in fnic_queuecommand() before issuing I/O so that io
Hiral> completion is serialized. But when releasing the lock we check
Hiral> for the I/O flag and this could be modified if IO abort occurs
Hiral> before I/O completion. In this case we wont release the lock and
Hiral> causes deadlock in some scenerios. Using the local variable to
Hiral> check the IO lock status will resolve the problem.

Maybe bool instead of int?

Otherwise OK.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Hiral Shah Aug. 17, 2015, 5:50 p.m. UTC | #2 
QnJvYWRjYXN0aW5n4oCmDQoNClJlZ2FyZHMsDQpIaXJhbA0KDQoNCg0KDQoNCg0KDQoNCk9uIDcv
MjAvMTUsIDU6NDggUE0sICJIaXJhbCBTaGFoIChoaXNoYWgpIiA8aGlzaGFoQGNpc2NvLmNvbT4g
d3JvdGU6DQoNCj5IaSBNYXJ0aW4sDQo+DQo+VGhhbmtzIGZvciB0aGUgc3VnZ2VzdGlvbi4gQWN0
dWFsbHkgYm9vdCBtYWtlIG1vcmUgc2Vuc2UgYXMgdGhlIHZhbHVlIHdpbGwgYmUgZWl0aGVyIDAg
b3IgMSBvbmx5LiBXZSBoYXZlIGFscmVhZHkgcmVsZWFzZSBmb2xsb3dpbmcgcGF0Y2ggdG8gb3Ro
ZXIgbm9uLWxpbnV4IGN1c3RvbWVycy4gV2Ugd2lsbCBtYWtlIHN1cmUgbmV4dCB0aW1lLg0KPg0K
Pg0KPlJlZ2FyZHMsDQo+SGlyYWwNCj4NCj4NCj4NCj4NCj4NCj4NCj4NCj4NCj5PbiA3LzE2LzE1
LCA5OjU2IFBNLCAiTWFydGluIEsuIFBldGVyc2VuIiA8bWFydGluLnBldGVyc2VuQG9yYWNsZS5j
b20+IHdyb3RlOg0KPg0KPj4+Pj4+PiAiSGlyYWwiID09IEhpcmFsIFNoYWggPGhpc2hhaEBjaXNj
by5jb20+IHdyaXRlczoNCj4+DQo+PkhpcmFsPiBXZSBhZGRlZCBjaGFuZ2VzIGluIGZuaWMgZHJp
dmVyIHBhdGNoIDEuNi4wLjE2IHRvIGFjcXVpcmUNCj4+SGlyYWw+IGlvX3JlcV9sb2NrIGluIGZu
aWNfcXVldWVjb21tYW5kKCkgYmVmb3JlIGlzc3VpbmcgSS9PIHNvIHRoYXQgaW8NCj4+SGlyYWw+
IGNvbXBsZXRpb24gaXMgc2VyaWFsaXplZC4gQnV0IHdoZW4gcmVsZWFzaW5nIHRoZSBsb2NrIHdl
IGNoZWNrDQo+PkhpcmFsPiBmb3IgdGhlIEkvTyBmbGFnIGFuZCB0aGlzIGNvdWxkIGJlIG1vZGlm
aWVkIGlmIElPIGFib3J0IG9jY3Vycw0KPj5IaXJhbD4gYmVmb3JlIEkvTyBjb21wbGV0aW9uLiBJ
biB0aGlzIGNhc2Ugd2Ugd29udCByZWxlYXNlIHRoZSBsb2NrIGFuZA0KPj5IaXJhbD4gY2F1c2Vz
IGRlYWRsb2NrIGluIHNvbWUgc2NlbmVyaW9zLiBVc2luZyB0aGUgbG9jYWwgdmFyaWFibGUgdG8N
Cj4+SGlyYWw+IGNoZWNrIHRoZSBJTyBsb2NrIHN0YXR1cyB3aWxsIHJlc29sdmUgdGhlIHByb2Js
ZW0uDQo+Pg0KPj5NYXliZSBib29sIGluc3RlYWQgb2YgaW50Pw0KPj4NCj4+T3RoZXJ3aXNlIE9L
Lg0KPj4NCj4+UmV2aWV3ZWQtYnk6IE1hcnRpbiBLLiBQZXRlcnNlbiA8bWFydGluLnBldGVyc2Vu
QG9yYWNsZS5jb20+DQo+Pg0KPj4tLSANCj4+TWFydGluIEsuIFBldGVyc2VuCU9yYWNsZSBMaW51
eCBFbmdpbmVlcmluZw0K
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index 26270c3..ce129e5 100644
--- a/drivers/scsi/fnic/fnic.h
+++ b/drivers/scsi/fnic/fnic.h
@@ -39,7 +39,7 @@ 
 
 #define DRV_NAME		"fnic"
 #define DRV_DESCRIPTION		"Cisco FCoE HBA Driver"
-#define DRV_VERSION		"1.6.0.17"
+#define DRV_VERSION		"1.6.0.17a"
 #define PFX			DRV_NAME ": "
 #define DFX                     DRV_NAME "%d: "
 
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 155b286..25436cd 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -425,6 +425,7 @@  static int fnic_queuecommand_lck(struct scsi_cmnd *sc, void (*done)(struct scsi_
 	unsigned long ptr;
 	struct fc_rport_priv *rdata;
 	spinlock_t *io_lock = NULL;
+	int io_lock_acquired = 0;
 
 	if (unlikely(fnic_chk_state_flags_locked(fnic, FNIC_FLAGS_IO_BLOCKED)))
 		return SCSI_MLQUEUE_HOST_BUSY;
@@ -518,6 +519,7 @@  static int fnic_queuecommand_lck(struct scsi_cmnd *sc, void (*done)(struct scsi_
 	spin_lock_irqsave(io_lock, flags);
 
 	/* initialize rest of io_req */
+	io_lock_acquired = 1;
 	io_req->port_id = rport->port_id;
 	io_req->start_time = jiffies;
 	CMD_STATE(sc) = FNIC_IOREQ_CMD_PENDING;
@@ -571,7 +573,7 @@  out:
 		  (((u64)CMD_FLAGS(sc) >> 32) | CMD_STATE(sc)));
 
 	/* if only we issued IO, will we have the io lock */
-	if (CMD_FLAGS(sc) & FNIC_IO_INITIALIZED)
+	if (io_lock_acquired)
 		spin_unlock_irqrestore(io_lock, flags);
 
 	atomic_dec(&fnic->in_flight);