lkml.org 
[lkml]   [2009]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [BUG] rbtree bug with mmotm 2009-04-14-17-24
On Wed, Apr 22 2009, Steve Wise wrote:
> Steve Wise wrote:
>> Jens Axboe wrote:
>>
>>
>>
>>>> Still crashes with this variant:
>>>>
>>>
>>> OK, so please try and revert a36e71f996e25d6213f57951f7ae1874086ec57e
>>> and see if that works?
>>>
>>> Or, better yet, please try and revert
>>> 55a63998b8967615a15e2211ba0ff3a84a565824 first. If that doesn't work,
>>> try the above revert.
>>>
>>>
>>
>> Reverting 55a63998b8967615a15e2211ba0ff3a84a565824 did the trick.
>>
>>
>> Steve.
>>
>>
>
> <snipit from 55a63998b8967615a15e2211ba0ff3a84a565824>
>
> @@ -200,17 +197,14 @@ static void __rb_erase_color(struct rb_node *node,
> struct rb_node *parent,
> {
> if (!other->rb_left ||
> rb_is_black(other->rb_left))
> {
> - register struct rb_node *o_right;
> - if ((o_right = other->rb_right))
> - rb_set_black(o_right);
> + rb_set_black(other->rb_right);
> rb_set_red(other);
> __rb_rotate_left(other, root);
> other = parent->rb_left;
> }
> rb_set_color(other, rb_color(parent));
> rb_set_black(parent);
> - if (other->rb_left)
> - rb_set_black(other->rb_left);
> + rb_set_black(other->rb_left);
> __rb_rotate_right(parent, root);
> node = root->rb_node;
> break;
>
>
> I don't know this code, but isn't the 'if (other->rb_left)' really
> needed? Or is it always true that if '!other->rb_left' is true entering
> this snipit, then after executing the first 'if' block, then
> 'other->rb_left' must be a valid ptr? (how's that for confusing english?
> :)

Heh, not sure. What is sure is that the commit in question is
problematic. Either because it itself has a bug, or because it exposes a
bug elsewhere in the rbtree code. So I'd suggest we just revert that
commit ASAP.

--
Jens Axboe



\
 
 \ /
  Last update: 2009-04-23 07:37    [W:0.063 / U:0.868 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site