Hello, Bart.

On Wed, Nov 28, 2012 at 01:52:51PM +0100, Bart Van Assche wrote:
> A SCSI LLD may start cleaning up host resources as soon as
> scsi_remove_host() returns. These host resources may be needed by
> the LLD in an implementation of one of the eh_* functions. So if
> one of the eh_* functions is in progress when scsi_remove_host()
> is invoked, wait until the eh_* function has finished. Also, do
> not invoke any of the eh_* functions after scsi_remove_host() has
> started.

Nice catch.

>  /**
> + * scsi_begin_eh - start host-related error handling
> + *
> + * Must be called before invoking any of the scsi_host_template.eh_* 
> functions
> + * to avoid that scsi_remove_host() returns while one of these callback
> + * functions is in progress.
> + *
> + * Returns true if invoking the eh_* function is allowed and false if not.
> + * If this function returns true then scsi_end_eh() must be called 
> eventually.
> + *
> + * Note: scsi_send_eh_cmnd() calls do not have to be protected by a
> + * scsi_begin_eh() / scsi_end_eh() pair since these operate on an unfinished
> + * block layer request. Since scsi_remove_host() waits until all SCSI devices
> + * have been removed and since blk_cleanup_queue() is invoked during SCSI
> + * device removal scsi_remove_host() won't finish while a scsi_send_eh_cmnd()
> + * call is in progress.
> + */
> +static bool scsi_begin_eh(struct Scsi_Host *host)
> +{
> +     bool res;
> +
> +     spin_lock_irq(host->host_lock);
> +     res = scsi_host_scan_allowed(host);
> +     if (res) {
> +             WARN_ON_ONCE(host->eh_active < 0 || host->eh_active > 1);
> +             host->eh_active++;
> +     }
> +     spin_unlock_irq(host->host_lock);
> +
> +     return res;
> +}

No biggie but I find it somewhat unusual to use bool success/failure
return for a function named scsi_begin_eh().  0/-errno would be more
conventional.  People do use bool return for get/acquite type
operations but it's a bit unusual to do that for begin().  Plus, the
code would actually be simpler with -errno failure, right?

        if (scsi_begin_eh(host))
                return FAST_IO_FAIL;

        /* no change */

        scsi_end_eh(host);
        return rtn;

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to