Messages in this thread | | | Date | Sun, 20 Sep 2015 10:05:20 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH 1/2] perf tools: Use postorder rbtree iteration when removing symbols |
| |
* Alex Snast <asnast@gmail.com> wrote:
> Avoid using rb_erase when removing symbols as it requires rbtree > rebalancing, instead preform a post order iteration when deleting tree > symbols. > > Signed-off-by: Alex Snast <asnast@gmail.com> > --- > tools/include/linux/rbtree.h | 14 ++++++++++++++ > tools/perf/util/symbol.c | 11 ++++------- > 2 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/tools/include/linux/rbtree.h b/tools/include/linux/rbtree.h > index 1125822..14c646d 100644 > --- a/tools/include/linux/rbtree.h > +++ b/tools/include/linux/rbtree.h > @@ -90,6 +90,20 @@ static inline void rb_link_node(struct rb_node *node, struct rb_node *parent, > ____ptr ? rb_entry(____ptr, type, member) : NULL; \ > }) > > +/** > + * rbtree_postorder_for_each_entry_safe - iterate over rb_root in post order of > + * given type safe against removal of rb_node entry > + * > + * @pos: the 'type *' to use as a loop cursor. > + * @n: another 'type *' to use as temporary storage > + * @root: 'rb_root *' of the rbtree. > + * @field: the name of the rb_node field within 'type'. > + */ > +#define rbtree_postorder_for_each_entry_safe(pos, n, root, field) \ > + for (pos = rb_entry_safe(rb_first_postorder(root), typeof(*pos), field); \ > + pos && ({ n = rb_entry_safe(rb_next_postorder(&pos->field), \ > + typeof(*pos), field); 1; }); \ > + pos = n)
So looks like this is something that include/linux/rbtree.h already has, right?
I think we should strive to match the two implementations and generate a build time warning (but not a build time failure) if the two diverge.
There were checking facilities added recently for another kernel source code file, to make sure tools/perf/util/intel-pt-decoder/insn.c matches arch/x86/lib/insn.c.
Btw., side note, regarding insn.c, the remaining delta between the two files:
-#include <asm/inat.h> -#include <asm/insn.h> +#include "inat.h" +#include "insn.h"
should be eliminated too I think, to make it more obvious when the two versions match.
Thanks,
Ingo
| |