Re: [PATCH] pci/iov: return a reference to PF on destroying VF

[Wei Yang <weiyang@linux.vnet.ibm.com>]

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@linux.vnet.ibm.com> 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