Re: [XenPPC] Fixed bug in ofd find by property

[Maria Butrico <butrico@xxxxxxxxxxxxx>]

A clarification about this patch and the problem that it fixes.  During 
boot we copy the OF tree to an OFD tree.  Since the orignal OF tree has 
properties whose values are references to other nodes, we fixup the OFD 
tree.  To do so we build a list of properties that are known, by their 
semantic, to be references to other nodes.  The code that build this 
list iterates through the siblings of a node and recurses down the 
children of a node.  The list is implemented by a head and a tail 
pointer.  The recursive steps updates the head pointer by returning the 
head pointer that was established in this level of the recursion and 
recursively by any other level.  In the original version of the code, 
i.e, before this patch,  the tail pointer was not updated by a recursive 
calls; thus a return from a recursive call would revert to the previous 
value of the tail pointer.  Since items are added at end tail end of the 
list, return from recursion would effectively truncate the list.
My patch invokes the recursive step with a reference to the tail 
pointer.  This is certainly not elegant.  We could alternatively change 
this so that the list is compiled in the reverse order by pushing 
elements at the head of the list.  This would eliminate the need for a 
tail pointer entirely.  For the use of this function that I outlined 
above this should not be a problem.  I do not know if any other uses of 
this function require a certain order. 

Maria Butrico wrote:
> Signed-off-by: Maria Butrico <butrico@xxxxxxxxxxxxxx>
>
> summary:     Fixed bug in ofd find by property.
> Bug was that find by property established a partial next/prev list,
> with only one element, so that use of this function in ofd fixup code
> could not fixup all the nodes that needed to be fixed.
>
> diff -r 76805cb82965 xen/arch/ppc/of-devtree.c
> --- a/xen/arch/ppc/of-devtree.c Wed Mar 15 13:19:41 2006 -0600
> +++ b/xen/arch/ppc/of-devtree.c Mon Mar 20 14:56:50 2006 -0500
> @@ -865,7 +865,7 @@ static ofdn_t ofd_find_by_prop(
>  static ofdn_t ofd_find_by_prop(
>      struct ofd_mem *m,
>      ofdn_t head,
> -    ofdn_t prev,
> +    ofdn_t *prev_p,
>      ofdn_t n,
>      const char *name,
>      const void *val,
> @@ -893,22 +893,22 @@ retry:
>              }
>          }
>          if ( match == 1 ) {
> -            if ( prev >= 0 ) {
> -                np = ofd_node_get(m, prev);
> +            if ( *prev_p >= 0 ) {
> +                np = ofd_node_get(m, *prev_p);
>                  np->on_next = n;
>              } else {
>                  head = n;
>              }
>              np = ofd_node_get(m, n);
> -            np->on_prev = prev;
> +            np->on_prev = *prev_p;
>              np->on_next = -1;
> -            prev = n;
> +            *prev_p = n;
>          }
>      }
>  
>      p = ofd_node_child(m, n);
>      if ( p > 0 ) {
> -        head = ofd_find_by_prop(m, head, prev, p, name, val, sz);
> +        head = ofd_find_by_prop(m, head, prev_p, p, name, val, sz);
>      }
>  
>      p = ofd_node_peer(m, n);
> @@ -933,7 +933,8 @@ ofdn_t ofd_node_find_by_prop(
>          n = OFD_ROOT;
>      }
>  
> -    return ofd_find_by_prop(m, -1, -1, n, name, val, sz);
> +    ofdn_t prev = -1;
> +    return ofd_find_by_prop(m, -1, &prev, n, name, val, sz);
>  }
>  
>  ofdn_t ofd_node_find_next(void *mem, ofdn_t n)
> _______________________________________________
> Xen-ppc-devel mailing list
> Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ppc-devel
>   


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