Commit 8798a694 authored by Michael Reed's avatar Michael Reed Committed by James Bottomley

[SCSI] scsi_transport_fc: remove invalid BUG_ON

I was doing some large lun count testing with 2.6.31 and hit
a BUG_ON() in fc_timeout_deleted_rport(), and it seems like it
should have been just a matter of time before someone did.

It seems invalid to set port_state under lock, then expect it to
remain set after releasing the lock.  Another thread called
fc_remote_port_add() when the lock was released, changing the
port_state.

This patch removes the BUG_ON and moves the test of the
port_state to inside the host_lock.  It's been running for
several weeks now with no ill effect.
Signed-off-by: default avatarMichael Reed <mdr@sgi.com>
Acked-by: default avatarJames Smart <james.smart@emulex.com>
Signed-off-by: default avatarJames Bottomley <James.Bottomley@suse.de>
parent ad630826
...@@ -2384,6 +2384,7 @@ fc_rport_final_delete(struct work_struct *work) ...@@ -2384,6 +2384,7 @@ fc_rport_final_delete(struct work_struct *work)
struct Scsi_Host *shost = rport_to_shost(rport); struct Scsi_Host *shost = rport_to_shost(rport);
struct fc_internal *i = to_fc_internal(shost->transportt); struct fc_internal *i = to_fc_internal(shost->transportt);
unsigned long flags; unsigned long flags;
int do_callback = 0;
/* /*
* if a scan is pending, flush the SCSI Host work_q so that * if a scan is pending, flush the SCSI Host work_q so that
...@@ -2422,8 +2423,15 @@ fc_rport_final_delete(struct work_struct *work) ...@@ -2422,8 +2423,15 @@ fc_rport_final_delete(struct work_struct *work)
* Avoid this call if we already called it when we preserved the * Avoid this call if we already called it when we preserved the
* rport for the binding. * rport for the binding.
*/ */
spin_lock_irqsave(shost->host_lock, flags);
if (!(rport->flags & FC_RPORT_DEVLOSS_CALLBK_DONE) && if (!(rport->flags & FC_RPORT_DEVLOSS_CALLBK_DONE) &&
(i->f->dev_loss_tmo_callbk)) (i->f->dev_loss_tmo_callbk)) {
rport->flags |= FC_RPORT_DEVLOSS_CALLBK_DONE;
do_callback = 1;
}
spin_unlock_irqrestore(shost->host_lock, flags);
if (do_callback)
i->f->dev_loss_tmo_callbk(rport); i->f->dev_loss_tmo_callbk(rport);
fc_bsg_remove(rport->rqst_q); fc_bsg_remove(rport->rqst_q);
...@@ -2970,6 +2978,7 @@ fc_timeout_deleted_rport(struct work_struct *work) ...@@ -2970,6 +2978,7 @@ fc_timeout_deleted_rport(struct work_struct *work)
struct fc_internal *i = to_fc_internal(shost->transportt); struct fc_internal *i = to_fc_internal(shost->transportt);
struct fc_host_attrs *fc_host = shost_to_fc_host(shost); struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
unsigned long flags; unsigned long flags;
int do_callback = 0;
spin_lock_irqsave(shost->host_lock, flags); spin_lock_irqsave(shost->host_lock, flags);
...@@ -3035,7 +3044,6 @@ fc_timeout_deleted_rport(struct work_struct *work) ...@@ -3035,7 +3044,6 @@ fc_timeout_deleted_rport(struct work_struct *work)
rport->roles = FC_PORT_ROLE_UNKNOWN; rport->roles = FC_PORT_ROLE_UNKNOWN;
rport->port_state = FC_PORTSTATE_NOTPRESENT; rport->port_state = FC_PORTSTATE_NOTPRESENT;
rport->flags &= ~FC_RPORT_FAST_FAIL_TIMEDOUT; rport->flags &= ~FC_RPORT_FAST_FAIL_TIMEDOUT;
rport->flags |= FC_RPORT_DEVLOSS_CALLBK_DONE;
/* /*
* Pre-emptively kill I/O rather than waiting for the work queue * Pre-emptively kill I/O rather than waiting for the work queue
...@@ -3045,7 +3053,9 @@ fc_timeout_deleted_rport(struct work_struct *work) ...@@ -3045,7 +3053,9 @@ fc_timeout_deleted_rport(struct work_struct *work)
spin_unlock_irqrestore(shost->host_lock, flags); spin_unlock_irqrestore(shost->host_lock, flags);
fc_terminate_rport_io(rport); fc_terminate_rport_io(rport);
BUG_ON(rport->port_state != FC_PORTSTATE_NOTPRESENT); spin_lock_irqsave(shost->host_lock, flags);
if (rport->port_state == FC_PORTSTATE_NOTPRESENT) { /* still missing */
/* remove the identifiers that aren't used in the consisting binding */ /* remove the identifiers that aren't used in the consisting binding */
switch (fc_host->tgtid_bind_type) { switch (fc_host->tgtid_bind_type) {
...@@ -3070,15 +3080,21 @@ fc_timeout_deleted_rport(struct work_struct *work) ...@@ -3070,15 +3080,21 @@ fc_timeout_deleted_rport(struct work_struct *work)
* went away and didn't come back - we'll remove * went away and didn't come back - we'll remove
* all attached scsi devices. * all attached scsi devices.
*/ */
rport->flags |= FC_RPORT_DEVLOSS_CALLBK_DONE;
fc_queue_work(shost, &rport->stgt_delete_work); fc_queue_work(shost, &rport->stgt_delete_work);
do_callback = 1;
}
spin_unlock_irqrestore(shost->host_lock, flags);
/* /*
* Notify the driver that the rport is now dead. The LLDD will * Notify the driver that the rport is now dead. The LLDD will
* also guarantee that any communication to the rport is terminated * also guarantee that any communication to the rport is terminated
* *
* Note: we set the CALLBK_DONE flag above to correspond * Note: we set the CALLBK_DONE flag above to correspond
*/ */
if (i->f->dev_loss_tmo_callbk) if (do_callback && i->f->dev_loss_tmo_callbk)
i->f->dev_loss_tmo_callbk(rport); i->f->dev_loss_tmo_callbk(rport);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment