On Wed, May 06, 2015 at 10:30:39AM -0500, Bjorn Helgaas wrote: >On Wed, May 6, 2015 at 1:09 AM, Wei Yang <weiyang@xxxxxxxxxxxxxxxxxx> wrote: >> On Tue, May 05, 2015 at 04:29:05PM -0500, Bjorn Helgaas wrote: > >>>Prior to your patch, the VF reference was released in virtfn_remove(), >>>which is only called via pci_disable_sriov(). This typically happens in >>>a driver .remove() method. The reference is *not* released if we call >>>pci_stop_and_remove_bus_device(VF) directly, as we would via the >>>remove_store() (sysfs "remove" file) or hot unplug paths, e.g., >>>pciehp_unconfigure_device(). >> >> You want to say the reference for VF or PF? > >Yes, I meant the PF reference. > >The hot unplug paths call pci_stop_and_remove_bus_device() for the VFs >first, then the PF. Calling it for the VF releases the VF reference >but not the PF one. Calling it for the PF will call virtfn_remove() >via the driver's .remove() method, but it probably does nothing >because when it calls pci_get_domain_bus_and_slot() to get the virtfn, >it gets a NULL because the VF has already been removed. > >>>What about the other things done in virtfn_remove(), e.g., the sysfs link >>>removal? Your patch fixes a reference count leak, but don't we still have >>>a sysfs link leak? >> >> Agree, I am afraid the sysfs would have a leak too. >> >> While I am not that familiar with the sysfs part, I don't dare to move that to >> pci_destroy_dev(). Need more investigation. > >OK. I don't want to fix half of the leak problem. I want to fix the >whole thing at once. > Sure, I will do some investigation and repost it. BTW, seems we also face the leak of the virtual bus. Will fix it in next version too. >Bjorn -- Richard Yang Help you, Help me -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
