WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ia64-devel

Re: [Xen-ia64-devel] [Patch 4/4] [RFC] Xwindow: Modify pci_acpi_scan_roo

To: Jun Kamada <kama@xxxxxxxxxxxxxx>
Subject: Re: [Xen-ia64-devel] [Patch 4/4] [RFC] Xwindow: Modify pci_acpi_scan_root()
From: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
Date: Sat, 2 Jun 2007 09:57:48 +0900
Cc: Alex Williamson <alex.williamson@xxxxxx>, xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 01 Jun 2007 17:55:45 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20070601085544.CF4B.KAMA@xxxxxxxxxxxxxx>
List-help: <mailto:xen-ia64-devel-request@lists.xensource.com?subject=help>
List-id: Discussion of the ia64 port of Xen <xen-ia64-devel.lists.xensource.com>
List-post: <mailto:xen-ia64-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ia64-devel>, <mailto:xen-ia64-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ia64-devel>, <mailto:xen-ia64-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20070530144336.1D7A.KAMA@xxxxxxxxxxxxxx> <1180590046.6221.113.camel@bling> <20070601085544.CF4B.KAMA@xxxxxxxxxxxxxx>
Sender: xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.4.2.1i
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