All of lore.kernel.org
 help / color / mirror / Atom feed
From: Himanshu Madhani <himanshu.madhani@cavium.com>
To: target-devel@vger.kernel.org, Bart.VanAssche@sandisk.com,
	hch@infradead.org, nab@linux-iscsi.org
Cc: giridhar.malavali@cavium.com, linux-scsi@vger.kernel.org,
	himanshu.madhani@cavium.com
Subject: [PATCH 14/15] qla2xxx: Fix sess_lock & hardware_lock lock order problem.
Date: Thu, 2 Feb 2017 11:42:37 -0800	[thread overview]
Message-ID: <1486064558-365-15-git-send-email-himanshu.madhani@cavium.com> (raw)
In-Reply-To: <1486064558-365-1-git-send-email-himanshu.madhani@cavium.com>

From: Quinn Tran <quinn.tran@cavium.com>

The main lock that needs to be held for CMD or TMR submission
to upper layer is the sess_lock. The sess_lock is used to
serialize cmd submission and session deletion. The addition
of hardware_lock being held is not necessary. This patch removes
hardware_lock dependency from CMD/TMR submission.

Use hardware_lock only for error response in this case.

Path1
       CPU0                    CPU1
       ----                    ----
  lock(&(&ha->tgt.sess_lock)->rlock);
                               lock(&(&ha->hardware_lock)->rlock);
                               lock(&(&ha->tgt.sess_lock)->rlock);
  lock(&(&ha->hardware_lock)->rlock);

Path2/deadlock
*** DEADLOCK ***
Call Trace:
dump_stack+0x85/0xc2
print_circular_bug+0x1e3/0x250
__lock_acquire+0x1425/0x1620
lock_acquire+0xbf/0x210
_raw_spin_lock_irqsave+0x53/0x70
qlt_sess_work_fn+0x21d/0x480 [qla2xxx]
process_one_work+0x1f4/0x6e0

Cc: <stable@vger.kernel.org>
Cc: Bart Van Assche <Bart.VanAssche@sandisk.com>
Reported-by: Bart Van Assche <Bart.VanAssche@sandisk.com>
Signed-off-by: Quinn Tran <quinn.tran@cavium.com>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
---
 drivers/scsi/qla2xxx/qla_target.c | 41 +++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 7c4adcf..ed63fa7 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -5835,30 +5835,23 @@ static void qlt_abort_work(struct qla_tgt *tgt,
 		}
 	}
 
-	spin_lock_irqsave(&ha->hardware_lock, flags);
-
-	if (tgt->tgt_stop)
-		goto out_term;
-
 	rc = __qlt_24xx_handle_abts(vha, &prm->abts, sess);
+	ha->tgt.tgt_ops->put_sess(sess);
+	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
+
 	if (rc != 0)
 		goto out_term;
-	spin_unlock_irqrestore(&ha->hardware_lock, flags);
-	if (sess)
-		ha->tgt.tgt_ops->put_sess(sess);
-	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
 	return;
 
 out_term2:
-	spin_lock_irqsave(&ha->hardware_lock, flags);
+	if (sess)
+		ha->tgt.tgt_ops->put_sess(sess);
+	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
 
 out_term:
+	spin_lock_irqsave(&ha->hardware_lock, flags);
 	qlt_24xx_send_abts_resp(vha, &prm->abts, FCP_TMF_REJECTED, false);
 	spin_unlock_irqrestore(&ha->hardware_lock, flags);
-
-	if (sess)
-		ha->tgt.tgt_ops->put_sess(sess);
-	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
 }
 
 static void qlt_tmr_work(struct qla_tgt *tgt,
@@ -5878,7 +5871,7 @@ static void qlt_tmr_work(struct qla_tgt *tgt,
 	spin_lock_irqsave(&ha->tgt.sess_lock, flags);
 
 	if (tgt->tgt_stop)
-		goto out_term;
+		goto out_term2;
 
 	s_id = prm->tm_iocb2.u.isp24.fcp_hdr.s_id;
 	sess = ha->tgt.tgt_ops->find_sess_by_s_id(vha, s_id);
@@ -5890,11 +5883,11 @@ static void qlt_tmr_work(struct qla_tgt *tgt,
 
 		spin_lock_irqsave(&ha->tgt.sess_lock, flags);
 		if (!sess)
-			goto out_term;
+			goto out_term2;
 	} else {
 		if (sess->deleted) {
 			sess = NULL;
-			goto out_term;
+			goto out_term2;
 		}
 
 		if (!kref_get_unless_zero(&sess->sess_kref)) {
@@ -5902,7 +5895,7 @@ static void qlt_tmr_work(struct qla_tgt *tgt,
 			    "%s: kref_get fail %8phC\n",
 			     __func__, sess->port_name);
 			sess = NULL;
-			goto out_term;
+			goto out_term2;
 		}
 	}
 
@@ -5912,17 +5905,19 @@ static void qlt_tmr_work(struct qla_tgt *tgt,
 	unpacked_lun = scsilun_to_int((struct scsi_lun *)&lun);
 
 	rc = qlt_issue_task_mgmt(sess, unpacked_lun, fn, iocb, 0);
-	if (rc != 0)
-		goto out_term;
-
 	ha->tgt.tgt_ops->put_sess(sess);
 	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
+
+	if (rc != 0)
+		goto out_term;
 	return;
 
+out_term2:
+	if (sess)
+		ha->tgt.tgt_ops->put_sess(sess);
+	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
 out_term:
 	qlt_send_term_exchange(vha, NULL, &prm->tm_iocb2, 1, 0);
-	ha->tgt.tgt_ops->put_sess(sess);
-	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
 }
 
 static void qlt_sess_work_fn(struct work_struct *work)
-- 
1.8.3.1

  parent reply	other threads:[~2017-02-02 19:44 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-02 19:42 [PATCH 00/15] qla2xxx: Bug Fixes and updates for target Himanshu Madhani
2017-02-02 19:42 ` [PATCH 01/15] qla2xxx: Fix delayed response to command for loop mode/direct connect Himanshu Madhani
2017-02-02 19:42 ` [PATCH 02/15] qla2xxx: Allow relogin to proceed if remote login did not finish Himanshu Madhani
2017-02-02 19:42 ` [PATCH 03/15] qla2xxx: Allow vref count to timeout on vport delete Himanshu Madhani
2017-02-02 19:42 ` [PATCH 04/15] qla2xxx: Use IOCB interface to submit non-critical MBX Himanshu Madhani
2017-02-02 19:42 ` [PATCH 05/15] qla2xxx: Add DebugFS node to display Port Database Himanshu Madhani
2017-02-02 19:42 ` [PATCH 06/15] qla2xxx: Improve T10-DIF/PI handling in driver Himanshu Madhani
2017-02-02 19:42 ` [PATCH 07/15] qla2xxx: Export DIF stats via debugfs Himanshu Madhani
2017-02-02 19:42 ` [PATCH 08/15] qla2xxx: Change scsi host lookup method Himanshu Madhani
2017-02-02 19:42 ` [PATCH 09/15] qla2xxx: Fix memory leak for abts processing Himanshu Madhani
2017-02-02 19:42 ` [PATCH 10/15] qla2xxx: Fix request queue corruption Himanshu Madhani
2017-02-02 19:42 ` [PATCH 11/15] qla2xxx: Fix inadequate lock protection for ABTS Himanshu Madhani
2017-02-02 19:42 ` [PATCH 12/15] qla2xxx: Fix warnings about inconsistent indenting Himanshu Madhani
2017-02-02 19:42 ` [PATCH 13/15] qla2xxx: Add async new target notification Himanshu Madhani
2017-02-02 19:42 ` Himanshu Madhani [this message]
2017-02-02 19:42 ` [PATCH 15/15] qla2xxx: Update driver version to 9.00.00.00-k Himanshu Madhani
2017-02-03 16:26 ` [PATCH 00/15] qla2xxx: Bug Fixes and updates for target Bart Van Assche
2017-02-03 17:25   ` Madhani, Himanshu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1486064558-365-15-git-send-email-himanshu.madhani@cavium.com \
    --to=himanshu.madhani@cavium.com \
    --cc=Bart.VanAssche@sandisk.com \
    --cc=giridhar.malavali@cavium.com \
    --cc=hch@infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=nab@linux-iscsi.org \
    --cc=target-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.