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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
