Skip to content

Commit

Permalink
qla2xxx: Fix sess_lock & hardware_lock lock order problem.
Browse files Browse the repository at this point in the history
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: <[email protected]>
Cc: Bart Van Assche <[email protected]>
Reported-by: Bart Van Assche <[email protected]>
Signed-off-by: Quinn Tran <[email protected]>
Signed-off-by: Himanshu Madhani <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
  • Loading branch information
Quinn Tran authored and Nicholas Bellinger committed Mar 19, 2017
1 parent 8f6fc8d commit f159b3c
Showing 1 changed file with 18 additions and 23 deletions.
41 changes: 18 additions & 23 deletions drivers/scsi/qla2xxx/qla_target.c
Original file line number Diff line number Diff line change
Expand Up @@ -5727,30 +5727,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,
Expand All @@ -5770,7 +5763,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);
Expand All @@ -5782,19 +5775,19 @@ 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)) {
ql_dbg(ql_dbg_tgt_tmr, vha, 0xffff,
"%s: kref_get fail %8phC\n",
__func__, sess->port_name);
sess = NULL;
goto out_term;
goto out_term2;
}
}

Expand All @@ -5804,17 +5797,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)
Expand Down

0 comments on commit f159b3c

Please sign in to comment.