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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <473C0297.5090004@cosmosbay.com>
Date:	Thu, 15 Nov 2007 09:25:59 +0100
From:	Eric Dumazet <dada1@...mosbay.com>
To:	Andi Kleen <andi@...stfloor.org>
CC:	"David S. Miller" <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [PATCH] NET : rt_check_expire() can take a long time, add a cond_resched()

Andi Kleen a écrit :
> Eric Dumazet <dada1@...mosbay.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.

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