On Thu, 2015-01-29 at 00:00 +0100, Christoph Hellwig wrote:
> This effectively reverts commits 85b6c7 ("[SCSI] sd: fix cache flushing on
> module removal (and individual device removal)" and dc4515ea ("scsi: always
> increment reference count").
> 
> We now never call scsi_device_get from the shutdown path, and the fact
> that we started grabbing reference there in commit 85b6c7 turned out
> turned out to create more problems than it solves, and required
> workarounds for workarounds for workarounds. Move back to properly checking
> the device state and carefully handle module refcounting.
> 
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
>  drivers/scsi/scsi.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 9b38299..95f0293 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -982,15 +982,18 @@ EXPORT_SYMBOL(scsi_report_opcode);
>   */
>  int scsi_device_get(struct scsi_device *sdev)
>  {
> -     if (sdev->sdev_state == SDEV_DEL)
> -             return -ENXIO;
> +     if (sdev->sdev_state == SDEV_DEL || sdev->sdev_state == SDEV_CANCEL)
> +             goto fail;
>       if (!get_device(&sdev->sdev_gendev))
> -             return -ENXIO;
> -     /* We can fail try_module_get if we're doing SCSI operations
> -      * from module exit (like cache flush) */
> -     __module_get(sdev->host->hostt->module);
> -
> +             goto fail;
> +     if (!try_module_get(sdev->host->hostt->module))
> +             goto fail_put_device;
>       return 0;
> +
> +fail_put_device:
> +     put_device(&sdev->sdev_gendev);
> +fail:
> +     return -ENXIO;
>  }
>  EXPORT_SYMBOL(scsi_device_get);

If we can get away with this, I'm all for this approach.  However, you
need to document in a comment or above the function that it may not be
called in module exit functions and why.

Other than the comment issue, the series looks good,

Thanks,

James

Reply via email to