[PATCH] sch_generic.c: Make dev_watchdog use deferrable timer

[PATCH] sch_generic.c: Make dev_watchdog use deferrable timer

[Parag Warudkar]

Reduces the number of wakeups from idle per second, makes powertop happy.

Signed-off-by: Parag Warudkar <parag.warudkar@gmail.com>

--- linux-2.6/net/sched/sch_generic.c	2007-12-07 10:04:43.000000000 -0500
+++ linux-2.6-work/net/sched/sch_generic.c	2007-12-16 17:57:05.000000000 -0500
@@ -213,7 +213,7 @@

  static void dev_watchdog_init(struct net_device *dev)
  {
-	init_timer(&dev->watchdog_timer);
+	init_timer_deferrable(&dev->watchdog_timer);
  	dev->watchdog_timer.data = (unsigned long)dev;
  	dev->watchdog_timer.function = dev_watchdog;
  }

[PATCH 1/2] net: flow hash timer power saving

[Stephen Hemminger]

The net flow hash randomization timer runs every 10 minutes.
This time is low priority and doesn't need to wakeup the CPU.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/core/flow.c	2007-12-18 07:43:31.000000000 -0800
+++ b/net/core/flow.c	2007-12-18 07:44:32.000000000 -0800
@@ -81,8 +81,8 @@ static void flow_cache_new_hashrnd(unsig
 	for_each_possible_cpu(i)
 		flow_hash_rnd_recalc(i) = 1;
 
-	flow_hash_rnd_timer.expires = jiffies + FLOW_HASH_RND_PERIOD;
-	add_timer(&flow_hash_rnd_timer);
+	mod_timer(&flow_hash_rnd_timer,
+		  round_jiffies(jiffies + FLOW_HASH_RND_PERIOD));
 }
 
 static void flow_entry_kill(int cpu, struct flow_cache_entry *fle)
@@ -352,7 +352,7 @@ static int __init flow_cache_init(void)
 	flow_lwm = 2 * flow_hash_size;
 	flow_hwm = 4 * flow_hash_size;
 
-	init_timer(&flow_hash_rnd_timer);
+	init_timer_deferrable(&flow_hash_rnd_timer);
 	flow_hash_rnd_timer.function = flow_cache_new_hashrnd;
 	flow_hash_rnd_timer.expires = jiffies + FLOW_HASH_RND_PERIOD;
 	add_timer(&flow_hash_rnd_timer);

[PATCH 2/2] net: neighbor timer power saving

[Stephen Hemminger]

The neighbor GC timer runs once a second, but it doesn't need to wake
up the machine.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/core/neighbour.c	2007-12-18 07:46:07.000000000 -0800
+++ b/net/core/neighbour.c	2007-12-18 07:47:36.000000000 -0800
@@ -270,7 +270,7 @@ static struct neighbour *neigh_alloc(str
 	n->nud_state	  = NUD_NONE;
 	n->output	  = neigh_blackhole;
 	n->parms	  = neigh_parms_clone(&tbl->parms);
-	init_timer(&n->timer);
+	init_timer_deferrable(&n->timer);
 	n->timer.function = neigh_timer_handler;
 	n->timer.data	  = (unsigned long)n;
 
@@ -740,7 +740,7 @@ static void neigh_timer_handler(unsigned
 
 	state = neigh->nud_state;
 	now = jiffies;
-	next = now + HZ;
+	next = round_jiffies(now + HZ);
 
 	if (!(state & NUD_IN_TIMER)) {
 #ifndef CONFIG_SMP
@@ -1372,7 +1372,7 @@ void neigh_table_init_no_netlink(struct 
 	get_random_bytes(&tbl->hash_rnd, sizeof(tbl->hash_rnd));
 
 	rwlock_init(&tbl->lock);
-	init_timer(&tbl->gc_timer);
+	init_timer_deferrable(&tbl->gc_timer);
 	tbl->gc_timer.data     = (unsigned long)tbl;
 	tbl->gc_timer.function = neigh_periodic_timer;
 	tbl->gc_timer.expires  = now + 1;

Re: [PATCH 2/2] net: neighbor timer power saving

[Eric Dumazet]

Stephen Hemminger a écrit :
> The neighbor GC timer runs once a second, but it doesn't need to wake
> up the machine.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> --- a/net/core/neighbour.c	2007-12-18 07:46:07.000000000 -0800
> +++ b/net/core/neighbour.c	2007-12-18 07:47:36.000000000 -0800
> @@ -270,7 +270,7 @@ static struct neighbour *neigh_alloc(str
>  	n->nud_state	  = NUD_NONE;
>  	n->output	  = neigh_blackhole;
>  	n->parms	  = neigh_parms_clone(&tbl->parms);
> -	init_timer(&n->timer);
> +	init_timer_deferrable(&n->timer);
>  	n->timer.function = neigh_timer_handler;
>  	n->timer.data	  = (unsigned long)n;
>  
> @@ -740,7 +740,7 @@ static void neigh_timer_handler(unsigned
>  
>  	state = neigh->nud_state;
>  	now = jiffies;
> -	next = now + HZ;
> +	next = round_jiffies(now + HZ);
>  
>  	if (!(state & NUD_IN_TIMER)) {
>  #ifndef CONFIG_SMP
> @@ -1372,7 +1372,7 @@ void neigh_table_init_no_netlink(struct 
>  	get_random_bytes(&tbl->hash_rnd, sizeof(tbl->hash_rnd));
>  
>  	rwlock_init(&tbl->lock);
> -	init_timer(&tbl->gc_timer);
> +	init_timer_deferrable(&tbl->gc_timer);
>  	tbl->gc_timer.data     = (unsigned long)tbl;
>  	tbl->gc_timer.function = neigh_periodic_timer;
>  	tbl->gc_timer.expires  = now + 1;

I wonder if this deferrable timer thing is the right way to go.

(like read_mostly thing if you want :) )

We are going to convert 99% timers to deferrable.

Maybe the right move should be to have the reverse attribute, to mark a timer 
as non deferrable...

Also, why use round_jiffies() on a deferrable timer ? That sounds unecessary ?

Re: [PATCH 2/2] net: neighbor timer power saving

[Stephen Hemminger]

On Wed, 19 Dec 2007 08:23:43 +0100
Eric Dumazet <dada1@cosmosbay.com> wrote:

> Stephen Hemminger a écrit :
> > The neighbor GC timer runs once a second, but it doesn't need to wake
> > up the machine.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> > 
> > --- a/net/core/neighbour.c	2007-12-18 07:46:07.000000000 -0800
> > +++ b/net/core/neighbour.c	2007-12-18 07:47:36.000000000 -0800
> > @@ -270,7 +270,7 @@ static struct neighbour *neigh_alloc(str
> >  	n->nud_state	  = NUD_NONE;
> >  	n->output	  = neigh_blackhole;
> >  	n->parms	  = neigh_parms_clone(&tbl->parms);
> > -	init_timer(&n->timer);
> > +	init_timer_deferrable(&n->timer);
> >  	n->timer.function = neigh_timer_handler;
> >  	n->timer.data	  = (unsigned long)n;
> >  
> > @@ -740,7 +740,7 @@ static void neigh_timer_handler(unsigned
> >  
> >  	state = neigh->nud_state;
> >  	now = jiffies;
> > -	next = now + HZ;
> > +	next = round_jiffies(now + HZ);
> >  
> >  	if (!(state & NUD_IN_TIMER)) {
> >  #ifndef CONFIG_SMP
> > @@ -1372,7 +1372,7 @@ void neigh_table_init_no_netlink(struct 
> >  	get_random_bytes(&tbl->hash_rnd, sizeof(tbl->hash_rnd));
> >  
> >  	rwlock_init(&tbl->lock);
> > -	init_timer(&tbl->gc_timer);
> > +	init_timer_deferrable(&tbl->gc_timer);
> >  	tbl->gc_timer.data     = (unsigned long)tbl;
> >  	tbl->gc_timer.function = neigh_periodic_timer;
> >  	tbl->gc_timer.expires  = now + 1;
> 
> I wonder if this deferrable timer thing is the right way to go.
> 
> (like read_mostly thing if you want :) )
> 
> We are going to convert 99% timers to deferrable.
> 
> Maybe the right move should be to have the reverse attribute, to mark a timer 
> as non deferrable...
> 
> Also, why use round_jiffies() on a deferrable timer ? That sounds unecessary ?

Thinking about it more, this looks like a case for just using round_jiffies().
The GC timer needs to run to clean up under DoS attack, and deferring it probably
isn't a good idea.

-- 
Stephen Hemminger <stephen.hemminger@vyatta.com>

Re: [PATCH 2/2] net: neighbor timer power saving

[Parag Warudkar]

On Dec 20, 2007 12:10 PM, Stephen Hemminger
<shemminger@linux-foundation.org> wrote:

> Thinking about it more, this looks like a case for just using round_jiffies().
> The GC timer needs to run to clean up under DoS attack, and deferring it probably
> isn't a good idea.

But what are the chances that a DoSed machine will be idling which
will prevent the GC timer to run? I would think there would be lot of
other activities going on (including non-deferrable timers running)
that will avoid this situation?

Parag

Re: [PATCH 2/2] net: neighbor timer power saving

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Wed, 19 Dec 2007 08:23:43 +0100

> We are going to convert 99% timers to deferrable.
> 
> Maybe the right move should be to have the reverse attribute, to
> mark a timer as non deferrable...

I think we are still in a learning process about what
exactly causes a timer to be converted to deferrable
or not.

It may sound crazy but flipping the switch on %99 of
our kernel timers is not the nicest way to figure it
out :-)

Therefore it might still make sense to keep the deferrable
marker for now, so that we can gradually convert the tree
checking easy case carefully.  This way the breakage is
containted very well.

Then once we have almost all of the timers verified, we
can reverse the meaning and mark the timers that remain
as non-deferrable and kill the deferrable marker.