James Bottomley <james.bottom...@hansenpartnership.com> writes: > On Thu, 2015-01-22 at 08:50 -0800, Christoph Hellwig wrote: >> We'll also still need to change scsi_device_put to deal with >> a negative refcount.. > > I don't believe so ... we never call module_refcount() now, and the > actual module ref count never goes negative. That's the point of > Rusty's change. In the unload routines, the actual module refcount is > zero. __module_get() will increment this and the final module_put() > will decrement it without triggering a warning. > > In the old code, we expected try_module_get() to fail but then used > module_refcount() to detect the failure and not do a put.
Yep, I've put that patch in my fixes queue now. Thanks, Rusty. From: Rusty Russell <ru...@rustcorp.com.au> Subject: scsi: always increment reference count James reported: > After e513cc1 module: Remove stop_machine from module unloading, > module_refcount() is returning (unsigned long)-1 when called from within > a routine that runs in module_exit. This is confusing the scsi device > put code which is coded to detect a module_refcount() of zero for > running within a module exit routine and not try to do another > module_put. The fix is to restore the original behaviour of > module_refcount() and return zero if we're running inside an exit > routine. The correct fix is to turn try_module_get() into __module_get(), and always do the module_put(). Acked-by: James Bottomley <james.bottom...@hansenpartnership.com> Signed-off-by: Rusty Russell <ru...@rustcorp.com.au> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index e02885451425..9b3829931f40 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -986,9 +986,9 @@ int scsi_device_get(struct scsi_device *sdev) return -ENXIO; if (!get_device(&sdev->sdev_gendev)) return -ENXIO; - /* We can fail this if we're doing SCSI operations + /* We can fail try_module_get if we're doing SCSI operations * from module exit (like cache flush) */ - try_module_get(sdev->host->hostt->module); + __module_get(sdev->host->hostt->module); return 0; } @@ -1004,14 +1004,7 @@ EXPORT_SYMBOL(scsi_device_get); */ void scsi_device_put(struct scsi_device *sdev) { -#ifdef CONFIG_MODULE_UNLOAD - struct module *module = sdev->host->hostt->module; - - /* The module refcount will be zero if scsi_device_get() - * was called from a module removal routine */ - if (module && module_refcount(module) != 0) - module_put(module); -#endif + module_put(sdev->host->hostt->module); put_device(&sdev->sdev_gendev); } EXPORT_SYMBOL(scsi_device_put); -- 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