Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Subject | Re: [PATCH 2.4.22-pre1][NET] timer cleanups | From | Vinay K Nallamothu <> | Date | Mon, 01 Sep 2003 19:30:51 +0530 |
| |
Hi Dave,
On Sun, 2003-08-31 at 09:03, David S. Miller wrote: > On Sat, 30 Aug 2003 21:11:37 +0530 > Vinay K Nallamothu <vinay-rc@naturesoft.net> wrote: > > > This patch does the following timer code cleanup: > > > > 1. Change del_timer/add_timer to mod_timer > > 2. Use static timer initialisation wherever applicable > > I'm not accepting this. In particular, the ip6_flowlabel.c > change is a bad idea because the code remains racey.
> Someone > submitted a similar move to mod_timer() for ip6_flowlabel That too was submitted by me. Unfortunately I never got to see your mail. No excuses though.
> > Just blindly doing these kinds of conversions to mod_timer() > is foolhardy. You need to apply some brains to it to make > sure you really are getting rid of whatever potential races > exist in the code. You are correct. While I tried to make sure that new bugs are not introduced, I haven't paid attention to the existing ones.
> And in the ip6_flowlabel.c case things are > as buggy as they were before.
Please find the updated patch below and let me know if it's ok.
Thanks Vinay
ip6_flowlabel.c: 1. Use static timer initializer 2. Use timer macros to handle jiffie wrap in comparisions 3. Convert del_timer/add_timer to mod_timer
ip6_flowlabel.c | 30 ++++++++++++++++-------------- 1 files changed, 16 insertions(+), 14 deletions(-)
diff -urN linux-2.4.22/net/ipv6/ip6_flowlabel.c linux-2.4.22-nvk/net/ipv6/ip6_flowlabel.c --- linux-2.4.22/net/ipv6/ip6_flowlabel.c 2003-06-14 10:03:12.000000000 +0530 +++ linux-2.4.22-nvk/net/ipv6/ip6_flowlabel.c 2003-09-01 16:47:03.000000000 +0530 @@ -48,7 +48,8 @@ static atomic_t fl_size = ATOMIC_INIT(0); static struct ip6_flowlabel *fl_ht[FL_HASH_MASK+1]; -static struct timer_list ip6_fl_gc_timer; +static void ip6_fl_gc(unsigned long dummy); +static struct timer_list ip6_fl_gc_timer = TIMER_INITIALIZER(ip6_fl_gc, 0, 0); /* FL hash table lock: it protects only of GC */ @@ -92,10 +93,12 @@ static void fl_release(struct ip6_flowlabel *fl) { + write_lock_bh(&ip6_fl_lock); + fl->lastuse = jiffies; if (atomic_dec_and_test(&fl->users)) { unsigned long ttd = fl->lastuse + fl->linger; - if ((long)(ttd - fl->expires) > 0) + if (time_after(ttd, fl->expires)) fl->expires = ttd; ttd = fl->expires; if (fl->opt && fl->share == IPV6_FL_S_EXCL) { @@ -103,11 +106,12 @@ fl->opt = NULL; kfree(opt); } - if (!del_timer(&ip6_fl_gc_timer) || - (long)(ip6_fl_gc_timer.expires - ttd) > 0) - ip6_fl_gc_timer.expires = ttd; - add_timer(&ip6_fl_gc_timer); + if (!timer_pending(&ip6_fl_gc_timer) || + time_after(ip6_fl_gc_timer.expires, ttd)) + mod_timer(&ip6_fl_gc_timer, ttd); } + + write_unlock_bh(&ip6_fl_lock); } static void ip6_fl_gc(unsigned long dummy) @@ -124,16 +128,16 @@ while ((fl=*flp) != NULL) { if (atomic_read(&fl->users) == 0) { unsigned long ttd = fl->lastuse + fl->linger; - if ((long)(ttd - fl->expires) > 0) + if (time_after(ttd, fl->expires)) fl->expires = ttd; ttd = fl->expires; - if ((long)(now - ttd) >= 0) { + if (time_after_eq(now, ttd)) { *flp = fl->next; fl_free(fl); atomic_dec(&fl_size); continue; } - if (!sched || (long)(ttd - sched) < 0) + if (!sched || time_before(ttd, sched)) sched = ttd; } flp = &fl->next; @@ -262,11 +266,11 @@ if (!expires) return -EPERM; fl->lastuse = jiffies; - if (fl->linger < linger) + if (time_before(fl->linger, linger)) fl->linger = linger; - if (expires < fl->linger) + if (time_before(expires, fl->linger)) expires = fl->linger; - if ((long)(fl->expires - (fl->lastuse+expires)) < 0) + if (time_before(fl->expires, fl->lastuse + expires)) fl->expires = fl->lastuse + expires; return 0; } @@ -609,8 +613,6 @@ void ip6_flowlabel_init() { - init_timer(&ip6_fl_gc_timer); - ip6_fl_gc_timer.function = ip6_fl_gc; #ifdef CONFIG_PROC_FS create_proc_read_entry("net/ip6_flowlabel", 0, 0, ip6_fl_read_proc, NULL); #endif
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |