[Ocfs2-tools-devel] [PATCH 2/2] Modify the check in io_cache_disconnect.

[tao.ma]

Joel Becker wrote:
> On Mon, Oct 22, 2007 at 09:51:07AM +0800, tao.ma wrote:
>   
>> I have seen this happen in practice when I try to use it to speed up the 
>> extent block iteration. ;)
>>     
>
> 	I'd like to reproduce it here.
>
>   
>> As for RB_ROOT, if you look at the definition of RB_ROOT and the how we use 
>> rb_link_node, you will get that we will use ic->ic_lookup->rb_node as the 
>> real rb-root which can be NULL during the insertion, not ic->ic_lookup.
>>     
>
> 	ic->ic_lookup->rb_node *is* RB_ROOT.  RB_ROOT is just a
> structure containing a struct rb_node.
>   
RB_ROOT contains a pointer to a struct rb_node. So it always points to 
the root and it is not the root itself. The real root's parent would be 
NULL and it does't point to RB_ROOT actually since RB_ROOT(which is an 
instance of rb-root) isn't defined as a rb-node(struct rb_node *parent).

> 	An icb is a larger structure containing an rb_node.  If our
> lookup functions ever return RB_ROOT instead of an icb, we have a
> problem.  All icbs should be children of RB_ROOT somehow.
>   
All icbs's rb-node should be children of RB_ROOT->rb_node, not RB_ROOT 
itself.
>   
>> In rb_erase, you can see that the code already handle the condition when 
>> the removed entry's rb_parent is NULL.
>>     
>
> 	Hmm.  I can't quite figure out how that ever happens.  It's a
> tree, nodes need parents.
>
>   
>> The test case should be very simple. You just need to erase the root. Maybe 
>> you just need to initialize the cache,  call io_cache_insert, 
>> io_cache_disconnect and io_cache_insert with the same block num. The second 
>> insert should be fail since the disconnect will not remove the block and 
>> you want to insert for a second time.
>>     
>
> 	Ok, that sounds like a worthwhile test case.  Let me see if I
> can make it happen here.
>   
I haven't tried it, but it should work. Please let me know if it doesn't 
output the error. I will try it here. ;)