| Message ID | 20210930052028.934747-3-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SCSI: fix race between releasing shost and unloading LLD module | expand |
On Thu, Sep 30, 2021 at 01:20:28PM +0800, Ming Lei wrote: > SCSI host release is triggered when SCSI device is released, and we have to > make sure that LLD module won't be unloaded before SCSI host instance is > released. > > So put LLD module refcnt after SCSI device is released. > > SCSI device release may be moved into workqueue context if scsi_device_put > is called in interrupt context, and handle this case by piggybacking > putting LLD module refcnt into SCSI device release handler. > > Reported-by: Changhui Zhong <czhong@redhat.com> > Reported-by: Yi Zhang <yi.zhang@redhat.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > drivers/scsi/scsi.c | 14 ++++++++++++-- > drivers/scsi/scsi_sysfs.c | 8 ++++++++ > include/scsi/scsi_device.h | 1 + > 3 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c > index b241f9e3885c..7cad256ba895 100644 > --- a/drivers/scsi/scsi.c > +++ b/drivers/scsi/scsi.c > @@ -553,8 +553,18 @@ EXPORT_SYMBOL(scsi_device_get); > */ > void scsi_device_put(struct scsi_device *sdev) > { > - module_put(sdev->host->hostt->module); > - put_device(&sdev->sdev_gendev); > + struct module *mod = sdev->host->hostt->module; > + /* > + * sdev->sdev_gendev's real release handler will be scheduled into > + * user context if we are in interrupt context, and we have to put > + * LLD module refcnt after the device is really released. > + */ > + preempt_disable(); > + if (put_device(&sdev->sdev_gendev) && in_interrupt()) Why does in_interrupt() matter here? And is this even set if you have threaded interrupts? This feels very wrong as you are doing something different if this is called depending on the context and you really do not have control over the context of when this is called at all. What problem is this solving? How is a host controller driver being unloaded before the children it controls are removed? Who is holding a reference on them and why is this happening only now? And who cares about unloading the kernel module in this fashion? thanks, greg k-h
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index b241f9e3885c..7cad256ba895 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -553,8 +553,18 @@ EXPORT_SYMBOL(scsi_device_get); */ void scsi_device_put(struct scsi_device *sdev) { - module_put(sdev->host->hostt->module); - put_device(&sdev->sdev_gendev); + struct module *mod = sdev->host->hostt->module; + /* + * sdev->sdev_gendev's real release handler will be scheduled into + * user context if we are in interrupt context, and we have to put + * LLD module refcnt after the device is really released. + */ + preempt_disable(); + if (put_device(&sdev->sdev_gendev) && in_interrupt()) + sdev->put_lld_mod_refcnt = 1; + else + module_put(mod); + preempt_enable(); } EXPORT_SYMBOL(scsi_device_put); diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 86793259e541..dc056ba5a656 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -449,9 +449,14 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) struct scsi_vpd *vpd_pg80 = NULL, *vpd_pg83 = NULL; struct scsi_vpd *vpd_pg0 = NULL, *vpd_pg89 = NULL; unsigned long flags; + struct module *lld_mod; + bool put_lld_mod_refcnt; sdev = container_of(work, struct scsi_device, ew.work); + lld_mod = sdev->host->hostt->module; + put_lld_mod_refcnt = sdev->put_lld_mod_refcnt; + scsi_dh_release_device(sdev); parent = sdev->sdev_gendev.parent; @@ -502,6 +507,9 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) if (parent) put_device(parent); + + if (put_lld_mod_refcnt) + module_put(lld_mod); } static void scsi_device_dev_release(struct device *dev) diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 430b73bd02ac..9d3fcb9cfd01 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -206,6 +206,7 @@ struct scsi_device { unsigned rpm_autosuspend:1; /* Enable runtime autosuspend at device * creation time */ unsigned ignore_media_change:1; /* Ignore MEDIA CHANGE on resume */ + unsigned put_lld_mod_refcnt:1; /* Put LLD mod refcnt */ bool offline_already; /* Device offline message logged */
SCSI host release is triggered when SCSI device is released, and we have to make sure that LLD module won't be unloaded before SCSI host instance is released. So put LLD module refcnt after SCSI device is released. SCSI device release may be moved into workqueue context if scsi_device_put is called in interrupt context, and handle this case by piggybacking putting LLD module refcnt into SCSI device release handler. Reported-by: Changhui Zhong <czhong@redhat.com> Reported-by: Yi Zhang <yi.zhang@redhat.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/scsi/scsi.c | 14 ++++++++++++-- drivers/scsi/scsi_sysfs.c | 8 ++++++++ include/scsi/scsi_device.h | 1 + 3 files changed, 21 insertions(+), 2 deletions(-)