On 10.10.24 08:55, Huang Ying wrote:
Currently, if __region_intersects() finds any overlapped but unmatched
resource, it walks the descendant resource tree to check for
overlapped and matched descendant resources. This is achieved using
for_each_resource(), which iterates not only the descent tree, but
also subsequent sibling trees in certain scenarios. While this
doesn't introduce bugs, it makes code hard to be understood and
potentially inefficient.
So, the patch renames next_resource() to __next_resource() and
modified it to return NULL after traversing all descent resources.
Test shows that this avoids unnecessary resource tree walking in
__region_intersects().
It appears even better to revise for_each_resource() to traverse the
descendant resource tree of "_root" only. But that will cause "_root"
to be evaluated twice, which I don't find a good way to eliminate.
I'm not sure I'm enjoying below code, it makes it harder for me to
understand what's happening.
I'm also not 100% sure why "p" becomes "root" and "dp" becomes "p" when
calling the function :) Likely this works as intended, but it's confusing
(IOW, bad naming, especially for dp).
I think you should just leave next_resource() alone and rather add
a new function that doesn't conditionally consume NULL pointers
(and also no skip_children because you're passing false either way).
static struct resource *next_resource_XXX(struct resource *root,
struct resource *p)
{
while (!p->sibling && p->parent) {
p = p->parent;
if (p == root)
return NULL;
}
return p->sibling;
}
Maybe even better, add a new for_each_resource() macro that expresses the intended semantics.
#define for_each_resource_XXX(_root, _p) \
for ((_p) = (_root)->child; (_p); (_p) = next_resource_XXX(_root, _p))
XXX TBD
Or do you think this should not only be "improved" for the __region_intersects() use case
but for all for_each_resource() users? I cannot tell easily.
--
Cheers,
David / dhildenb
