pkt_sched: Remove smp_wmb() in qdisc_watchdog()
Subsystems (4)
RFC PATCH 1/1 pkt_sched: Remove smp_wmb() in qdisc_watchdog()
Hi,
I hope Stephen will find a minute to reconsider and/or comment this a bit.
Thanks,
Jarek P.
---------->
While implementing a TCQ_F_THROTTLED flag there was used an smp_wmb() in qdisc_watchdog(), but since this flag is practically used only in sch_netem(), and since it's not even clear what reordering is avoided here (TCQ_F_THROTTLED vs. __QDISC_STATE_SCHED?) it seems the barrier could be safely removed.
---
net/sched/sch_api.c | 1 - net/sched/sch_netem.c | 1 - 2 files changed, 0 insertions(+), 2 deletions(-)
net/sched/sch_api.c +0 -1
@@ -450,7 +450,6 @@ static enum hrtimer_restart qdisc_watchdog(struct hrtimer *timer) timer); wd->qdisc->flags &= ~TCQ_F_THROTTLED ; -smp_wmb ();__netif_schedule (qdisc_root (wd->qdisc));return HRTIMER_NORESTART ;
net/sched/sch_netem.c +0 -1
@@ -266,7 +266,6 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)struct netem_sched_data *q =qdisc_priv (sch);struct sk_buff *skb; -smp_mb ();if (sch->flags &TCQ_F_THROTTLED )return NULL ;
On Fri, Dec 19, 2008 at 11:30:47AM +0000, Jarek Poplawski wrote:
...
Quoted 3 lines
While implementing a
TCQ_F_THROTTLEDflag there was used ansmp_wmb()inqdisc_watchdog(), but since this flag is practically used only insch_netem(), and since it's not even clear what reordering is avoided
Hmm.. I forgot to mention sch_cbq(), but it doesn't use a barrier anyway.
Jarek P.
Quoted 10 lines · On Fri, 19 Dec 2008 11:41:25 +0000 Jarek Poplawski <[email protected]> wrote:
On Fri, Dec 19, 2008 at 11:30:47AM +0000, Jarek Poplawski wrote:
...Quoted 3 lines
While implementing a
TCQ_F_THROTTLEDflag there was used ansmp_wmb()inqdisc_watchdog(), but since this flag is practically used only insch_netem(), and since it's not even clear what reordering is avoidedHmm.. I forgot to mention
sch_cbq(), but it doesn't use a barrier anyway.Jarek P.
The barrier was copied from earlier version of tbf is unnecessary.
PATCH v2 1/1 pkt_sched: Remove smp_wmb() in qdisc_watchdog()
On Fri, Dec 19, 2008 at 06:18:51PM -0800, Stephen Hemminger wrote:
...
Quoted 1 line
The barrier was copied from earlier version of tbf is unnecessary.
So I resend this patch with only a changelog updated
Thanks,
Jarek P.
---------->
While implementing a TCQ_F_THROTTLED flag there was used an smp_wmb() in qdisc_watchdog(). Stephen Hemminger acknowledged the barrier was copied from earlier version of tbf and is unnecessaray, so it's removed here.
---
net/sched/sch_api.c | 1 - net/sched/sch_netem.c | 1 - 2 files changed, 0 insertions(+), 2 deletions(-)
net/sched/sch_api.c +0 -1
@@ -450,7 +450,6 @@ static enum hrtimer_restart qdisc_watchdog(struct hrtimer *timer) timer); wd->qdisc->flags &= ~TCQ_F_THROTTLED ; -smp_wmb ();__netif_schedule (qdisc_root (wd->qdisc));return HRTIMER_NORESTART ;
net/sched/sch_netem.c +0 -1
@@ -266,7 +266,6 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)struct netem_sched_data *q =qdisc_priv (sch);struct sk_buff *skb; -smp_mb ();if (sch->flags &TCQ_F_THROTTLED )return NULL ;
From: Jarek Poplawski <[email protected]>
Date: Fri, 19 Dec 2008 11:30:47 +0000
Quoted 7 lines
While implementing a
TCQ_F_THROTTLEDflag there was used ansmp_wmb()inqdisc_watchdog(), but since this flag is practically used only insch_netem(), and since it's not even clear what reordering is avoided here (TCQ_F_THROTTLEDvs.__QDISC_STATE_SCHED?) it seems the barrier could be safely removed.Signed-off-by Jarek Poplawski <[email protected]>
Applied to net-next-2.6, thanks Jarek.