* [PATCH] drop_monitor: make last_rx timestamp private
@ 2009-08-31 19:58 Neil Horman
2009-09-02 1:21 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Neil Horman @ 2009-08-31 19:58 UTC (permalink / raw)
To: netdev; +Cc: davem, nhorman, eric.dumazet
Hey all-
It was recently pointed out to me that the last_rx field of the net_device
structure wasn't updated regularly. In fact only the bonding driver really uses
it currently. Since the drop_monitor code relies on the last_rx field to detect
drops on recevie in hardware, We need to find a more reliable way to rate limit
our drop checks (so that we don't check for drops on every frame recevied, which
would be inefficient. This patch makes a last_rx timestamp that is private to
the drop monitor code and is updated for every device that we track.
Neil
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
drop_monitor.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 9d66fa9..34a05ce 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -52,6 +52,7 @@ struct per_cpu_dm_data {
struct dm_hw_stat_delta {
struct net_device *dev;
+ unsigned long last_rx;
struct list_head list;
struct rcu_head rcu;
unsigned long last_drop_val;
@@ -179,18 +180,21 @@ static void trace_napi_poll_hit(struct napi_struct *napi)
{
struct dm_hw_stat_delta *new_stat;
- /*
- * Ratelimit our check time to dm_hw_check_delta jiffies
- */
- if (!time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
- return;
rcu_read_lock();
list_for_each_entry_rcu(new_stat, &hw_stats_list, list) {
+ /*
+ * only add a note to our monitor buffer if:
+ * 1) this is the dev we received on
+ * 2) its after the last_rx delta
+ * 3) our rx_dropped count has gone up
+ */
if ((new_stat->dev == napi->dev) &&
+ (time_after(jiffies, new_stat->last_rx + dm_hw_check_delta)) &&
(napi->dev->stats.rx_dropped != new_stat->last_drop_val)) {
trace_drop_common(NULL, NULL);
new_stat->last_drop_val = napi->dev->stats.rx_dropped;
+ new_stat->last_rx = jiffies;
break;
}
}
@@ -286,6 +290,7 @@ static int dropmon_net_event(struct notifier_block *ev_block,
goto out;
new_stat->dev = dev;
+ new_stat->last_rx = jiffies;
INIT_RCU_HEAD(&new_stat->rcu);
spin_lock(&trace_state_lock);
list_add_rcu(&new_stat->list, &hw_stats_list);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drop_monitor: make last_rx timestamp private
2009-08-31 19:58 [PATCH] drop_monitor: make last_rx timestamp private Neil Horman
@ 2009-09-02 1:21 ` David Miller
2009-09-02 10:55 ` Neil Horman
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2009-09-02 1:21 UTC (permalink / raw)
To: nhorman; +Cc: netdev, eric.dumazet
From: Neil Horman <nhorman@tuxdriver.com>
Date: Mon, 31 Aug 2009 15:58:47 -0400
> It was recently pointed out to me that the last_rx field of the net_device
> structure wasn't updated regularly. In fact only the bonding driver really uses
> it currently. Since the drop_monitor code relies on the last_rx field to detect
> drops on recevie in hardware, We need to find a more reliable way to rate limit
> our drop checks (so that we don't check for drops on every frame recevied, which
> would be inefficient. This patch makes a last_rx timestamp that is private to
> the drop monitor code and is updated for every device that we track.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Neil, this doesn't apply to net-next-2.6:
> diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
> index 9d66fa9..34a05ce 100644
> --- a/net/core/drop_monitor.c
> +++ b/net/core/drop_monitor.c
...
> @@ -179,18 +180,21 @@ static void trace_napi_poll_hit(struct napi_struct *napi)
> {
> struct dm_hw_stat_delta *new_stat;
>
> - /*
> - * Ratelimit our check time to dm_hw_check_delta jiffies
> - */
> - if (!time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
> - return;
>
> rcu_read_lock();
> list_for_each_entry_rcu(new_stat, &hw_stats_list, list) {
In net-next-2.6 this test reads:
/*
* Ratelimit our check time to dm_hw_check_delta jiffies
*/
if (!napi->dev ||
!time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
return;
and you must retain the napi->dev NULL check there as otherwise
the list traversal tests will blindly dereference that pointer.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drop_monitor: make last_rx timestamp private
2009-09-02 1:21 ` David Miller
@ 2009-09-02 10:55 ` Neil Horman
2009-09-02 13:36 ` Neil Horman
0 siblings, 1 reply; 5+ messages in thread
From: Neil Horman @ 2009-09-02 10:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev, eric.dumazet
On Tue, Sep 01, 2009 at 06:21:27PM -0700, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Mon, 31 Aug 2009 15:58:47 -0400
>
> > It was recently pointed out to me that the last_rx field of the net_device
> > structure wasn't updated regularly. In fact only the bonding driver really uses
> > it currently. Since the drop_monitor code relies on the last_rx field to detect
> > drops on recevie in hardware, We need to find a more reliable way to rate limit
> > our drop checks (so that we don't check for drops on every frame recevied, which
> > would be inefficient. This patch makes a last_rx timestamp that is private to
> > the drop monitor code and is updated for every device that we track.
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>
> Neil, this doesn't apply to net-next-2.6:
>
> > diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
> > index 9d66fa9..34a05ce 100644
> > --- a/net/core/drop_monitor.c
> > +++ b/net/core/drop_monitor.c
> ...
> > @@ -179,18 +180,21 @@ static void trace_napi_poll_hit(struct napi_struct *napi)
> > {
> > struct dm_hw_stat_delta *new_stat;
> >
> > - /*
> > - * Ratelimit our check time to dm_hw_check_delta jiffies
> > - */
> > - if (!time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
> > - return;
> >
> > rcu_read_lock();
> > list_for_each_entry_rcu(new_stat, &hw_stats_list, list) {
>
> In net-next-2.6 this test reads:
>
> /*
> * Ratelimit our check time to dm_hw_check_delta jiffies
> */
> if (!napi->dev ||
> !time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
> return;
>
> and you must retain the napi->dev NULL check there as otherwise
> the list traversal tests will blindly dereference that pointer.
>
Apologies, this raced with Xiao fix to trace_napi_poll hit, which introduced
that null check. I'll rediff/repost shortly.
Neil
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drop_monitor: make last_rx timestamp private
2009-09-02 10:55 ` Neil Horman
@ 2009-09-02 13:36 ` Neil Horman
2009-09-02 21:37 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Neil Horman @ 2009-09-02 13:36 UTC (permalink / raw)
To: David Miller; +Cc: netdev, eric.dumazet
Ok, heres a repost, unchanged saved for being massaged to fit in with the recent
changes that my origional patch raced against. Thanks!
Neil
It was recently pointed out to me that the last_rx field of the net_device
structure wasn't updated regularly. In fact only the bonding driver really uses
it currently. Since the drop_monitor code relies on the last_rx field to detect
drops on recevie in hardware, We need to find a more reliable way to rate limit
our drop checks (so that we don't check for drops on every frame recevied, which
would be inefficient. This patch makes a last_rx timestamp that is private to
the drop monitor code and is updated for every device that we track.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
drop_monitor.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index d311202..0a113f2 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -52,6 +52,7 @@ struct per_cpu_dm_data {
struct dm_hw_stat_delta {
struct net_device *dev;
+ unsigned long last_rx;
struct list_head list;
struct rcu_head rcu;
unsigned long last_drop_val;
@@ -180,18 +181,25 @@ static void trace_napi_poll_hit(struct napi_struct *napi)
struct dm_hw_stat_delta *new_stat;
/*
- * Ratelimit our check time to dm_hw_check_delta jiffies
+ * Don't check napi structures with no associated device
*/
- if (!napi->dev ||
- !time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
+ if (!napi->dev)
return;
rcu_read_lock();
list_for_each_entry_rcu(new_stat, &hw_stats_list, list) {
+ /*
+ * only add a note to our monitor buffer if:
+ * 1) this is the dev we received on
+ * 2) its after the last_rx delta
+ * 3) our rx_dropped count has gone up
+ */
if ((new_stat->dev == napi->dev) &&
+ (time_after(jiffies, new_stat->last_rx + dm_hw_check_delta)) &&
(napi->dev->stats.rx_dropped != new_stat->last_drop_val)) {
trace_drop_common(NULL, NULL);
new_stat->last_drop_val = napi->dev->stats.rx_dropped;
+ new_stat->last_rx = jiffies;
break;
}
}
@@ -287,6 +295,7 @@ static int dropmon_net_event(struct notifier_block *ev_block,
goto out;
new_stat->dev = dev;
+ new_stat->last_rx = jiffies;
INIT_RCU_HEAD(&new_stat->rcu);
spin_lock(&trace_state_lock);
list_add_rcu(&new_stat->list, &hw_stats_list);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drop_monitor: make last_rx timestamp private
2009-09-02 13:36 ` Neil Horman
@ 2009-09-02 21:37 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2009-09-02 21:37 UTC (permalink / raw)
To: nhorman; +Cc: netdev, eric.dumazet
From: Neil Horman <nhorman@tuxdriver.com>
Date: Wed, 2 Sep 2009 09:36:20 -0400
> Ok, heres a repost, unchanged saved for being massaged to fit in with the recent
> changes that my origional patch raced against. Thanks!
> Neil
>
>
> It was recently pointed out to me that the last_rx field of the net_device
> structure wasn't updated regularly. In fact only the bonding driver really uses
> it currently. Since the drop_monitor code relies on the last_rx field to detect
> drops on recevie in hardware, We need to find a more reliable way to rate limit
> our drop checks (so that we don't check for drops on every frame recevied, which
> would be inefficient. This patch makes a last_rx timestamp that is private to
> the drop monitor code and is updated for every device that we track.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks Neil.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-09-02 21:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-31 19:58 [PATCH] drop_monitor: make last_rx timestamp private Neil Horman
2009-09-02 1:21 ` David Miller
2009-09-02 10:55 ` Neil Horman
2009-09-02 13:36 ` Neil Horman
2009-09-02 21:37 ` 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.