On 10/01/2010 07:18 AM, Hannes Reinecke wrote:
This patch fixes a regression introduced by commit
083a469db4ecf3b286a96b5b722c37fc1affe0be
qla2xxx_eh_wait_on_command() is waiting for an srb to
complete, which will never happen as the routine took
a reference to the srb previously and will only drop it
after this function. So every command abort will fail.
Signed-off-by: Hannes Reinecke<hare@xxxxxxx>
Cc: Andrew Vasquez<andrew.vasquez@xxxxxxxxxx>
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 1e4bff6..0af7549 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -883,6 +883,9 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
}
spin_unlock_irqrestore(&ha->hardware_lock, flags);
+ if (got_ref)
+ qla2x00_sp_compl(ha, sp);
+
You can just get rid of got_ref, because if you just move the compl call
up a little more you know that in that code path we always get a ref.
And there is no need to hold the ref to where it is above. See attached
patch.
We did something similar for qla4xxx. Looks like qla4xxx has a race when
it checks the CMD_SP validity and grabs a ref, but that is different story.
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index bdd53f0..6ea314f 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -832,7 +832,6 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
struct qla_hw_data *ha = vha->hw;
struct req_que *req = vha->req;
srb_t *spt;
- int got_ref = 0;
fc_block_scsi_eh(cmd);
@@ -866,7 +865,6 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
/* Get a reference to the sp and drop the lock.*/
sp_get(sp);
- got_ref++;
spin_unlock_irqrestore(&ha->hardware_lock, flags);
if (ha->isp_ops->abort_command(sp)) {
@@ -879,6 +877,7 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
wait = 1;
}
spin_lock_irqsave(&ha->hardware_lock, flags);
+ qla2x00_sp_compl(ha, sp);
break;
}
spin_unlock_irqrestore(&ha->hardware_lock, flags);
@@ -893,9 +892,6 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
}
}
- if (got_ref)
- qla2x00_sp_compl(ha, sp);
-
qla_printk(KERN_INFO, ha,
"scsi(%ld:%d:%d): Abort command issued -- %d %lx %x.\n",
vha->host_no, id, lun, wait, serial, ret);
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index acfc7d9..7921d76 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -470,7 +470,6 @@ static int iscsi_user_scan_session(struct device *dev, void *data)
struct Scsi_Host *shost;
struct iscsi_cls_host *ihost;
unsigned long flags;
- unsigned int id;
if (!iscsi_is_session_dev(dev))
return 0;
@@ -488,15 +487,14 @@ static int iscsi_user_scan_session(struct device *dev, void *data)
spin_unlock_irqrestore(&session->lock, flags);
goto user_scan_exit;
}
- id = session->target_id;
spin_unlock_irqrestore(&session->lock, flags);
- if (id != ISCSI_MAX_TARGET) {
+ if (session->target_id != ISCSI_MAX_TARGET) {
if ((scan_data->channel == SCAN_WILD_CARD ||
scan_data->channel == 0) &&
(scan_data->id == SCAN_WILD_CARD ||
- scan_data->id == id))
- scsi_scan_target(&session->dev, 0, id,
+ scan_data->id == session->target_id))
+ scsi_scan_target(&session->dev, 0, session->target_id,
scan_data->lun, 1);
}
@@ -642,6 +640,7 @@ static void __iscsi_unblock_session(struct work_struct *work)
void iscsi_unblock_session(struct iscsi_cls_session *session)
{
queue_work(iscsi_eh_timer_workq, &session->unblock_work);
+ flush_work(&session->unblock_work);
}
EXPORT_SYMBOL_GPL(iscsi_unblock_session);
@@ -662,6 +661,10 @@ static void __iscsi_block_session(struct work_struct *work)
queue_delayed_work(iscsi_eh_timer_workq,
&session->recovery_work,
session->recovery_tmo * HZ);
+
+ queue_delayed_work(iscsi_eh_timer_workq,
+ &session->dev_loss_work,
+ session->recovery_tmo * HZ);
}
void iscsi_block_session(struct iscsi_cls_session *session)
@@ -1383,6 +1386,10 @@ iscsi_set_param(struct iscsi_transport *transport, struct iscsi_uevent *ev)
sscanf(data, "%d", &value);
session->recovery_tmo = value;
break;
+ case ISCSI_PARAM_SESS_DEV_LOSS_TMO:
+ sscanf(data, "%d", &value);
+ session->dev_loss_tmo = value;
+ break;
default:
err = transport->set_param(conn, ev->u.set_param.param,
data, ev->u.set_param.len);
@@ -1849,6 +1856,7 @@ static ISCSI_CLASS_ATTR(priv_sess, field, S_IRUGO | S_IWUGO, \
show_priv_session_##field, \
store_priv_session_##field)
iscsi_priv_session_rw_attr(recovery_tmo, "%d");
+iscsi_priv_session_rw_attr(dev_loss_tmo, "%d");
/*
* iSCSI host attrs
@@ -2071,6 +2079,7 @@ iscsi_register_transport(struct iscsi_transport *tt)
SETUP_SESSION_RD_ATTR(ifacename, ISCSI_IFACE_NAME);
SETUP_SESSION_RD_ATTR(initiatorname, ISCSI_INITIATOR_NAME);
SETUP_SESSION_RD_ATTR(targetalias, ISCSI_TARGET_ALIAS);
+ SETUP_PRIV_SESSION_RW_ATTR(dev_loss_tmo);
SETUP_PRIV_SESSION_RW_ATTR(recovery_tmo);
SETUP_PRIV_SESSION_RD_ATTR(state);
