Re: [Xen-ia64-devel] [Patch 4/4] [RFC] Xwindow: Modify pci_acpi_scan_root()

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

Hi.

You are trying to avoid duplicated ioremap hypercall on
overlapped regions. On the other hand, duplicated ioremap hypercall
doesn't harm as long as arguments are consistent.
So the code can be simplified, I suppose.

But I'm not sure whether this optimization is necessary or
how region overlaps.
I'm not sure how the optimization affects on the boot time
of big irons. Opinions?
If you think such a optimization is necessary, you may want to reduce
more hypercalls being aware of PAGE_SIZE.

thanks.

On Fri, Jun 01, 2007 at 10:24:23AM +0900, Jun Kamada wrote:
> Hi Alex-san,
> 
> Thank you for your helpful comments.
> 
> I'm going to modify the patch according to your comments.
> BTW, I will make some explanation about following your comment.
> 
> On Wed, 30 May 2007 23:40:46 -0600
> Alex Williamson <alex.williamson@xxxxxx> wrote:
> > Please add some comments on what the code below is trying to accomplish.
> > Comments are good ;)
> > 
> > > +               }
> > > +
> > > +               if (curr_lvl > prev_lvl) {
> > > +                       if (ptr->child != 0) {
> > > +                               ptr = ptr->child;
> > > +                               curr_lvl++;
> > > +                               prev_lvl++;
> > > +                       } else if (ptr->sibling != 0) {
> > > +                               ptr = ptr->sibling;
> > > +                       } else {
> > > +                               ptr = ptr->parent;
> > > +                               curr_lvl--;
> > > +                               prev_lvl++;
> > > +                               if (curr_lvl == 0)
> > > +                                       goto out;
> 
> This function (__make_issue_list()) traverses all elements of the
> resource structure list by depth first algorhithm.  
> 
> Structure of the list is shown below.
> 
> -----
> C: Pointer to Child
> P: Pointer to Parent
> S: Pointer to Sibling
> 
> --------------------------------------------------
> |                                                 |
> |                                                 |
> -->|---|-+-|---|                                  |
>    | C | P | S +                                  |          <== Level i-1
>    |-+-|---|---|                                  |
>      |                                            |
>    --                                             |
>    |                                              |
>    -->|---|-+-|---|  |-->|---|-+-|---|  |-->|---|-+-|---|
>       | C | P | S +--    | C | P | S +--    | C | P | S |    <== Level i
>       |-+-|---|---|      |-+-|---|---|      |-+-|---|---|
>         |
>       --
>       |
>       -->|---|-+-|---|
>          | C | P | S +                                       <= Level i+1
>          |-+-|---|---|
> 
> Condition "curr_lvl > prev_lvl" indicates that the "ptr" has gone down
> from level i-1 to level i. On that case, "ptr" tries to go down to level
> i+1 furthermore (because of depth first) if child exists. However, if
> no child exits, "ptr" goes to sibling. If no child and no sibling exist,
> "ptr" goes up.
> 
> On go up mode (condition "curr_lvl < prev_lvl"), "ptr" tries to go to
> sibling and enters go down mode. If no sibling exists, "ptr" goes to
> parent.
> 
> -----
> At this point, I found a bug on the code as shown below. Without the
> line, "ptr" cannot go to the other children. I would like to fix it and
> re-post.
> 
>         } else if (curr_lvl < prev_lvl) {
>             if (ptr->sibling != 0) {
>                 ptr = ptr->sibling;
>                 prev_lvl = curr_lvl - 1;      <== We need this line.
>             } else {
>                 ptr = ptr->parent;
>                 curr_lvl--;
>                 prev_lvl--;
>                 if (curr_lvl == 0)
>                     goto out;
>             }
> 
> 
> Thanks,
> 
> -----
> Jun Kamada
> Virtual Systems Development Div.
> Platform Technology Development Unit
> Fujitsu Ltd.
> kama@xxxxxxxxxxxxxx
> 
> 
> 
> _______________________________________________
> Xen-ia64-devel mailing list
> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ia64-devel
> 

-- 
yamahata

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel