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

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

[Jarek Poplawski]

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

Let's delete timer reliably in sfq_destroy().


Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

---

 net/sched/sch_sfq.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index a20e2ef..f0463d7 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -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)

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

[Patrick McHardy]

Jarek Poplawski wrote:
> [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.

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

[Jarek Poplawski]

On Sun, Apr 27, 2008 at 08:24:34PM +0200, Patrick McHardy wrote:
> Jarek Poplawski wrote:
>> [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.

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

[Patrick McHardy]

Jarek Poplawski wrote:
> On Sun, Apr 27, 2008 at 08:24:34PM +0200, Patrick McHardy wrote:
>   
>> Jarek Poplawski wrote:
>>     
>>> [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?
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.

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

[David Miller]

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Sun, 27 Apr 2008 14:59:21 +0200

> [NET_SCHED] sch_sfq: use del_timer_sync() in sfq_destroy()
> 
> Let's delete timer reliably in sfq_destroy().
> 
> 
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

Applied, thanks.