David S. Miller wrote:
From: Ben Greear <[EMAIL PROTECTED]>
Date: Wed, 07 Sep 2005 14:14:42 -0700
David S. Miller wrote:
Doing an add_timer() for something already scheduled it fine.
Does it actually cause a second timer to fire? If not, this
would keep us from correctly decrementing the reference count.
Good point, it doesn't. That could lose a reference.
That sounds good to me, but how would the original code handle it?
Ie, when you call add_timer while the timer is already added,
did it choose the shortest time, or the longest?
In this case the existing timeout is used.
I believe the patch I sent will preserve the original behaviour
with regard to when the timers fire.
It might be safest to put the fix in with the old behaviour,
and then add the optimization for possibly shortening the timer
to the mm tree (or wheverer new code goes to get tested widely).
New patch is attached, w/out the linux-c stuff.
Thanks,
Ben
--
Ben Greear <[EMAIL PROTECTED]>
Candela Technologies Inc http://www.candelatech.com
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -138,6 +138,7 @@ struct neighbour
int (*output)(struct sk_buff *skb);
struct sk_buff_head arp_queue;
struct timer_list timer;
+ u32 in_timer; /* boolean, space for more flags as needed. */
struct neigh_ops *ops;
u8 primary_key[0];
};
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -153,11 +153,34 @@ static int neigh_forced_gc(struct neigh_
return shrunk;
}
-static int neigh_del_timer(struct neighbour *n)
-{
- if ((n->nud_state & NUD_IN_TIMER) &&
- del_timer(&n->timer)) {
- neigh_release(n);
+static int neigh_del_timer(struct neighbour *n) {
+ if (n->in_timer) {
+ if (!(n->nud_state & NUD_IN_TIMER)) {
+ printk("ERROR: in_timer true, but NUD_IN_TIMER false, neigh: %p\n", n);
+ /* dump_stack(); */
+ }
+ if (del_timer(&n->timer)) {
+ n->in_timer = 0;
+ neigh_release(n);
+ return 1;
+ }
+ }
+ return 0;
+}
+
+static int neigh_add_timer(struct neighbour *n, u32 expires) {
+ if (n->in_timer) {
+ /* This hits fairly often, not too sure why/how. */
+ /* TODO: Maybe should remove existing timer if it expires
+ * later than this one and then insert the new timer? --Ben*/
+ NEIGH_PRINTK2("ERROR: trying to add timer, but already there, neigh: %p\n", n);
+ /* dump_stack(); */
+ }
+ else {
+ n->timer.expires = expires;
+ neigh_hold(n);
+ add_timer(&n->timer);
+ n->in_timer = 1;
return 1;
}
return 0;
@@ -737,6 +760,9 @@ static void neigh_timer_handler(unsigned
write_lock(&neigh->lock);
+ /* We are out of the timer list at this point. */
+ neigh->in_timer = 0;
+
state = neigh->nud_state;
now = jiffies;
next = now + HZ;
@@ -806,11 +832,9 @@ static void neigh_timer_handler(unsigned
}
if (neigh->nud_state & NUD_IN_TIMER) {
- neigh_hold(neigh);
if (time_before(next, jiffies + HZ/2))
next = jiffies + HZ/2;
- neigh->timer.expires = next;
- add_timer(&neigh->timer);
+ neigh_add_timer(neigh, next);
}
if (neigh->nud_state & (NUD_INCOMPLETE | NUD_PROBE)) {
struct sk_buff *skb = skb_peek(&neigh->arp_queue);
@@ -851,9 +875,7 @@ int __neigh_event_send(struct neighbour
if (neigh->parms->mcast_probes + neigh->parms->app_probes) {
atomic_set(&neigh->probes, neigh->parms->ucast_probes);
neigh->nud_state = NUD_INCOMPLETE;
- neigh_hold(neigh);
- neigh->timer.expires = now + 1;
- add_timer(&neigh->timer);
+ neigh_add_timer(neigh, now + 1);
} else {
neigh->nud_state = NUD_FAILED;
write_unlock_bh(&neigh->lock);
@@ -864,10 +886,8 @@ int __neigh_event_send(struct neighbour
}
} else if (neigh->nud_state & NUD_STALE) {
NEIGH_PRINTK2("neigh %p is delayed.\n", neigh);
- neigh_hold(neigh);
neigh->nud_state = NUD_DELAY;
- neigh->timer.expires = jiffies + neigh->parms->delay_probe_time;
- add_timer(&neigh->timer);
+ neigh_add_timer(neigh, jiffies + neigh->parms->delay_probe_time);
}
if (neigh->nud_state == NUD_INCOMPLETE) {
@@ -1012,11 +1032,9 @@ int neigh_update(struct neighbour *neigh
if (new != old) {
neigh_del_timer(neigh);
if (new & NUD_IN_TIMER) {
- neigh_hold(neigh);
- neigh->timer.expires = jiffies +
+ neigh_add_timer(neigh, jiffies +
((new & NUD_REACHABLE) ?
- neigh->parms->reachable_time : 0);
- add_timer(&neigh->timer);
+ neigh->parms->reachable_time : 0));
}
neigh->nud_state = new;
}
