On Sun, 18 Sep 2005, James Bottomley wrote:
> void scsi_remove_host(struct Scsi_Host *shost)
> {
> + unsigned long flags;
> down(&shost->scan_mutex);
> - scsi_host_set_state(shost, SHOST_CANCEL);
> + spin_lock_irqsave(shost->host_lock, flags);
> + if (scsi_host_set_state(shost, SHOST_CANCEL))
> + if (scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY)) {
> + spin_unlock_irqrestore(shost->host_lock, flags);
> + up(&shost->scan_mutex);
> + return;
> + }
> + spin_unlock_irqrestore(shost->host_lock, flags);
> up(&shost->scan_mutex);
> scsi_forget_host(shost);
Personally I would prefer to see something like:
if (scsi_host_set_state(shost,
(shost->shost_state == SHOST_RECOVERY) ?
SHOST_CANCEL_RECOVERY : SHOST_CANCEL)) {
spin_unlock_irqrestore...
with similar changes in the other places. It avoids a function call and
prevents misleading error messages from showing up in the system log.
Actually, I would probably do
unsigned long flags;
int rc;
down(&shost->scan_mutex);
spin_lock_irqsave(shost->host_lock, flags);
rc = scsi_host_set_state(shost,
(shost->shost_state == SHOST_RECOVERY) ?
SHOST_CANCEL_RECOVERY : SHOST_CANCEL);
spin_unlock_irqrestore(shost->host_lock, flags);
up(&shost->scan_mutex);
if (rc)
return;
scsi_forget_host(shost);
...
just to avoid the extra spin_unlock call. Those macros can expand to a
surprising length.
Alan Stern
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
