Re: [PATCH] PCI: Fix device node for virtual bus

[Gavin Shan <gwshan@linux.vnet.ibm.com>]

On Wed, Nov 12, 2014 at 12:02:08PM -0700, Bjorn Helgaas wrote:
>On Tue, Nov 11, 2014 at 10:24 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>> On Tue, Nov 11, 2014 at 09:08:23PM -0700, Bjorn Helgaas wrote:
>>>On Tue, Nov 11, 2014 at 8:11 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>>>> On Tue, Nov 11, 2014 at 07:55:19PM -0700, Bjorn Helgaas wrote:
>>>>>On Tue, Nov 11, 2014 at 7:42 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>>>>>> pci_set_bus_of_node() sets virtual PCI bus's device node to PHB's
>>>>>> device node wrongly. The patch fixes the issue.
>>>>>
>>>>>This needs more detail.  How is this bug visible to users?  Is there a
>>>>>bug report?  Is this a regression?  Should it be marked for stable?
>>>>>
>>>>
>>>> I don't have opened bug for it. I just found the problem (maybe not
>>>> a problem) when reading the code: The original implementation binds
>>>> PHB device-tree node with "virtual bus", which doesn't make sense.
>>>
>>>Does this result in something being wrong in sysfs?  How is this bug
>>>visible to users?
>>>
>>
>> No, I don't think it caused anything wrong in sysfs. So I'm not sure
>> it's a real problem and need the fix. Please judge.
>
>Well, that's not really the way I work.  If somebody proposes a patch,
>I expect the changelog to be an argument for why the patch is needed.
>I'm not disagreeing with your patch; I'm just trying to get you to
>provide the justification for it.  I'm not an OF expert, so it's not
>obvious to me.  I probably could spend some time researching it and
>convince myself, but I would rather have *you* convince me :)
>

Thank you for the detailed explaining. I'm trying to convince you.
Please drop it in case I fail :)

Basically, we have 3 types of buses with SR-IOV case inclusive:
(A) root bus leading from PHB; (B) normal bus leading from PCI
bridge; (C) virtual bus to hold SR-IOV VFs. A's device node is set
to PHB's device node and B's device node is equal to parent PCI
bridge's device node. So the device node of PCI bus is set that
one of parent device (either PHB or PCI bridge for A/B). For (C),
the bus's device node would be NULL, which is consistent with that
its parent device is NULL.

Kernel code can convert PCI bus to its corresponding device node
with function pci_bus_to_OF_node(). The code is usually unware
what type (PHB, PCI bridge or device) the device node is. With
current code, we poke the PHB device node for SR-IOV virtual buses.
and potentially wrong properties retrieved. I personally don't
think it's appropriate and it's worthy to have NULL device nodes
for those virtual buses.

>>>> Yes, I guess it should be marked for stable if you don't object.
>
>Once we know what the user-visible effect of this change is, it will
>probably be obvious whether it should be marked for stable.  If it
>doesn't really fix anything, it probably should not go to stable.
>

Yep, it's not good candidate for stable.

Thanks,
Gavin