diff mbox

[net-2.6] Re: rib_trie / Fix inflate_threshold_root. Now=15 size=11 bits

Message ID 20090630070929.GB5589@ff.dom.local
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Jarek Poplawski June 30, 2009, 7:09 a.m. UTC
On Mon, Jun 29, 2009 at 10:47:03AM +0000, Jarek Poplawski wrote:
> On Mon, Jun 29, 2009 at 11:51:52AM +0200, Paweł Staszewski wrote:
> > I apply this patch
> >
> > fib_triestats in attached file :)
> 
> Great! But it would be nice to check if this (accidentally ;-) might
> fix the previous problem, so I attach below the patch with "manual
> RCU", which btw. (or even more important) should verify RCU use here.
> 
> It should be applied on top of this last "Fix..., part3". And
> again: it's quite probable it can fail, so with caution, no hurry
> (it can wait for quiet time)...

Pawel, here is another try to check what's going on here, so just
like before, but this one on top of these 2 last working patches,
plus quite time... (Stats aren't necessary; if these are some doubts
let me know.)

Thanks,
Jarek P.
--------------------> (synchronize_rcu take 5)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Paweł Staszewski June 30, 2009, 8:16 p.m. UTC | #1
Jarek Poplawski pisze:
> On Mon, Jun 29, 2009 at 10:47:03AM +0000, Jarek Poplawski wrote:
>   
>> On Mon, Jun 29, 2009 at 11:51:52AM +0200, Paweł Staszewski wrote:
>>     
>>> I apply this patch
>>>
>>> fib_triestats in attached file :)
>>>       
>> Great! But it would be nice to check if this (accidentally ;-) might
>> fix the previous problem, so I attach below the patch with "manual
>> RCU", which btw. (or even more important) should verify RCU use here.
>>
>> It should be applied on top of this last "Fix..., part3". And
>> again: it's quite probable it can fail, so with caution, no hurry
>> (it can wait for quiet time)...
>>     
>
> Pawel, here is another try to check what's going on here, so just
> like before, but this one on top of these 2 last working patches,
> plus quite time... (Stats aren't necessary; if these are some doubts
> let me know.)
>
> Thanks,
> Jarek P.
> --------------------> (synchronize_rcu take 5)
>
> diff -Nurp a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> --- a/net/ipv4/fib_trie.c	2009-06-29 10:00:14.000000000 +0000
> +++ b/net/ipv4/fib_trie.c	2009-06-30 06:50:35.000000000 +0000
> @@ -1036,6 +1036,7 @@ static void trie_rebalance(struct trie *
>  
>  	rcu_assign_pointer(t->trie, (struct node *)tn);
>  	tnode_free_flush();
> +	synchronize_rcu();
>  
>  	return;
>  }
>   

Apply and tested

Traffic is not forwarded after apply this patch.:)

> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>   

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarek Poplawski June 30, 2009, 8:41 p.m. UTC | #2
On Tue, Jun 30, 2009 at 10:16:57PM +0200, Paweł Staszewski wrote:
> Jarek Poplawski pisze:
>> On Mon, Jun 29, 2009 at 10:47:03AM +0000, Jarek Poplawski wrote:
>>   
>>> On Mon, Jun 29, 2009 at 11:51:52AM +0200, Paweł Staszewski wrote:
>>>     
>>>> I apply this patch
>>>>
>>>> fib_triestats in attached file :)
>>>>       
>>> Great! But it would be nice to check if this (accidentally ;-) might
>>> fix the previous problem, so I attach below the patch with "manual
>>> RCU", which btw. (or even more important) should verify RCU use here.
>>>
>>> It should be applied on top of this last "Fix..., part3". And
>>> again: it's quite probable it can fail, so with caution, no hurry
>>> (it can wait for quiet time)...
>>>     
>>
>> Pawel, here is another try to check what's going on here, so just
>> like before, but this one on top of these 2 last working patches,
>> plus quite time... (Stats aren't necessary; if these are some doubts
>> let me know.)
>>
>> Thanks,
>> Jarek P.
>> --------------------> (synchronize_rcu take 5)
>>
>> diff -Nurp a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
>> --- a/net/ipv4/fib_trie.c	2009-06-29 10:00:14.000000000 +0000
>> +++ b/net/ipv4/fib_trie.c	2009-06-30 06:50:35.000000000 +0000
>> @@ -1036,6 +1036,7 @@ static void trie_rebalance(struct trie *
>>   	rcu_assign_pointer(t->trie, (struct node *)tn);
>>  	tnode_free_flush();
>> +	synchronize_rcu();
>>   	return;
>>  }
>>   
>
> Apply and tested
>
> Traffic is not forwarded after apply this patch.:)

A little comment: these last 2 patches weren't exactly to fix the
problem you reported, which should be mostly fixed by the earlier
patch.

There is some other bug, which you omit with CONFIG_PREEMPT_NONE
(but it's not for sure there is no by effects). So, I'd like to be
sure you are willing and can (without too much risk) to do more such
tests. Alas I've no way to generate similar conditions so it would
simply have to wait for somebody else.

Many thanks again,
Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paweł Staszewski June 30, 2009, 11:31 p.m. UTC | #3
Jarek Poplawski pisze:
> On Tue, Jun 30, 2009 at 10:16:57PM +0200, Paweł Staszewski wrote:
>   
>> Jarek Poplawski pisze:
>>     
>>> On Mon, Jun 29, 2009 at 10:47:03AM +0000, Jarek Poplawski wrote:
>>>   
>>>       
>>>> On Mon, Jun 29, 2009 at 11:51:52AM +0200, Paweł Staszewski wrote:
>>>>     
>>>>         
>>>>> I apply this patch
>>>>>
>>>>> fib_triestats in attached file :)
>>>>>       
>>>>>           
>>>> Great! But it would be nice to check if this (accidentally ;-) might
>>>> fix the previous problem, so I attach below the patch with "manual
>>>> RCU", which btw. (or even more important) should verify RCU use here.
>>>>
>>>> It should be applied on top of this last "Fix..., part3". And
>>>> again: it's quite probable it can fail, so with caution, no hurry
>>>> (it can wait for quiet time)...
>>>>     
>>>>         
>>> Pawel, here is another try to check what's going on here, so just
>>> like before, but this one on top of these 2 last working patches,
>>> plus quite time... (Stats aren't necessary; if these are some doubts
>>> let me know.)
>>>
>>> Thanks,
>>> Jarek P.
>>> --------------------> (synchronize_rcu take 5)
>>>
>>> diff -Nurp a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
>>> --- a/net/ipv4/fib_trie.c	2009-06-29 10:00:14.000000000 +0000
>>> +++ b/net/ipv4/fib_trie.c	2009-06-30 06:50:35.000000000 +0000
>>> @@ -1036,6 +1036,7 @@ static void trie_rebalance(struct trie *
>>>   	rcu_assign_pointer(t->trie, (struct node *)tn);
>>>  	tnode_free_flush();
>>> +	synchronize_rcu();
>>>   	return;
>>>  }
>>>   
>>>       
>> Apply and tested
>>
>> Traffic is not forwarded after apply this patch.:)
>>     
>
> A little comment: these last 2 patches weren't exactly to fix the
> problem you reported, which should be mostly fixed by the earlier
> patch.
>
> There is some other bug, which you omit with CONFIG_PREEMPT_NONE
> (but it's not for sure there is no by effects). So, I'd like to be
> sure you are willing and can (without too much risk) to do more such
> tests. Alas I've no way to generate similar conditions so it would
> simply have to wait for somebody else.
>
>   
Yes i can make tests like this.
My network is splited to test clients and other normal clients
so it's really no problem to make testing. - if testing clients working 
then traffic from normal clients is also switched to this router (but if 
traffic is not forwarded "like in this case" for testing clients then 
failover switching them to working router )

and other point to make this tests - is that - it is good to have all in 
linux kernel networking working well :)

Regards
Paweł Staszewski

> Many thanks again,
> Jarek P.
>
>
>   

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff -Nurp a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
--- a/net/ipv4/fib_trie.c	2009-06-29 10:00:14.000000000 +0000
+++ b/net/ipv4/fib_trie.c	2009-06-30 06:50:35.000000000 +0000
@@ -1036,6 +1036,7 @@  static void trie_rebalance(struct trie *
 
 	rcu_assign_pointer(t->trie, (struct node *)tn);
 	tnode_free_flush();
+	synchronize_rcu();
 
 	return;
 }