On 2020/3/26 下午8:17, Jason Gunthorpe wrote:

On Thu, Mar 26, 2020 at 01:50:53PM +0800, Jason Wang wrote:

quoted

quoted

quoted

+	adapter->vdpa.dma_dev = dev;
+	ret = vdpa_register_device(&adapter->vdpa);
+	if (ret) {
+		IFCVF_ERR(adapter->dev, "Failed to register ifcvf to vdpa bus");
+		goto err_msix;
+	}
+
+	return 0;
+
+err_msix:
+	put_device(&adapter->vdpa.dev);
+	return ret;
+err_alloc:
+	pci_free_irq_vectors(pdev);
+err_vectors:
+	pci_release_regions(pdev);
+err_regions:
+	pci_disable_device(pdev);
+err_enable:
+	return ret;
+}

I personally don't like seeing goto unwinds with multiple returns, and
here I think it is actually a tiny bug.

All touches to the PCI device must stop before the driver core
remove() returns - so these pci function cannot be in the kref put
release function anyhow.

I'm not sure I get here. IFCVF held refcnt of its PCI parent, so it looks to
me it's safe to free PCI resources in vDPA free callback?

The refcnt doesn't prevent the driver core from re-binding the
pci_device to another driver. Then the refcount put would do a
pci_disable_device() after another driver has started

For this reason all touches to a struct pci_device must stop before
remove returns.

Jason

Ok, will send a new version shortly.

Thanks