From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753888AbbITIFf (ORCPT ); Sun, 20 Sep 2015 04:05:35 -0400 Received: from mail-wi0-f180.google.com ([209.85.212.180]:34328 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753333AbbITIFZ (ORCPT ); Sun, 20 Sep 2015 04:05:25 -0400 Date: Sun, 20 Sep 2015 10:05:20 +0200 From: Ingo Molnar To: Alex Snast Cc: Arnaldo Carvalho de Melo , Ingo Molnar , Peter Zijlstra , linux-kernel@vger.kernel.org, Jiri Olsa , Adrian Hunter , Namhyung Kim Subject: Re: [PATCH 1/2] perf tools: Use postorder rbtree iteration when removing symbols Message-ID: <20150920080519.GA8234@gmail.com> References: <1442667815-10749-1-git-send-email-asnast@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1442667815-10749-1-git-send-email-asnast@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Alex Snast 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 > --- > 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 -#include +#include "inat.h" +#include "insn.h" should be eliminated too I think, to make it more obvious when the two versions match. Thanks, Ingo