On Wed, 2017-02-22 at 17:07 +0100, Hannes Reinecke wrote:
>       spin_lock_irqsave(shost->host_lock, flags);
> +     WARN_ON(shost->shost_state != SHOST_RUNNING &&
> +             shost->shost_state != SHOST_CANCEL &&
> +             shost->shost_state != SHOST_RECOVERY &&
> +             shost->shost_state != SHOST_CANCEL_RECOVERY);
>       if (scsi_host_set_state(shost, SHOST_RECOVERY))
> -             if (scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY))
> -                     goto out_unlock;
> +             scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY);

Please issue a warning if the second scsi_host_set_state() fails. And once
that failure triggers a warning, I don't think we need the newly added
WARN_ON() statement anymore. Something else that surprised me is that you
consistently use WARN_ON() instead of WARN_ON_ONCE() in this patch?
  
Otherwise this patch looks fine to me.

Bart.

Reply via email to