Date: Fri, 14 Apr 2017 00:24:55 +0200
From: Alexandru Moise <00moses.alexander00@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: peterz@infradead.org, akpm@linux-foundation.org, dhowells@redhat.com,
        keescook@chromium.org, fykcee1@gmail.com
Subject: Question regarding Linux implementation of rbtrees
Message-ID: <20170413222455.GA15239@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.8.1 (2017-04-11)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2005
Lines: 57

Seeing as RB_RED is defined to be 0 in include/linux/rbtree_augmented.h
A call of this form: rb_set_parent_color(node, parent, RB_RED);
as seen in __rb_insert would only end up reassigning the parent "color"
(which is the parent pointer value cast to unsigned long) OR'd with 0.
Which would mean that nothing would really change regarding the parent's
"color". So, that would lead one to think that the diagram at case 2 showing
the grandparent's color going from black to red could not be completely accurate
as the Linux implementation presently stands.

Could the maintainers provide an answer as to why the below patch is the
__wrong__ thing to do? Apart from the obvious "the values of the macros
might change in the future".

Thanks,
../Alex
---
 lib/rbtree.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/lib/rbtree.c b/lib/rbtree.c
index 4ba2828a67c0..6b540be4dda4 100644
--- a/lib/rbtree.c
+++ b/lib/rbtree.c
@@ -135,7 +135,6 @@ __rb_insert(struct rb_node *node, struct rb_root *root,
 				rb_set_parent_color(parent, gparent, RB_BLACK);
 				node = gparent;
 				parent = rb_parent(node);
-				rb_set_parent_color(node, parent, RB_RED);
 				continue;
 			}
 
@@ -159,7 +158,6 @@ __rb_insert(struct rb_node *node, struct rb_root *root,
 				if (tmp)
 					rb_set_parent_color(tmp, parent,
 							    RB_BLACK);
-				rb_set_parent_color(parent, node, RB_RED);
 				augment_rotate(parent, node);
 				parent = node;
 				tmp = node->rb_right;
@@ -189,7 +187,6 @@ __rb_insert(struct rb_node *node, struct rb_root *root,
 				rb_set_parent_color(parent, gparent, RB_BLACK);
 				node = gparent;
 				parent = rb_parent(node);
-				rb_set_parent_color(node, parent, RB_RED);
 				continue;
 			}
 
@@ -202,7 +199,6 @@ __rb_insert(struct rb_node *node, struct rb_root *root,
 				if (tmp)
 					rb_set_parent_color(tmp, parent,
 							    RB_BLACK);
-				rb_set_parent_color(parent, node, RB_RED);
 				augment_rotate(parent, node);
 				parent = node;
 				tmp = node->rb_left;
-- 
2.12.2