diff mbox

[v2,10/14] qla2xxx: Fix request queue corruption.

Message ID 1486161655-2307-11-git-send-email-himanshu.madhani@cavium.com (mailing list archive)
State Deferred
Headers show

Commit Message

Madhani, Himanshu Feb. 3, 2017, 10:40 p.m. UTC 
From: Quinn Tran <quinn.tran@cavium.com>

When FW notify driver or driver detects low FW resource,
driver tries to send out Busy SCSI Status to tell Initiator
side to back off. During the send process, the lock was not held.

Signed-off-by: Quinn Tran <quinn.tran@qlogic.com>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
---
 drivers/scsi/qla2xxx/qla_target.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Bart Van Assche Feb. 8, 2017, 7:22 p.m. UTC | #1 
On Fri, 2017-02-03 at 14:40 -0800, Himanshu Madhani wrote:
> From: Quinn Tran <quinn.tran@cavium.com>
> 
> When FW notify driver or driver detects low FW resource,
> driver tries to send out Busy SCSI Status to tell Initiator
> side to back off. During the send process, the lock was not held.
> 
> Signed-off-by: Quinn Tran <quinn.tran@qlogic.com>
> Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
> ---
>  drivers/scsi/qla2xxx/qla_target.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
> index b61cbb8..b5fb9c55 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -5170,16 +5170,22 @@ static int __qlt_send_busy(struct scsi_qla_host *vha,
>  
>  static int
>  qlt_chk_qfull_thresh_hold(struct scsi_qla_host *vha,
> -	struct atio_from_isp *atio)
> +	struct atio_from_isp *atio, uint8_t ha_locked)
>  {
>  	struct qla_hw_data *ha = vha->hw;
>  	uint16_t status;
> +	unsigned long flags;
>  
>  	if (ha->tgt.num_pend_cmds < Q_FULL_THRESH_HOLD(ha))
>  		return 0;
>  
> +	if (!ha_locked)
> +		spin_lock_irqsave(&ha->hardware_lock, flags);
>  	status = temp_sam_status;
>  	qlt_send_busy(vha, atio, status);
> +	if (!ha_locked)
> +		spin_unlock_irqrestore(&ha->hardware_lock, flags);
> +
>  	return 1;
>  }
>  
> @@ -5224,7 +5230,7 @@ static void qlt_24xx_atio_pkt(struct scsi_qla_host *vha,
>  
>  
>  		if (likely(atio->u.isp24.fcp_cmnd.task_mgmt_flags == 0)) {
> -			rc = qlt_chk_qfull_thresh_hold(vha, atio);
> +			rc = qlt_chk_qfull_thresh_hold(vha, atio, ha_locked);
>  			if (rc != 0) {
>  				tgt->atio_irq_cmd_count--;
>  				return;
> @@ -5347,7 +5353,7 @@ static void qlt_response_pkt(struct scsi_qla_host *vha, response_t *pkt)
>  			break;
>  		}
>  
> -		rc = qlt_chk_qfull_thresh_hold(vha, atio);
> +		rc = qlt_chk_qfull_thresh_hold(vha, atio, 1);
>  		if (rc != 0) {
>  			tgt->irq_cmd_count--;
>  			return;

Hello Quinn,

Please consider to use bool instead of uint8_t for ha_locked and true /
false instead of 1 / 0.

Bart.--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tran, Quinn Feb. 9, 2017, 9:34 p.m. UTC | #2 
QmFydCwgdGhhbmtzIGZvciByZXZpZXdpbmcuICBXaWxsIGNsZWFuIGl0IHVwLg0KDQpSZWdhcmRz
LA0KUXVpbm4gVHJhbg0KDQotLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KRnJvbTogPGxpbnV4
LXNjc2ktb3duZXJAdmdlci5rZXJuZWwub3JnPiBvbiBiZWhhbGYgb2YgQmFydCBWYW4gQXNzY2hl
IDxCYXJ0LlZhbkFzc2NoZUBzYW5kaXNrLmNvbT4NCkRhdGU6IFdlZG5lc2RheSwgRmVicnVhcnkg
OCwgMjAxNyBhdCAxMToyMiBBTQ0KVG86ICJoY2hAaW5mcmFkZWFkLm9yZyIgPGhjaEBpbmZyYWRl
YWQub3JnPiwgIk1hZGhhbmksIEhpbWFuc2h1IiA8SGltYW5zaHUuTWFkaGFuaUBjYXZpdW0uY29t
PiwgdGFyZ2V0LWRldmVsIDx0YXJnZXQtZGV2ZWxAdmdlci5rZXJuZWwub3JnPiwgTmljaG9sYXMg
QmVsbGluZ2VyIDxuYWJAbGludXgtaXNjc2kub3JnPg0KQ2M6ICJsaW51eC1zY3NpQHZnZXIua2Vy
bmVsLm9yZyIgPGxpbnV4LXNjc2lAdmdlci5rZXJuZWwub3JnPiwgIk1hbGF2YWxpLCBHaXJpZGhh
ciIgPEdpcmlkaGFyLk1hbGF2YWxpQGNhdml1bS5jb20+DQpTdWJqZWN0OiBSZTogW1BBVENIIHYy
IDEwLzE0XSBxbGEyeHh4OiBGaXggcmVxdWVzdCBxdWV1ZSBjb3JydXB0aW9uLg0KUmVzZW50LUZy
b206IDxxdWlubi50cmFuQHFsb2dpYy5jb20+DQoNCiAgICBPbiBGcmksIDIwMTctMDItMDMgYXQg
MTQ6NDAgLTA4MDAsIEhpbWFuc2h1IE1hZGhhbmkgd3JvdGU6DQogICAgPiBGcm9tOiBRdWlubiBU
cmFuIDxxdWlubi50cmFuQGNhdml1bS5jb20+DQogICAgPiANCiAgICA+IFdoZW4gRlcgbm90aWZ5
IGRyaXZlciBvciBkcml2ZXIgZGV0ZWN0cyBsb3cgRlcgcmVzb3VyY2UsDQogICAgPiBkcml2ZXIg
dHJpZXMgdG8gc2VuZCBvdXQgQnVzeSBTQ1NJIFN0YXR1cyB0byB0ZWxsIEluaXRpYXRvcg0KICAg
ID4gc2lkZSB0byBiYWNrIG9mZi4gRHVyaW5nIHRoZSBzZW5kIHByb2Nlc3MsIHRoZSBsb2NrIHdh
cyBub3QgaGVsZC4NCiAgICA+IA0KICAgID4gU2lnbmVkLW9mZi1ieTogUXVpbm4gVHJhbiA8cXVp
bm4udHJhbkBxbG9naWMuY29tPg0KICAgID4gU2lnbmVkLW9mZi1ieTogSGltYW5zaHUgTWFkaGFu
aSA8aGltYW5zaHUubWFkaGFuaUBjYXZpdW0uY29tPg0KICAgID4gLS0tDQogICAgPiAgZHJpdmVy
cy9zY3NpL3FsYTJ4eHgvcWxhX3RhcmdldC5jIHwgMTIgKysrKysrKysrLS0tDQogICAgPiAgMSBm
aWxlIGNoYW5nZWQsIDkgaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMoLSkNCiAgICA+IA0KICAg
ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvc2NzaS9xbGEyeHh4L3FsYV90YXJnZXQuYyBiL2RyaXZl
cnMvc2NzaS9xbGEyeHh4L3FsYV90YXJnZXQuYw0KICAgID4gaW5kZXggYjYxY2JiOC4uYjVmYjlj
NTUgMTAwNjQ0DQogICAgPiAtLS0gYS9kcml2ZXJzL3Njc2kvcWxhMnh4eC9xbGFfdGFyZ2V0LmMN
CiAgICA+ICsrKyBiL2RyaXZlcnMvc2NzaS9xbGEyeHh4L3FsYV90YXJnZXQuYw0KICAgID4gQEAg
LTUxNzAsMTYgKzUxNzAsMjIgQEAgc3RhdGljIGludCBfX3FsdF9zZW5kX2J1c3koc3RydWN0IHNj
c2lfcWxhX2hvc3QgKnZoYSwNCiAgICA+ICANCiAgICA+ICBzdGF0aWMgaW50DQogICAgPiAgcWx0
X2Noa19xZnVsbF90aHJlc2hfaG9sZChzdHJ1Y3Qgc2NzaV9xbGFfaG9zdCAqdmhhLA0KICAgID4g
LQlzdHJ1Y3QgYXRpb19mcm9tX2lzcCAqYXRpbykNCiAgICA+ICsJc3RydWN0IGF0aW9fZnJvbV9p
c3AgKmF0aW8sIHVpbnQ4X3QgaGFfbG9ja2VkKQ0KICAgID4gIHsNCiAgICA+ICAJc3RydWN0IHFs
YV9od19kYXRhICpoYSA9IHZoYS0+aHc7DQogICAgPiAgCXVpbnQxNl90IHN0YXR1czsNCiAgICA+
ICsJdW5zaWduZWQgbG9uZyBmbGFnczsNCiAgICA+ICANCiAgICA+ICAJaWYgKGhhLT50Z3QubnVt
X3BlbmRfY21kcyA8IFFfRlVMTF9USFJFU0hfSE9MRChoYSkpDQogICAgPiAgCQlyZXR1cm4gMDsN
CiAgICA+ICANCiAgICA+ICsJaWYgKCFoYV9sb2NrZWQpDQogICAgPiArCQlzcGluX2xvY2tfaXJx
c2F2ZSgmaGEtPmhhcmR3YXJlX2xvY2ssIGZsYWdzKTsNCiAgICA+ICAJc3RhdHVzID0gdGVtcF9z
YW1fc3RhdHVzOw0KICAgID4gIAlxbHRfc2VuZF9idXN5KHZoYSwgYXRpbywgc3RhdHVzKTsNCiAg
ICA+ICsJaWYgKCFoYV9sb2NrZWQpDQogICAgPiArCQlzcGluX3VubG9ja19pcnFyZXN0b3JlKCZo
YS0+aGFyZHdhcmVfbG9jaywgZmxhZ3MpOw0KICAgID4gKw0KICAgID4gIAlyZXR1cm4gMTsNCiAg
ICA+ICB9DQogICAgPiAgDQogICAgPiBAQCAtNTIyNCw3ICs1MjMwLDcgQEAgc3RhdGljIHZvaWQg
cWx0XzI0eHhfYXRpb19wa3Qoc3RydWN0IHNjc2lfcWxhX2hvc3QgKnZoYSwNCiAgICA+ICANCiAg
ICA+ICANCiAgICA+ICAJCWlmIChsaWtlbHkoYXRpby0+dS5pc3AyNC5mY3BfY21uZC50YXNrX21n
bXRfZmxhZ3MgPT0gMCkpIHsNCiAgICA+IC0JCQlyYyA9IHFsdF9jaGtfcWZ1bGxfdGhyZXNoX2hv
bGQodmhhLCBhdGlvKTsNCiAgICA+ICsJCQlyYyA9IHFsdF9jaGtfcWZ1bGxfdGhyZXNoX2hvbGQo
dmhhLCBhdGlvLCBoYV9sb2NrZWQpOw0KICAgID4gIAkJCWlmIChyYyAhPSAwKSB7DQogICAgPiAg
CQkJCXRndC0+YXRpb19pcnFfY21kX2NvdW50LS07DQogICAgPiAgCQkJCXJldHVybjsNCiAgICA+
IEBAIC01MzQ3LDcgKzUzNTMsNyBAQCBzdGF0aWMgdm9pZCBxbHRfcmVzcG9uc2VfcGt0KHN0cnVj
dCBzY3NpX3FsYV9ob3N0ICp2aGEsIHJlc3BvbnNlX3QgKnBrdCkNCiAgICA+ICAJCQlicmVhazsN
CiAgICA+ICAJCX0NCiAgICA+ICANCiAgICA+IC0JCXJjID0gcWx0X2Noa19xZnVsbF90aHJlc2hf
aG9sZCh2aGEsIGF0aW8pOw0KICAgID4gKwkJcmMgPSBxbHRfY2hrX3FmdWxsX3RocmVzaF9ob2xk
KHZoYSwgYXRpbywgMSk7DQogICAgPiAgCQlpZiAocmMgIT0gMCkgew0KICAgID4gIAkJCXRndC0+
aXJxX2NtZF9jb3VudC0tOw0KICAgID4gIAkJCXJldHVybjsNCiAgICANCiAgICBIZWxsbyBRdWlu
biwNCiAgICANCiAgICBQbGVhc2UgY29uc2lkZXIgdG8gdXNlIGJvb2wgaW5zdGVhZCBvZiB1aW50
OF90IGZvciBoYV9sb2NrZWQgYW5kIHRydWUgLw0KICAgIGZhbHNlIGluc3RlYWQgb2YgMSAvIDAu
DQogICAgDQogICAgQmFydC4NCiAgICANCg0K
--
To unsubscribe from this list: send the line "unsubscribe target-devel" 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/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index b61cbb8..b5fb9c55 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -5170,16 +5170,22 @@  static int __qlt_send_busy(struct scsi_qla_host *vha,
 
 static int
 qlt_chk_qfull_thresh_hold(struct scsi_qla_host *vha,
-	struct atio_from_isp *atio)
+	struct atio_from_isp *atio, uint8_t ha_locked)
 {
 	struct qla_hw_data *ha = vha->hw;
 	uint16_t status;
+	unsigned long flags;
 
 	if (ha->tgt.num_pend_cmds < Q_FULL_THRESH_HOLD(ha))
 		return 0;
 
+	if (!ha_locked)
+		spin_lock_irqsave(&ha->hardware_lock, flags);
 	status = temp_sam_status;
 	qlt_send_busy(vha, atio, status);
+	if (!ha_locked)
+		spin_unlock_irqrestore(&ha->hardware_lock, flags);
+
 	return 1;
 }
 
@@ -5224,7 +5230,7 @@  static void qlt_24xx_atio_pkt(struct scsi_qla_host *vha,
 
 
 		if (likely(atio->u.isp24.fcp_cmnd.task_mgmt_flags == 0)) {
-			rc = qlt_chk_qfull_thresh_hold(vha, atio);
+			rc = qlt_chk_qfull_thresh_hold(vha, atio, ha_locked);
 			if (rc != 0) {
 				tgt->atio_irq_cmd_count--;
 				return;
@@ -5347,7 +5353,7 @@  static void qlt_response_pkt(struct scsi_qla_host *vha, response_t *pkt)
 			break;
 		}
 
-		rc = qlt_chk_qfull_thresh_hold(vha, atio);
+		rc = qlt_chk_qfull_thresh_hold(vha, atio, 1);
 		if (rc != 0) {
 			tgt->irq_cmd_count--;
 			return;