summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBenjamin Thery <benjamin.thery@bull.net>2008-09-13 01:16:37 +0200
committerDavid S. Miller <davem@davemloft.net>2008-09-13 01:16:37 +0200
commitf262b59becc3f557da6460232abac13706402849 (patch)
tree01ecdd0121791955ffea3afd9bd6e2e3fc7c515d
parentvlan: vlan device not reading gso max size of parent. (diff)
downloadlinux-f262b59becc3f557da6460232abac13706402849.tar.xz
linux-f262b59becc3f557da6460232abac13706402849.zip
net: fix scheduling of dst_gc_task by __dst_free
The dst garbage collector dst_gc_task() may not be scheduled as we expect it to be in __dst_free(). Indeed, when the dst_gc_timer was replaced by the delayed_work dst_gc_work, the mod_timer() call used to schedule the garbage collector at an earlier date was replaced by a schedule_delayed_work() (see commit 86bba269d08f0c545ae76c90b56727f65d62d57f). But, the behaviour of mod_timer() and schedule_delayed_work() is different in the way they handle the delay. mod_timer() stops the timer and re-arm it with the new given delay, whereas schedule_delayed_work() only check if the work is already queued in the workqueue (and queue it (with delay) if it is not) BUT it does NOT take into account the new delay (even if the new delay is earlier in time). schedule_delayed_work() returns 0 if it didn't queue the work, but we don't check the return code in __dst_free(). If I understand the code in __dst_free() correctly, we want dst_gc_task to be queued after DST_GC_INC jiffies if we pass the test (and not in some undetermined time in the future), so I think we should add a call to cancel_delayed_work() before schedule_delayed_work(). Patch below. Or we should at least test the return code of schedule_delayed_work(), and reset the values of dst_garbage.timer_inc and dst_garbage.timer_expires back to their former values if schedule_delayed_work() failed. Otherwise the subsequent calls to __dst_free will test the wrong values and assume wrong thing about when the garbage collector is supposed to be scheduled. dst_gc_task() also calls schedule_delayed_work() without checking its return code (or calling cancel_scheduled_work() first), but it should fine there: dst_gc_task is the routine of the delayed_work, so no dst_gc_work should be pending in the queue when it's running. Signed-off-by: Benjamin Thery <benjamin.thery@bull.net> Acked-by: Eric Dumazet <dada1@cosmosbay.com> Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--net/core/dst.c1
1 files changed, 1 insertions, 0 deletions
diff --git a/net/core/dst.c b/net/core/dst.c
index fe03266130b6..09c1530f4681 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -203,6 +203,7 @@ void __dst_free(struct dst_entry * dst)
if (dst_garbage.timer_inc > DST_GC_INC) {
dst_garbage.timer_inc = DST_GC_INC;
dst_garbage.timer_expires = DST_GC_MIN;
+ cancel_delayed_work(&dst_gc_work);
schedule_delayed_work(&dst_gc_work, dst_garbage.timer_expires);
}
spin_unlock_bh(&dst_garbage.lock);