Re: [PATCH] NET : rt_check_expire() can take a long time, add a cond_resched()

[Eric Dumazet <dada1@cosmosbay.com>]

Andi Kleen a écrit :
> Eric Dumazet <dada1@cosmosbay.com> writes:
>> Using a "if (need_resched())" test before calling "cond_resched();" is
>> necessary to avoid spending too much time doing the resched check.
> 
> The only difference between cond_resched() and if (need_resched())
> cond_resched() is one function call less and one might_sleep less. If
> the might_sleep or the function call are really problems (did you
> measure it? -- i doubt it somewhat) then it would be better to fix the
> generic code to either inline that or supply a __cond_resched()
> without might_sleep.

Please note that :

if (need_resched())
     cond_resched();

will re-test need_resched() once cond_resched() is called.

So it may sound unnecessary but in the rt_check_expire() case, with a loop 
potentially doing XXX.XXX iterations, being able to bypass the function call 
is a clear win (in my bench case, 25 ms instead of 88 ms). Impact on I-cache 
is irrelevant here as this rt_check_expires() runs once every 60 sec.

I think the actual cond_resched() is fine for other uses in the kernel, that 
are not used in a loop : In the general case, kernel text size should be as 
small as possible to reduce I-cache pressure, so a function call is better 
than an inline.

> 
> A cheaper change might have been to just limit the number of buckets
> scanned.
> 

Well, not in some particular cases, when there are 3 millions of routes for 
example in the cache. We really want to scan/free them eventually :)

An admin already has the possibility to tune 
/proc/sys/net/ipv4/route/gc_interval and /proc/sys/net/ipv4/route/gc_timeout, 
so on a big cache, it will probably set gc_interval to 1 instead of 60

Next step will be to move "ip route flush cache" and rt_secret_rebuild() 
handling from softirq to process context too, since this still can kill a machine.