Re: [PATCH 1/2] powerpc/powernv: clear the refcount for pci_dev on powernv platform

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

On Mon, Apr 21, 2014 at 01:35:34PM +1000, Benjamin Herrenschmidt wrote:
>On Mon, 2014-04-21 at 10:25 +0800, Wei Yang wrote:
>> When pcibios_remove_pci_devices() is removing pci devices, it will release
>> pci device respectively. When the refcount of the device is 0, the pci_dev
>> structure will be destroyed.
>> 
>> On PowerNV platform, the pci_dev will not be destroyed since the refcount is
>> not 0.
>> 
>> After applying the patch, this warning is cleared during the EEH hotplug
>> event.
>
>You have to be careful here. We take a reference to the device in the
>structure eeh_dev, that means we might access it after it's freed if
>we don't increase the refcount.

Ben,

Thanks for reminding.

Hmm, I checked the eeh_dev structure, there is a field pdev in eeh_dev, which
is not a ref to the device.

Then I did a check in eeh hotplug code patch. The pci_dev remove/install
happens in eeh_reset_device(), between pcibios_remove_pci_devices() and
pcibios_add_pci_devices(). During this period, we will try to clear/set the PE
state and restore the BAR of a pci device. But the BAR is restored through the
device node instead of pci_dev structure. As my understanding the eeh code
here has the assumption that these pci devices in the same PE are removed.
Maybe I missed something, needs confirmation from Gavin.

The code I removed in this patch is introduced in commit
184cd4a3(powerpc/powernv: PCI support for p7IOC under OPAL v2).
Sounds the original purpose is to make sure a pci device is covered by a Bus
PE, so don't remove it. After booting up, Bus PE will always be there, which
means we will always hold a refcount to a pci device.

I didn't come up with a better idea, or leaving the warning is fine.

Any comments are welcome :)

>
>Cheers,
>Ben.
>