* [PATCH] Fix byte stats on eql @ 2012-03-25 10:01 David Woodhouse 2012-03-25 17:28 ` David Miller 2012-03-26 7:29 ` [PATCH] eql: Fix byte stats David Woodhouse 0 siblings, 2 replies; 7+ messages in thread From: David Woodhouse @ 2012-03-25 10:01 UTC (permalink / raw) To: netdev [-- Attachment #1: Type: text/plain, Size: 648 bytes --] Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> diff --git a/drivers/net/eql.c b/drivers/net/eql.c index a59cf96..f2d3741 100644 --- a/drivers/net/eql.c +++ b/drivers/net/eql.c @@ -345,6 +345,7 @@ static netdev_tx_t eql_slave_xmit(struct sk_buff *skb, struct net_device *dev) slave->bytes_queued += skb->len; dev_queue_xmit(skb); dev->stats.tx_packets++; + dev->stats.tx_bytes += skb->len; } else { dev->stats.tx_dropped++; dev_kfree_skb(skb); -- David Woodhouse Open Source Technology Centre David.Woodhouse@intel.com Intel Corporation [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5818 bytes --] ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix byte stats on eql 2012-03-25 10:01 [PATCH] Fix byte stats on eql David Woodhouse @ 2012-03-25 17:28 ` David Miller 2012-03-26 7:29 ` [PATCH] eql: Fix byte stats David Woodhouse 1 sibling, 0 replies; 7+ messages in thread From: David Miller @ 2012-03-25 17:28 UTC (permalink / raw) To: dwmw2; +Cc: netdev From: David Woodhouse <dwmw2@infradead.org> Date: Sun, 25 Mar 2012 11:01:40 +0100 > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> Please use a driver/subsystem prefix in your Subject lines, in this case "eql: Fix byte stats." would have been appropriate. > @@ -345,6 +345,7 @@ static netdev_tx_t eql_slave_xmit(struct sk_buff > *skb, struct net_device *dev) Patch corrupted by your email client. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] eql: Fix byte stats 2012-03-25 10:01 [PATCH] Fix byte stats on eql David Woodhouse 2012-03-25 17:28 ` David Miller @ 2012-03-26 7:29 ` David Woodhouse 2012-03-27 8:12 ` Eric Dumazet 1 sibling, 1 reply; 7+ messages in thread From: David Woodhouse @ 2012-03-26 7:29 UTC (permalink / raw) To: netdev --- Sorry, forgot to tell Evolution not to word-wrap last time. It's a manual process — insert the patch from a text file, and set it to 'Preformatted'. I've also disabled the signature on this version; I'm not entirely sure if the git-am bug with QP got fixed yet. diff --git a/drivers/net/eql.c b/drivers/net/eql.c index a59cf96..f2d3741 100644 --- a/drivers/net/eql.c +++ b/drivers/net/eql.c @@ -345,6 +345,7 @@ static netdev_tx_t eql_slave_xmit(struct sk_buff *skb, struct net_device *dev) slave->bytes_queued += skb->len; dev_queue_xmit(skb); dev->stats.tx_packets++; + dev->stats.tx_bytes += skb->len; } else { dev->stats.tx_dropped++; dev_kfree_skb(skb); -- dwmw2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] eql: Fix byte stats 2012-03-26 7:29 ` [PATCH] eql: Fix byte stats David Woodhouse @ 2012-03-27 8:12 ` Eric Dumazet 2012-03-27 8:17 ` David Woodhouse 0 siblings, 1 reply; 7+ messages in thread From: Eric Dumazet @ 2012-03-27 8:12 UTC (permalink / raw) To: David Woodhouse; +Cc: netdev Le lundi 26 mars 2012 à 08:29 +0100, David Woodhouse a écrit : Missing changelog and "Signed-off-by: ..." > --- > Sorry, forgot to tell Evolution not to word-wrap last time. It's a > manual process — insert the patch from a text file, and set it to > 'Preformatted'. I've also disabled the signature on this version; I'm > not entirely sure if the git-am bug with QP got fixed yet. > I use Evolution too, its not that bad :) > diff --git a/drivers/net/eql.c b/drivers/net/eql.c > index a59cf96..f2d3741 100644 > --- a/drivers/net/eql.c > +++ b/drivers/net/eql.c > @@ -345,6 +345,7 @@ static netdev_tx_t eql_slave_xmit(struct sk_buff *skb, struct net_device *dev) > slave->bytes_queued += skb->len; > dev_queue_xmit(skb); > dev->stats.tx_packets++; > + dev->stats.tx_bytes += skb->len; > } else { > dev->stats.tx_dropped++; > dev_kfree_skb(skb); > That adds a bug unfortunately. After dev_queue_xmit(skb) call, you cant safely dereference skb anymore. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] eql: Fix byte stats 2012-03-27 8:12 ` Eric Dumazet @ 2012-03-27 8:17 ` David Woodhouse 2012-03-27 8:52 ` [PATCH] eql: dont rely on HZ=100 Eric Dumazet 0 siblings, 1 reply; 7+ messages in thread From: David Woodhouse @ 2012-03-27 8:17 UTC (permalink / raw) To: Eric Dumazet; +Cc: netdev [-- Attachment #1: Type: text/plain, Size: 671 bytes --] On Tue, 2012-03-27 at 01:12 -0700, Eric Dumazet wrote: > Le lundi 26 mars 2012 à 08:29 +0100, David Woodhouse a écrit : > > Missing changelog and "Signed-off-by: ..." The changelog was in the subject line; there's nothing more to say. The lack of Signed-off-by is pure stupidity on my part. The baby ate my brain. That's my excuse and I'm sticking to it. > That adds a bug unfortunately. > > After dev_queue_xmit(skb) call, you cant safely dereference skb anymore. Thanks. I blame the baby for that one too. Once she's been taken swimming and dropped off at nursery and I've had at least two more cups of tea, I'll have another go :) -- dwmw2 [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5818 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] eql: dont rely on HZ=100 2012-03-27 8:17 ` David Woodhouse @ 2012-03-27 8:52 ` Eric Dumazet 2012-03-28 2:47 ` David Miller 0 siblings, 1 reply; 7+ messages in thread From: Eric Dumazet @ 2012-03-27 8:52 UTC (permalink / raw) To: David Woodhouse, David Miller; +Cc: netdev Le mardi 27 mars 2012 à 09:17 +0100, David Woodhouse a écrit : > Thanks. I blame the baby for that one too. Once she's been taken > swimming and dropped off at nursery and I've had at least two more cups > of tea, I'll have another go :) > Looking at this driver, it seems it depends on HZ=100 ? [PATCH] eql: dont rely on HZ=100 HZ is more likely to be 1000 these days. timer handlers are run from softirq, no need to disable bh skb priority 1 is TC_PRIO_FILLER Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- drivers/net/eql.c | 7 ++++--- include/linux/if_eql.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/eql.c b/drivers/net/eql.c index a59cf96..f219d38 100644 --- a/drivers/net/eql.c +++ b/drivers/net/eql.c @@ -125,6 +125,7 @@ #include <linux/if.h> #include <linux/if_arp.h> #include <linux/if_eql.h> +#include <linux/pkt_sched.h> #include <asm/uaccess.h> @@ -143,7 +144,7 @@ static void eql_timer(unsigned long param) equalizer_t *eql = (equalizer_t *) param; struct list_head *this, *tmp, *head; - spin_lock_bh(&eql->queue.lock); + spin_lock(&eql->queue.lock); head = &eql->queue.all_slaves; list_for_each_safe(this, tmp, head) { slave_t *slave = list_entry(this, slave_t, list); @@ -157,7 +158,7 @@ static void eql_timer(unsigned long param) } } - spin_unlock_bh(&eql->queue.lock); + spin_unlock(&eql->queue.lock); eql->timer.expires = jiffies + EQL_DEFAULT_RESCHED_IVAL; add_timer(&eql->timer); @@ -341,7 +342,7 @@ static netdev_tx_t eql_slave_xmit(struct sk_buff *skb, struct net_device *dev) struct net_device *slave_dev = slave->dev; skb->dev = slave_dev; - skb->priority = 1; + skb->priority = TC_PRIO_FILLER; slave->bytes_queued += skb->len; dev_queue_xmit(skb); dev->stats.tx_packets++; diff --git a/include/linux/if_eql.h b/include/linux/if_eql.h index 79c4f26..18a5d02 100644 --- a/include/linux/if_eql.h +++ b/include/linux/if_eql.h @@ -22,7 +22,7 @@ #define EQL_DEFAULT_SLAVE_PRIORITY 28800 #define EQL_DEFAULT_MAX_SLAVES 4 #define EQL_DEFAULT_MTU 576 -#define EQL_DEFAULT_RESCHED_IVAL 100 +#define EQL_DEFAULT_RESCHED_IVAL HZ #define EQL_ENSLAVE (SIOCDEVPRIVATE) #define EQL_EMANCIPATE (SIOCDEVPRIVATE + 1) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] eql: dont rely on HZ=100 2012-03-27 8:52 ` [PATCH] eql: dont rely on HZ=100 Eric Dumazet @ 2012-03-28 2:47 ` David Miller 0 siblings, 0 replies; 7+ messages in thread From: David Miller @ 2012-03-28 2:47 UTC (permalink / raw) To: eric.dumazet; +Cc: dwmw2, netdev From: Eric Dumazet <eric.dumazet@gmail.com> Date: Tue, 27 Mar 2012 01:52:00 -0700 > [PATCH] eql: dont rely on HZ=100 > > HZ is more likely to be 1000 these days. > > timer handlers are run from softirq, no need to disable bh > > skb priority 1 is TC_PRIO_FILLER > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Applied. Strange we export this EQL_DEFAULT_RESCHED_IVAL to userspace, where HZ will take on a different value I think, something akin to USER_HZ. Anyways, that's a different unrelated problem to the bug you're fixing. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-03-28 2:48 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-25 10:01 [PATCH] Fix byte stats on eql David Woodhouse 2012-03-25 17:28 ` David Miller 2012-03-26 7:29 ` [PATCH] eql: Fix byte stats David Woodhouse 2012-03-27 8:12 ` Eric Dumazet 2012-03-27 8:17 ` David Woodhouse 2012-03-27 8:52 ` [PATCH] eql: dont rely on HZ=100 Eric Dumazet 2012-03-28 2:47 ` 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).