On 01/06/2021 14:11, Ming Lei wrote:
We don't call scsi_host_cls_release() either, so I guess a ref count is
leaked for shost_dev - I see its refcount is 1 at exit in
scsi_add_host_with_dma(). We have the device_initialize(), device_add(),
device_del() in the alloc and add host functions, but I don't know who is
responsible for the final "device put".
Hammm, we still need to put ->shost_dev before returning the error, and the
following delta patch can fix the issue, and it should have been wrapped
into the 1st one.
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 22a58e453a0c..532165462a42 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -306,6 +306,8 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
pm_runtime_set_suspended(&shost->shost_gendev);
pm_runtime_put_noidle(&shost->shost_gendev);
fail:
+ /* drop ref of ->shost_dev so that caller can release this host */
+ put_device(&shost->shost_dev);
return error;
}
EXPORT_SYMBOL(scsi_add_host_with_dma);
That looks better now.
And we can see the equivalent on the normal removal path in
scsi_remove_host() -> device_unregister(&shost->shost_dev), which does a
device_del()+put_device().
So could we actually just have:
out_del_dev:
unregister_dev(&shost->shost_dev)
I am not sure if we are required to keep that shost_dev reference all
the way until the exit, as you do.
Thanks,
John
