On Wed, Feb 17, 2021 at 02:28:35PM -0600, Bjorn Helgaas wrote:

On Wed, Feb 17, 2021 at 03:25:22PM -0400, Jason Gunthorpe wrote:

quoted

On Wed, Feb 17, 2021 at 12:02:39PM -0600, Bjorn Helgaas wrote:

quoted

quoted

BTW, I asked more than once how these sysfs knobs should be handled
in the PCI/core.

Thanks for the pointers.  This is the first instance I can think of
where we want to create PCI core sysfs files based on a driver
binding, so there really isn't a precedent.

The MSI stuff does it today, doesn't it? eg:

virtblk_probe (this is a driver bind)
  init_vq
   virtio_find_vqs
    vp_modern_find_vqs
     vp_find_vqs
      vp_find_vqs_msix
       vp_request_msix_vectors
        pci_alloc_irq_vectors_affinity
         __pci_enable_msi_range
          msi_capability_init
	   populate_msi_sysfs
	    	ret = sysfs_create_groups(&pdev->dev.kobj, msi_irq_groups);

And the sysfs is removed during pci_disable_msi(), also called by the
driver

Yes, you're right, I didn't notice that one.

I'm not quite convinced that we clean up correctly in all cases --
pci_disable_msix(), pci_disable_msi(), pci_free_irq_vectors(),
pcim_release(), etc are called by several drivers, but in my quick
look I didn't see a guaranteed-to-be-called path to the cleanup during
driver unbind.  I probably just missed it.

 
I think the contract is the driver has to pair the msi enable with the
msi disable on its own? It is very similar to what is happening here.

Probably there are bugs in drivers on error paths, but there are
always bugs in drivers on error paths..

Jason