* [PATCH] core: linkwatch should use jiffies64
@ 2006-05-07 10:13 Stefan Rompf
2006-05-08 6:07 ` David S. Miller
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Rompf @ 2006-05-07 10:13 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
Hi,
the linkwatch code can overflow on a jiffies wrap, scheduling
work with a too large delay. If the delay is >0x80000000,
internal_add_timer() seems to overflow too, hiding the bug, so
this isn't triggered too easily.
Best solution is to use jiffies64 for calculation as these
events happen with any possible delay in between.
This should be 2.6.17 stuff.
Signed-off-by: Stefan Rompf <stefan@loplof.de>
--- linux-2.6.17-rc3/net/core/link_watch.c.orig 2006-04-27 20:37:09.000000000 +0200
+++ linux-2.6.17-rc3/net/core/link_watch.c 2006-04-27 21:49:00.000000000 +0200
@@ -32,8 +32,8 @@
LW_SE_USED
};
+static u64 linkwatch_nextevent;
static unsigned long linkwatch_flags;
-static unsigned long linkwatch_nextevent;
static void linkwatch_event(void *dummy);
static DECLARE_WORK(linkwatch_work, linkwatch_event, NULL);
@@ -136,7 +136,7 @@
* cause a storm of messages on the netlink
* socket
*/
- linkwatch_nextevent = jiffies + HZ;
+ linkwatch_nextevent = get_jiffies_64() + HZ;
clear_bit(LW_RUNNING, &linkwatch_flags);
rtnl_lock();
@@ -170,7 +170,7 @@
spin_unlock_irqrestore(&lweventlist_lock, flags);
if (!test_and_set_bit(LW_RUNNING, &linkwatch_flags)) {
- unsigned long thisevent = jiffies;
+ u64 thisevent = get_jiffies_64();
if (thisevent >= linkwatch_nextevent) {
schedule_work(&linkwatch_work);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] core: linkwatch should use jiffies64
2006-05-07 10:13 [PATCH] core: linkwatch should use jiffies64 Stefan Rompf
@ 2006-05-08 6:07 ` David S. Miller
2006-05-08 18:28 ` Stefan Rompf
0 siblings, 1 reply; 6+ messages in thread
From: David S. Miller @ 2006-05-08 6:07 UTC (permalink / raw)
To: stefan; +Cc: netdev
From: Stefan Rompf <stefan@loplof.de>
Date: Sun, 7 May 2006 12:13:56 +0200
> the linkwatch code can overflow on a jiffies wrap, scheduling
> work with a too large delay. If the delay is >0x80000000,
> internal_add_timer() seems to overflow too, hiding the bug, so
> this isn't triggered too easily.
What is so special about what linkwatch is doing such
that it needs this kind of treatment and other similar
pieces or code do not?
We have all sorts of interfaces such as time_after() et el.
in order to deal with wrapping issues. And furthermore
using 64-bit jiffies here might not be appropriate because
they are not guarenteed to be accessed atomically, nothing
here prevents the upper 32-bits from being updated while
we sample the lower 32-bits on 32-bit systems that will do
the 64-bit jiffied load using two 32-bit loads.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] core: linkwatch should use jiffies64
2006-05-08 6:07 ` David S. Miller
@ 2006-05-08 18:28 ` Stefan Rompf
2006-05-09 11:26 ` [NET] linkwatch: Handle jiffies wrap-around Herbert Xu
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Rompf @ 2006-05-08 18:28 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev
Am Montag 08 Mai 2006 08:07 schrieb David S. Miller:
> What is so special about what linkwatch is doing such
> that it needs this kind of treatment and other similar
> pieces or code do not?
>
> We have all sorts of interfaces such as time_after() et el.
> in order to deal with wrapping issues.
time_after() and friends can handle jiffies wrapping, however they require the
difference between compared times to be less than 0x80000000 jiffies (about
24 days on HZ=1000) to work reliably on 32bit architectures. So if the
network is stable for 24 days, events generated within days 25-49 will suffer
a *huge* false delay.
> And furthermore
> using 64-bit jiffies here might not be appropriate because
> they are not guarenteed to be accessed atomically,
get_jiffies_64() handles this transparently.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* [NET] linkwatch: Handle jiffies wrap-around
2006-05-08 18:28 ` Stefan Rompf
@ 2006-05-09 11:26 ` Herbert Xu
2006-05-09 16:51 ` Stefan Rompf
0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2006-05-09 11:26 UTC (permalink / raw)
To: Stefan Rompf; +Cc: David S. Miller, netdev
[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]
On Mon, May 08, 2006 at 06:28:56PM +0000, Stefan Rompf wrote:
>
> time_after() and friends can handle jiffies wrapping, however they require the
> difference between compared times to be less than 0x80000000 jiffies (about
> 24 days on HZ=1000) to work reliably on 32bit architectures. So if the
> network is stable for 24 days, events generated within days 25-49 will suffer
> a *huge* false delay.
How about something like this?
[NET] linkwatch: Handle jiffies wrap-around
The test used in the linkwatch does not handle wrap-arounds correctly.
Since the intention of the code is to eliminate bursts of messages we
can afford to delay things up to a second. Using that fact we can
easily handle wrap-arounds by making sure that we don't delay things
by more than one second.
This is based on diagnosis and a patch by Stefan Rompf.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[-- Attachment #2: linkwatch-wrap.patch --]
[-- Type: text/plain, Size: 717 bytes --]
diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index 341de44..646937c 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -170,13 +170,13 @@
spin_unlock_irqrestore(&lweventlist_lock, flags);
if (!test_and_set_bit(LW_RUNNING, &linkwatch_flags)) {
- unsigned long thisevent = jiffies;
+ unsigned long delay = linkwatch_nextevent - jiffies;
- if (thisevent >= linkwatch_nextevent) {
+ /* If we wrap around we'll delay it by at most HZ. */
+ if (!delay || delay > HZ)
schedule_work(&linkwatch_work);
- } else {
- schedule_delayed_work(&linkwatch_work, linkwatch_nextevent - thisevent);
- }
+ else
+ schedule_delayed_work(&linkwatch_work, delay);
}
}
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [NET] linkwatch: Handle jiffies wrap-around
2006-05-09 11:26 ` [NET] linkwatch: Handle jiffies wrap-around Herbert Xu
@ 2006-05-09 16:51 ` Stefan Rompf
2006-05-09 22:28 ` David S. Miller
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Rompf @ 2006-05-09 16:51 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev
Am Dienstag 09 Mai 2006 13:26 schrieb Herbert Xu:
> The test used in the linkwatch does not handle wrap-arounds correctly.
> Since the intention of the code is to eliminate bursts of messages we
> can afford to delay things up to a second.
looks good, the code generates only up to 1 second delay for one second every
49 days, avoiding 64bit operations. David, please prefer this patch over mine.
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Acked-by: Stefan Rompf <stefan@loplof.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-05-09 22:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-05-07 10:13 [PATCH] core: linkwatch should use jiffies64 Stefan Rompf
2006-05-08 6:07 ` David S. Miller
2006-05-08 18:28 ` Stefan Rompf
2006-05-09 11:26 ` [NET] linkwatch: Handle jiffies wrap-around Herbert Xu
2006-05-09 16:51 ` Stefan Rompf
2006-05-09 22:28 ` David S. Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).