Thread (14 messages) 14 messages, 3 authors, 2021-06-02

Re: [PATCH V3 0/3] scsi: two fixes in scsi_add_host_with_dma

From: John Garry <hidden>
Date: 2021-06-01 15:08:00

On 01/06/2021 14:11, Ming Lei wrote:
quoted hunk ↗ jump to hunk
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help