Re: [PATCH 2/8] trace/trace_stat: use rbtree postorder iterationhelper instead of opencoding

[Steven Rostedt]

On Fri,  1 Nov 2013 15:38:46 -0700
Cody P Schafer <cody@xxxxxxxxxxxxxxxxxx> wrote:

> Use rbtree_postorder_for_each_entry_safe() to destroy the rbtree instead
> of opencoding an alternate postorder iteration that modifies the tree
> 
> Signed-off-by: Cody P Schafer <cody@xxxxxxxxxxxxxxxxxx>
> ---
>  kernel/trace/trace_stat.c | 42 ++++++------------------------------------
>  1 file changed, 6 insertions(+), 36 deletions(-)
> 
> diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
> index 847f88a..fa53acc 100644
> --- a/kernel/trace/trace_stat.c
> +++ b/kernel/trace/trace_stat.c
> @@ -43,46 +43,16 @@ static DEFINE_MUTEX(all_stat_sessions_mutex);
>  /* The root directory for all stat files */
>  static struct dentry		*stat_dir;
>  
> -/*
> - * Iterate through the rbtree using a post order traversal path
> - * to release the next node.
> - * It won't necessary release one at each iteration
> - * but it will at least advance closer to the next one
> - * to be released.
> - */
> -static struct rb_node *release_next(struct tracer_stat *ts,
> -				    struct rb_node *node)
> +static void __reset_stat_session(struct stat_session *session)
>  {
> -	struct stat_node *snode;
> -	struct rb_node *parent = rb_parent(node);
> -
> -	if (node->rb_left)
> -		return node->rb_left;
> -	else if (node->rb_right)
> -		return node->rb_right;
> -	else {
> -		if (!parent)
> -			;
> -		else if (parent->rb_left == node)
> -			parent->rb_left = NULL;
> -		else
> -			parent->rb_right = NULL;
> +	struct stat_node *snode, *n;
>  
> -		snode = container_of(node, struct stat_node, node);
> -		if (ts->stat_release)
> -			ts->stat_release(snode->stat);
> +	rbtree_postorder_for_each_entry_safe(snode, n, &session->stat_root,
> +			node) {

This is one of those cases that a line break is uglier than keeping it
on the same line. Heck, it's only 4 characters over the 80 char limit.

Other than that, I'm fine with this patch. Want me to take this
separately?

-- Steve


> +		if (session->ts->stat_release)
> +			session->ts->stat_release(snode->stat);
>  		kfree(snode);
> -
> -		return parent;
>  	}
> -}
> -
> -static void __reset_stat_session(struct stat_session *session)
> -{
> -	struct rb_node *node = session->stat_root.rb_node;
> -
> -	while (node)
> -		node = release_next(session->ts, node);
>  
>  	session->stat_root = RB_ROOT;
>  }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/