* [PATCH] sfq: deadlock in error path
@ 2011-02-03 1:19 Stephen Hemminger
2011-02-03 1:51 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: Stephen Hemminger @ 2011-02-03 1:19 UTC (permalink / raw)
To: David Miller, Eric Dumazet; +Cc: netdev
The change to allow divisor to be a parameter (in 2.6.38-rc1)
commit 817fb15dfd988d8dda916ee04fa506f0c466b9d6
introduced a possible deadlock caught by sparse.
The scheduler tree lock was left locked in the case of an incorrect
divisor value. Simplest fix is to move test outside of lock
which also solves problem of partial update.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/sched/sch_sfq.c 2011-02-02 17:12:06.338204106 -0800
+++ b/net/sched/sch_sfq.c 2011-02-02 17:13:39.127205019 -0800
@@ -491,17 +491,18 @@ static int sfq_change(struct Qdisc *sch,
if (opt->nla_len < nla_attr_size(sizeof(*ctl)))
return -EINVAL;
+ if (ctl->divisor &&
+ (!is_power_of_2(ctl->divisor) || ctl->divisor > 65536))
+ return -EINVAL;
+
sch_tree_lock(sch);
q->quantum = ctl->quantum ? : psched_mtu(qdisc_dev(sch));
q->scaled_quantum = SFQ_ALLOT_SIZE(q->quantum);
q->perturb_period = ctl->perturb_period * HZ;
if (ctl->limit)
q->limit = min_t(u32, ctl->limit, SFQ_DEPTH - 1);
- if (ctl->divisor) {
- if (!is_power_of_2(ctl->divisor) || ctl->divisor > 65536)
- return -EINVAL;
+ if (ctl->divisor)
q->divisor = ctl->divisor;
- }
qlen = sch->q.qlen;
while (sch->q.qlen > q->limit)
sfq_drop(sch);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] sfq: deadlock in error path
2011-02-03 1:19 [PATCH] sfq: deadlock in error path Stephen Hemminger
@ 2011-02-03 1:51 ` Eric Dumazet
2011-02-03 4:51 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2011-02-03 1:51 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
Le mercredi 02 février 2011 à 17:19 -0800, Stephen Hemminger a écrit :
> The change to allow divisor to be a parameter (in 2.6.38-rc1)
> commit 817fb15dfd988d8dda916ee04fa506f0c466b9d6
> introduced a possible deadlock caught by sparse.
>
> The scheduler tree lock was left locked in the case of an incorrect
> divisor value. Simplest fix is to move test outside of lock
> which also solves problem of partial update.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Oops, thanks Stephen !
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
But please, please, please, please, please, remove dada1@cosmosbay.com,
as this address is no longer usable for me !
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] sfq: deadlock in error path
2011-02-03 1:51 ` Eric Dumazet
@ 2011-02-03 4:51 ` David Miller
0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2011-02-03 4:51 UTC (permalink / raw)
To: eric.dumazet; +Cc: shemminger, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 03 Feb 2011 02:51:02 +0100
> Le mercredi 02 février 2011 à 17:19 -0800, Stephen Hemminger a écrit :
>> The change to allow divisor to be a parameter (in 2.6.38-rc1)
>> commit 817fb15dfd988d8dda916ee04fa506f0c466b9d6
>> introduced a possible deadlock caught by sparse.
>>
>> The scheduler tree lock was left locked in the case of an incorrect
>> divisor value. Simplest fix is to move test outside of lock
>> which also solves problem of partial update.
>>
>> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> Oops, thanks Stephen !
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-02-03 4:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-03 1:19 [PATCH] sfq: deadlock in error path Stephen Hemminger
2011-02-03 1:51 ` Eric Dumazet
2011-02-03 4:51 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.