Skip to contentSkip to conversation

sch_sfq: use del_timer_sync() in sfq_destroy()

PATCH Applied Jarek Poplawski 3 participants 5 messages Apr 27, 2008 – Apr 29, 2008

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sch_sfq: use del_timer_sync() in sfq_destroy()

[NET_SCHED] sch_sfq: use del_timer_sync() in sfq_destroy()

Let's delete timer reliably in sfq_destroy().

---

 net/sched/sch_sfq.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
net/sched/sch_sfq.c +2 -1
index a20e2ef..f0463d7 100644
@@ -521,7 +521,8 @@ static void sfq_destroy(struct Qdisc *sch)
 	struct sfq_sched_data *q = qdisc_priv(sch);
 
 	tcf_destroy_chain(q->filter_list);
-	del_timer(&q->perturb_timer);
+	q->perturb_period = 0;
+	del_timer_sync(&q->perturb_timer);
 }
 
 static int sfq_dump(struct Qdisc *sch, struct sk_buff *skb)
#1 lore raw
Quoted 3 lines · Jarek Poplawski

[NET_SCHED] sch_sfq: use del_timer_sync() in sfq_destroy()

Let's delete timer reliably in sfq_destroy().

This is broken, we hold dev->queue_lock in ->destroy.

#2 lore raw
Quoted 7 lines · Patrick McHardy
Quoted 3 lines · Jarek Poplawski

[NET_SCHED] sch_sfq: use del_timer_sync() in sfq_destroy()

Let's delete timer reliably in sfq_destroy().

This is broken, we hold dev->queue_lock in ->destroy.

But sfq timer (sfq_perturbation()) doesn't use this timer... Could you explain your point more?

Thanks,
Jarek P.

#3 lore raw
Quoted 6 lines · Jarek Poplawski
Quoted 4 lines · Patrick McHardy
Quoted 2 lines · Jarek Poplawski

[NET_SCHED] sch_sfq: use del_timer_sync() in sfq_destroy()

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

Let's delete timer reliably in sfq_destroy().

This is broken, we hold dev->queue_lock in ->destroy.

But sfq timer (sfq_perturbation()) doesn't use this timer... Could you explain your point more?

My point was that it might sleep and can thus not be called in atomic context. This was a misunderstanding on my side though and your patch is fine.