* [net PATCH v2] net: sched: lock imbalance in hhf qdisc
@ 2014-05-01 16:23 John Fastabend
2014-05-01 17:12 ` Eric Dumazet
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: John Fastabend @ 2014-05-01 16:23 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, edumazet, vtlam
hhf_change() takes the sch_tree_lock and releases it but misses the
error cases. Fix the missed case here.
To reproduce try a command like this,
# tc qdisc change dev p3p2 root hhf quantum 40960 non_hh_weight 300000
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
net/sched/sch_hhf.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
index edee03d..6e957c3 100644
--- a/net/sched/sch_hhf.c
+++ b/net/sched/sch_hhf.c
@@ -553,11 +553,6 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
if (err < 0)
return err;
- sch_tree_lock(sch);
-
- if (tb[TCA_HHF_BACKLOG_LIMIT])
- sch->limit = nla_get_u32(tb[TCA_HHF_BACKLOG_LIMIT]);
-
if (tb[TCA_HHF_QUANTUM])
new_quantum = nla_get_u32(tb[TCA_HHF_QUANTUM]);
@@ -567,6 +562,12 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
non_hh_quantum = (u64)new_quantum * new_hhf_non_hh_weight;
if (non_hh_quantum > INT_MAX)
return -EINVAL;
+
+ sch_tree_lock(sch);
+
+ if (tb[TCA_HHF_BACKLOG_LIMIT])
+ sch->limit = nla_get_u32(tb[TCA_HHF_BACKLOG_LIMIT]);
+
q->quantum = new_quantum;
q->hhf_non_hh_weight = new_hhf_non_hh_weight;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [net PATCH v2] net: sched: lock imbalance in hhf qdisc
2014-05-01 16:23 [net PATCH v2] net: sched: lock imbalance in hhf qdisc John Fastabend
@ 2014-05-01 17:12 ` Eric Dumazet
2014-05-02 8:26 ` Terry Lam
2014-05-04 23:42 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2014-05-01 17:12 UTC (permalink / raw)
To: John Fastabend; +Cc: netdev, edumazet, vtlam
On Thu, 2014-05-01 at 09:23 -0700, John Fastabend wrote:
> hhf_change() takes the sch_tree_lock and releases it but misses the
> error cases. Fix the missed case here.
>
> To reproduce try a command like this,
>
> # tc qdisc change dev p3p2 root hhf quantum 40960 non_hh_weight 300000
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
> net/sched/sch_hhf.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
Signed-off-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [net PATCH v2] net: sched: lock imbalance in hhf qdisc
2014-05-01 16:23 [net PATCH v2] net: sched: lock imbalance in hhf qdisc John Fastabend
2014-05-01 17:12 ` Eric Dumazet
@ 2014-05-02 8:26 ` Terry Lam
2014-05-04 23:42 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Terry Lam @ 2014-05-02 8:26 UTC (permalink / raw)
To: John Fastabend; +Cc: Eric Dumazet, Linux Netdev List, Eric Dumazet
Thanks a lot for the fix, John and Eric!
Regards,
Terry
On Thu, May 1, 2014 at 9:23 AM, John Fastabend <john.fastabend@gmail.com> wrote:
> hhf_change() takes the sch_tree_lock and releases it but misses the
> error cases. Fix the missed case here.
>
> To reproduce try a command like this,
>
> # tc qdisc change dev p3p2 root hhf quantum 40960 non_hh_weight 300000
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
> net/sched/sch_hhf.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
> index edee03d..6e957c3 100644
> --- a/net/sched/sch_hhf.c
> +++ b/net/sched/sch_hhf.c
> @@ -553,11 +553,6 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
> if (err < 0)
> return err;
>
> - sch_tree_lock(sch);
> -
> - if (tb[TCA_HHF_BACKLOG_LIMIT])
> - sch->limit = nla_get_u32(tb[TCA_HHF_BACKLOG_LIMIT]);
> -
> if (tb[TCA_HHF_QUANTUM])
> new_quantum = nla_get_u32(tb[TCA_HHF_QUANTUM]);
>
> @@ -567,6 +562,12 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
> non_hh_quantum = (u64)new_quantum * new_hhf_non_hh_weight;
> if (non_hh_quantum > INT_MAX)
> return -EINVAL;
> +
> + sch_tree_lock(sch);
> +
> + if (tb[TCA_HHF_BACKLOG_LIMIT])
> + sch->limit = nla_get_u32(tb[TCA_HHF_BACKLOG_LIMIT]);
> +
> q->quantum = new_quantum;
> q->hhf_non_hh_weight = new_hhf_non_hh_weight;
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [net PATCH v2] net: sched: lock imbalance in hhf qdisc
2014-05-01 16:23 [net PATCH v2] net: sched: lock imbalance in hhf qdisc John Fastabend
2014-05-01 17:12 ` Eric Dumazet
2014-05-02 8:26 ` Terry Lam
@ 2014-05-04 23:42 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2014-05-04 23:42 UTC (permalink / raw)
To: john.fastabend; +Cc: eric.dumazet, netdev, edumazet, vtlam
From: John Fastabend <john.fastabend@gmail.com>
Date: Thu, 01 May 2014 09:23:06 -0700
> hhf_change() takes the sch_tree_lock and releases it but misses the
> error cases. Fix the missed case here.
>
> To reproduce try a command like this,
>
> # tc qdisc change dev p3p2 root hhf quantum 40960 non_hh_weight 300000
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-05-04 23:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-01 16:23 [net PATCH v2] net: sched: lock imbalance in hhf qdisc John Fastabend
2014-05-01 17:12 ` Eric Dumazet
2014-05-02 8:26 ` Terry Lam
2014-05-04 23:42 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).