lkml.org 
[lkml]   [2009]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 08/12] PCI: Introduce /sys/bus/pci/devices/.../remove
* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> Alex Chiang wrote:
>> * Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
>>> Alex Chiang wrote:
>>>> +
>>>> +static void remove_callback(struct device *dev)
>>>> +{
>>>> + int bridge = 0;
>>>> + struct pci_dev *pdev = to_pci_dev(dev);
>>>> +
>>>> + mutex_lock(&pci_remove_rescan_mutex);
>>>> +
>>>> + if (pdev->subordinate)
>>>> + bridge = 1;
>>>> +
>>>> + pci_remove_bus_device(pdev);
>>>> + if (bridge && list_empty(&pdev->bus->devices))
>>>> + pci_remove_bus(pdev->bus);
>>> I cannot understand the above two lines. Could you explain
>>> what it intend?
>>
>> If the user says:
>>
>> echo 1 > /sys/bus/pci/devices/.../remove
>>
>> And that device is a bridge, then we need to specifically call
>> pci_remove_bus as well, to actually remove the bus itself.
>> Without it, pci_bus_remove_device() will remove all of its
>> children (and subordinate buses) in a depth-first manner, but we
>> will never actually remove the bus that the user specified.
>>
>
> Do you mean user removes bridge device to remove its *primary*
> bus? It is very strange. I think the bus should be removed
> when its parent bridge is removed.

You are correct.

>> In other words, without it, we will still see the bus in:
>>
>> /sys/class/pci_bus/...
>>
>
> What is the problem?
>
>> We only want to remove the bus if it has no children left. I
>> think the check for list_empty(&pdev->bus->devices) might be
>> overkill... I can try taking that bit out and testing again.
>>
>
> I think we don't need the two lines. But if you do that, you
> need list_empty(&pdev->bus->devices), doesn't it? On the other
> hand, we must not check 'bridge' in the if statement. Or bus
> will never be removed when non-bridge device is removed last
> on the bus.
>
> Again, I think we don't need the two lines. But am I
> misunderstanding something?

No, you are correct.

I think what was happening was that I inserted that code before I
discovered the double-free in the PCIe port driver, and that
extra call to pci_remove_bus() helped mask the double-free.

I re-tested again tonight with the port driver fix, and also
removing the two lines you mention, and it is behaving correctly.

As always, thanks for your review.

/ac



\
 
 \ /
  Last update: 2009-03-20 04:57    [W:0.137 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site