On Mon, Jul 8, 2013 at 8:38 PM, Wei Yang <weiyang@xxxxxxxxxxxxxxxxxx> wrote:
> On Mon, Jul 08, 2013 at 02:46:17PM -0600, Bjorn Helgaas wrote:
>>On Mon, Jul 1, 2013 at 9:10 AM, Wei Yang <weiyang@xxxxxxxxxxxxxxxxxx> wrote:
>>> Normally, on one pci bus there would be more devices than pci buses. When
>>> calculating the depth of pci bus, it would be more time efficient by
>>> enumerating through the child buses instead of the child devices.
>>>
>>> Also by doing so, the code seems more self explaining. Previously, it go
>>> through the pci devices and check whether a bridge introduce a child bus or
>>> not, which needs more background knowledge to understand it.
>>>
>>> This patch caculating the depth by enumerating on pci bus hierachy in an
>>> iterative way.
>>
>>Your code does have the advantage of not being recursive, but the
>>original code is significantly shorter and, in my opinion, much more
>>readable. This is not in a performance path, so I don't see any
>>advantage in optimizing.
>
> Thanks for your comments~
>
> Yes, this doesn't benefit a lot on the performance.
>
> The benefit of this code is not only the recursive approach, but on
> calculating the depth of pci bus tree by iterating on pci bus tree instead of
> the pci device tree.
>
> In my mind, there is less pci bus structrue than the pci device structure, and
> hope the code would be more self explaining for the audience by going through
> the pci bus tree to calculate the pci bus tree depth.
>
> My original code looks like below. Do you think this one is more friendly to
> the audience?
Yes.
> @@ -1300,21 +1300,19 @@ static void pci_bus_dump_resources(struct pci_bus *bus)
> static int __init pci_bus_get_depth(struct pci_bus *bus)
> {
> int depth = 0;
> - struct pci_dev *dev;
> + struct pci_bus *child_bus;
>
> - list_for_each_entry(dev, &bus->devices, bus_list) {
> + list_for_each_entry(child_bus, &bus->children, node){
> int ret;
> - struct pci_bus *b = dev->subordinate;
> - if (!b)
> - continue;
>
> - ret = pci_bus_get_depth(b);
> + ret = pci_bus_get_depth(child_bus);
> if (ret + 1 > depth)
> depth = ret + 1;
> }
>
> return depth;
> }
>
> And yes again, this change will not bring much improvement for the performance.
> If this is not good, just ignore the code. :-)
>
>>
>>> Signed-off-by: Wei Yang <weiyang@xxxxxxxxxxxxxxxxxx>
>>> Reviewed-by: Gavin Shan <shangw@xxxxxxxxxxxxxxxxxx>
>>> Reviewed-by: Ram Pai <linuxram@xxxxxxxxxx>
>>> Reviewed-by: Xiao Guangrong <xiaoguangrong@xxxxxxxxxxxxxxxxxx>
>>> Reviewed-by: Mike Qiu <qiudayu@xxxxxxxxxxxxxxxxxx>
>>> ---
>>> drivers/pci/setup-bus.c | 43 ++++++++++++++++++++++++++++++++-----------
>>> 1 files changed, 32 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>>> index 16abaaa..b333f73 100644
>>> --- a/drivers/pci/setup-bus.c
>>> +++ b/drivers/pci/setup-bus.c
>>> @@ -1299,22 +1299,43 @@ static void pci_bus_dump_resources(struct pci_bus *bus)
>>>
>>> static int __init pci_bus_get_depth(struct pci_bus *bus)
>>> {
>>> - int depth = 0;
>>> - struct pci_dev *dev;
>>> + int max_depth, depth;
>>> + struct pci_bus *parent, *curr;
>>> + struct list_head *node;
>>>
>>> - list_for_each_entry(dev, &bus->devices, bus_list) {
>>> - int ret;
>>> - struct pci_bus *b = dev->subordinate;
>>> - if (!b)
>>> - continue;
>>> + /* no child? */
>>> + if (list_empty(&bus->children))
>>> + return 0;
>>>
>>> - ret = pci_bus_get_depth(b);
>>> - if (ret + 1 > depth)
>>> - depth = ret + 1;
>>> + node = bus->children.next;
>>> + parent = bus;
>>> + max_depth = depth = 1;
>>> +
>>> + while (parent) {
>>> + /* hit the head, go back to parent level */
>>> + if (node == &parent->children) {
>>> + node = parent->node.next;
>>> + parent = parent->parent;
>>> + depth--;
>>> + continue;
>>> + }
>>> + curr = list_entry(node, struct pci_bus, node);
>>> + /* depth first */
>>> + if (!list_empty(&curr->children)) {
>>> + node = curr->children.next;
>>> + parent = curr;
>>> + depth++;
>>> + if (max_depth < depth)
>>> + max_depth = depth;
>>> + }
>>> + /* no child, go to the sibling */
>>> + else
>>> + node = curr->node.next;
>>> }
>>>
>>> - return depth;
>>> + return max_depth;
>>> }
>>> +
>>> static int __init pci_get_max_depth(void)
>>> {
>>> int depth = 0;
>>> --
>>> 1.7.5.4
>>>
>>> --
>>> 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
>
> --
> 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
