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: > > On Wed, Feb 17, 2021 at 12:02:39PM -0600, Bjorn Helgaas wrote: > > > > > > 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
