Re: [PATCH V2] scsi: core: put LLD module refcnt after SCSI device is released

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Sep 30, 2021 at 03:40:26PM +0800, Ming Lei wrote:
> SCSI host release is triggered when SCSI device is freed, and we have to
> make sure that LLD module won't be unloaded before SCSI host instance is
> released because shost->hostt is required in host release handler.
> 
> So put LLD module refcnt after SCSI device is released.
> 
> The real release handler can be run from wq context in case of
> in_interrupt(), so add one atomic counter for serializing putting
> module via current and wq context. This way is fine since we don't
> call scsi_device_put() in fast IO path.
> 
> Reported-by: Changhui Zhong <czhong@redhat.com>
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/scsi/scsi.c        |  8 +++++++-
>  drivers/scsi/scsi_sysfs.c  | 10 ++++++++++
>  include/scsi/scsi_device.h |  2 ++
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index b241f9e3885c..b6612161587f 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -553,8 +553,14 @@ EXPORT_SYMBOL(scsi_device_get);
>   */
>  void scsi_device_put(struct scsi_device *sdev)
>  {
> -	module_put(sdev->host->hostt->module);
> +	struct module *mod = sdev->host->hostt->module;
> +
> +	atomic_inc(&sdev->put_dev_cnt);

Ick, no!  Why are you making a new lock and reference count for no
reason?

> +
>  	put_device(&sdev->sdev_gendev);
> +
> +	if (atomic_dec_if_positive(&sdev->put_dev_cnt) >= 0)
> +		module_put(mod);

How do you know if your module pointer is still valid here?

Why do you care?

What problem are you trying to solve and why is it unique to scsi
devices?

thanks,

greg k-h