Message ID | 499E2C54.9000409@cn.fujitsu.com |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
From: Wei Yongjun <yjwei@cn.fujitsu.com> Date: Fri, 20 Feb 2009 12:06:44 +0800 > The functions time_before is more robust for comparing > jiffies against other values. > > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> There is some history of this in the networking code. If you inspect carefully, the open-coded versions of time comparisons in the networking have a larger window of acceptance. So actually, it is a regression fo use time_*() helpers in these cases. If these things have been like this for so long, it is very likely there is a good reason :-) -- 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
David Miller <davem@davemloft.net> wrote: > > There is some history of this in the networking code. > > If you inspect carefully, the open-coded versions of > time comparisons in the networking have a larger window > of acceptance. Hmm, it looks like the macro version is actually better in this case. - if (jiffies - t->err_time < IPTUNNEL_ERR_TIMEO) This wraps at jiffies = LONG_MAX + t->err_time + 1 + if (time_before(jiffies, t->err_time + IPTUNNEL_ERR_TIMEO)) This expands to (long)jiffies - (long)(t->err_time + IPTUNNEL_ERR_TIMEO) < 0 which wraps at jiffies = LONG_MAX + t->err_time + IPTUNNEL_ERR_TIMEO + 1 So assuming that IPTUNNEL_ERR_TIMEO > 0, then the macro version wraps around after the open-coded version, which would seem to mean that it's better, no? Cheers,
From: Herbert Xu <herbert@gondor.apana.org.au> Date: Fri, 20 Feb 2009 16:39:35 +0800 > So assuming that IPTUNNEL_ERR_TIMEO > 0, then the macro version > wraps around after the open-coded version, which would seem to > mean that it's better, no? Seems that way... Let me see if I can find Alexey's email where he explains all of this. I really don't want to touch these things without being able to read that again. If anyone else can find his posting about this stuff from years ago, please post the link. Thanks. -- 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
Wei Yongjun <yjwei@cn.fujitsu.com> wrote: > The functions time_before is more robust for comparing > jiffies against other values. > > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> Please resubmit these patches so that they can be reviewed again. Thanks,
From: Herbert Xu <herbert@gondor.apana.org.au> Date: Tue, 24 Feb 2009 15:13:52 +0800 > Wei Yongjun <yjwei@cn.fujitsu.com> wrote: > > The functions time_before is more robust for comparing > > jiffies against other values. > > > > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> > > Please resubmit these patches so that they can be reviewed again. Indeed, I think my original objections have been cleared up after some research. -- 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 --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index 0101521..811bef7 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c @@ -432,7 +432,7 @@ static void ipgre_err(struct sk_buff *skb, u32 info) if (t->parms.iph.ttl == 0 && type == ICMP_TIME_EXCEEDED) goto out; - if (jiffies - t->err_time < IPTUNNEL_ERR_TIMEO) + if (time_before(jiffies, t->err_time + IPTUNNEL_ERR_TIMEO)) t->err_count++; else t->err_count = 1; @@ -744,7 +744,8 @@ static int ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev) #endif if (tunnel->err_count > 0) { - if (jiffies - tunnel->err_time < IPTUNNEL_ERR_TIMEO) { + if (time_before(jiffies, + tunnel->err_time + IPTUNNEL_ERR_TIMEO)) { tunnel->err_count--; dst_link_failure(skb);
The functions time_before is more robust for comparing jiffies against other values. Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> --- net/ipv4/ip_gre.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)