LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH -rt] timer: upper bound on loops of __run_timers processing
@ 2014-12-30 19:52 Marcelo Tosatti
  2014-12-30 21:05 ` Rik van Riel
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Marcelo Tosatti @ 2014-12-30 19:52 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Steven Rostedt, Clark Williams, Luiz Capitulino, Rik van Riel,
	linux-kernel


Commit "timers: do not raise softirq unconditionally", allows for timer
wheel processing (__run_timers) to be delayed for long periods of time.

The effect is that 

loops = jiffies - base->timer_jiffies

Can grow to very large values resulting in __run_timers taking hundreds
of milliseconds to execute.

Fix by creating an upper bound on the number of loops to be processed.
This allows a nohz=off kernel to achieve desired latencies.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

diff --git a/kernel/timer.c b/kernel/timer.c
index f59e18c..c128416 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1488,6 +1488,12 @@ void run_local_timers(void)
 	}
 #endif
 
+	if (time_after_eq(jiffies, base->timer_jiffies)) {
+		unsigned long jiffies_delta = jiffies - base->timer_jiffies;
+		if (jiffies_delta > TVR_SIZE)
+			raise_softirq(TIMER_SOFTIRQ);
+	}
+
 	if (!base->active_timers)
 		goto out;
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH -rt] timer: upper bound on loops of __run_timers processing
  2014-12-30 19:52 [PATCH -rt] timer: upper bound on loops of __run_timers processing Marcelo Tosatti
@ 2014-12-30 21:05 ` Rik van Riel
  2015-01-14 17:21 ` Marcelo Tosatti
  2015-02-17 23:08 ` Rik van Riel
  2 siblings, 0 replies; 7+ messages in thread
From: Rik van Riel @ 2014-12-30 21:05 UTC (permalink / raw)
  To: Marcelo Tosatti, Thomas Gleixner
  Cc: Steven Rostedt, Clark Williams, Luiz Capitulino, linux-kernel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/30/2014 02:52 PM, Marcelo Tosatti wrote:
> 
> Commit "timers: do not raise softirq unconditionally", allows for
> timer wheel processing (__run_timers) to be delayed for long
> periods of time.
> 
> The effect is that
> 
> loops = jiffies - base->timer_jiffies
> 
> Can grow to very large values resulting in __run_timers taking
> hundreds of milliseconds to execute.
> 
> Fix by creating an upper bound on the number of loops to be
> processed. This allows a nohz=off kernel to achieve desired
> latencies.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> diff --git a/kernel/timer.c b/kernel/timer.c index f59e18c..c128416
> 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -1488,6
> +1488,12 @@ void run_local_timers(void) } #endif
> 
> +	if (time_after_eq(jiffies, base->timer_jiffies)) { +		unsigned
> long jiffies_delta = jiffies - base->timer_jiffies; +		if
> (jiffies_delta > TVR_SIZE) +			raise_softirq(TIMER_SOFTIRQ); +	}

It certainly looks like cascading the timers over and over and
over again could be a lot of work.

Acked-by: Rik van Riel <riel@redhat.com>

- -- 
All rights reversed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJUoxOfAAoJEM553pKExN6DO+0H/i2TXS2DJ6cObGZytYhsRz/U
4oqH4eCjwK5JczFMsyoUh/kXIkEQukMudj74katoP0TARlcnZF2G+R/Hj4XccCpz
X84DI7SCFZIwl3gpiuOoR81mosd1VkLBouwYj1Aglt+5Sh63yq91lreX20Tw/swF
jlorAWAAblQdt2zC5smjtbbhfKMuPC8qJFGoiGmLLPC/1l9XmGmfymB2WfN8IZm4
DrqAlCSm8+xxGdwUwAZTuHuHX15VOpGuUuEV2E8BM+6G93HI8iJe5E7g5hRymcNp
NPCe+BFgZ9AxHqNokC1cSWrqaS3koBbTJRDKAqxSEptmYQMZ/a61ydYJQmBtImw=
=uqAv
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH -rt] timer: upper bound on loops of __run_timers processing
  2014-12-30 19:52 [PATCH -rt] timer: upper bound on loops of __run_timers processing Marcelo Tosatti
  2014-12-30 21:05 ` Rik van Riel
@ 2015-01-14 17:21 ` Marcelo Tosatti
  2015-01-14 17:41   ` Steven Rostedt
  2015-01-26 21:12   ` Marcelo Tosatti
  2015-02-17 23:08 ` Rik van Riel
  2 siblings, 2 replies; 7+ messages in thread
From: Marcelo Tosatti @ 2015-01-14 17:21 UTC (permalink / raw)
  To: Thomas Gleixner, Steven Rostedt
  Cc: Steven Rostedt, Clark Williams, Luiz Capitulino, Rik van Riel,
	linux-kernel, linux-rt-users


Ping?

On Tue, Dec 30, 2014 at 05:52:28PM -0200, Marcelo Tosatti wrote:
> 
> Commit "timers: do not raise softirq unconditionally", allows for timer
> wheel processing (__run_timers) to be delayed for long periods of time.
> 
> The effect is that 
> 
> loops = jiffies - base->timer_jiffies
> 
> Can grow to very large values resulting in __run_timers taking hundreds
> of milliseconds to execute.
> 
> Fix by creating an upper bound on the number of loops to be processed.
> This allows a nohz=off kernel to achieve desired latencies.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> diff --git a/kernel/timer.c b/kernel/timer.c
> index f59e18c..c128416 100644
> --- a/kernel/timer.c
> +++ b/kernel/timer.c
> @@ -1488,6 +1488,12 @@ void run_local_timers(void)
>  	}
>  #endif
>  
> +	if (time_after_eq(jiffies, base->timer_jiffies)) {
> +		unsigned long jiffies_delta = jiffies - base->timer_jiffies;
> +		if (jiffies_delta > TVR_SIZE)
> +			raise_softirq(TIMER_SOFTIRQ);
> +	}
> +
>  	if (!base->active_timers)
>  		goto out;
>  

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH -rt] timer: upper bound on loops of __run_timers processing
  2015-01-14 17:21 ` Marcelo Tosatti
@ 2015-01-14 17:41   ` Steven Rostedt
  2015-01-26 21:12   ` Marcelo Tosatti
  1 sibling, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2015-01-14 17:41 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Thomas Gleixner, Clark Williams, Luiz Capitulino, Rik van Riel,
	linux-kernel, linux-rt-users

On Wed, 14 Jan 2015 15:21:47 -0200
Marcelo Tosatti <mtosatti@redhat.com> wrote:

> 
> Ping?
> 

Sorry, I've fallen behind since the holidays and still trying to catch
up. I'll try to look at this this week.

-- Steve

> On Tue, Dec 30, 2014 at 05:52:28PM -0200, Marcelo Tosatti wrote:
> > 
> > Commit "timers: do not raise softirq unconditionally", allows for timer
> > wheel processing (__run_timers) to be delayed for long periods of time.
> > 
> > The effect is that 
> > 
> > loops = jiffies - base->timer_jiffies
> > 
> > Can grow to very large values resulting in __run_timers taking hundreds
> > of milliseconds to execute.
> > 
> > Fix by creating an upper bound on the number of loops to be processed.
> > This allows a nohz=off kernel to achieve desired latencies.
> > 
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > diff --git a/kernel/timer.c b/kernel/timer.c
> > index f59e18c..c128416 100644
> > --- a/kernel/timer.c
> > +++ b/kernel/timer.c
> > @@ -1488,6 +1488,12 @@ void run_local_timers(void)
> >  	}
> >  #endif
> >  
> > +	if (time_after_eq(jiffies, base->timer_jiffies)) {
> > +		unsigned long jiffies_delta = jiffies - base->timer_jiffies;
> > +		if (jiffies_delta > TVR_SIZE)
> > +			raise_softirq(TIMER_SOFTIRQ);
> > +	}
> > +
> >  	if (!base->active_timers)
> >  		goto out;
> >  


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH -rt] timer: upper bound on loops of __run_timers processing
  2015-01-14 17:21 ` Marcelo Tosatti
  2015-01-14 17:41   ` Steven Rostedt
@ 2015-01-26 21:12   ` Marcelo Tosatti
  2015-02-17 17:24     ` Sebastian Andrzej Siewior
  1 sibling, 1 reply; 7+ messages in thread
From: Marcelo Tosatti @ 2015-01-26 21:12 UTC (permalink / raw)
  To: Thomas Gleixner, Steven Rostedt
  Cc: Clark Williams, Luiz Capitulino, Rik van Riel, linux-kernel,
	linux-rt-users


Ping ?

On Wed, Jan 14, 2015 at 03:21:47PM -0200, Marcelo Tosatti wrote:
> 
> Ping?
> 
> On Tue, Dec 30, 2014 at 05:52:28PM -0200, Marcelo Tosatti wrote:
> > 
> > Commit "timers: do not raise softirq unconditionally", allows for timer
> > wheel processing (__run_timers) to be delayed for long periods of time.
> > 
> > The effect is that 
> > 
> > loops = jiffies - base->timer_jiffies
> > 
> > Can grow to very large values resulting in __run_timers taking hundreds
> > of milliseconds to execute.
> > 
> > Fix by creating an upper bound on the number of loops to be processed.
> > This allows a nohz=off kernel to achieve desired latencies.
> > 
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > diff --git a/kernel/timer.c b/kernel/timer.c
> > index f59e18c..c128416 100644
> > --- a/kernel/timer.c
> > +++ b/kernel/timer.c
> > @@ -1488,6 +1488,12 @@ void run_local_timers(void)
> >  	}
> >  #endif
> >  
> > +	if (time_after_eq(jiffies, base->timer_jiffies)) {
> > +		unsigned long jiffies_delta = jiffies - base->timer_jiffies;
> > +		if (jiffies_delta > TVR_SIZE)
> > +			raise_softirq(TIMER_SOFTIRQ);
> > +	}
> > +
> >  	if (!base->active_timers)
> >  		goto out;
> >  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH -rt] timer: upper bound on loops of __run_timers processing
  2015-01-26 21:12   ` Marcelo Tosatti
@ 2015-02-17 17:24     ` Sebastian Andrzej Siewior
  0 siblings, 0 replies; 7+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-02-17 17:24 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Thomas Gleixner, Steven Rostedt, Clark Williams, Luiz Capitulino,
	Rik van Riel, linux-kernel, linux-rt-users

* Marcelo Tosatti | 2015-01-26 19:12:31 [-0200]:

>Ping ?

please resent.

Sebastian

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH -rt] timer: upper bound on loops of __run_timers processing
  2014-12-30 19:52 [PATCH -rt] timer: upper bound on loops of __run_timers processing Marcelo Tosatti
  2014-12-30 21:05 ` Rik van Riel
  2015-01-14 17:21 ` Marcelo Tosatti
@ 2015-02-17 23:08 ` Rik van Riel
  2 siblings, 0 replies; 7+ messages in thread
From: Rik van Riel @ 2015-02-17 23:08 UTC (permalink / raw)
  To: Marcelo Tosatti, Thomas Gleixner
  Cc: Steven Rostedt, Clark Williams, Luiz Capitulino, linux-kernel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/30/2014 02:52 PM, Marcelo Tosatti wrote:
> 
> Commit "timers: do not raise softirq unconditionally", allows for
> timer wheel processing (__run_timers) to be delayed for long
> periods of time.
> 
> The effect is that
> 
> loops = jiffies - base->timer_jiffies
> 
> Can grow to very large values resulting in __run_timers taking
> hundreds of milliseconds to execute.
> 
> Fix by creating an upper bound on the number of loops to be
> processed. This allows a nohz=off kernel to achieve desired
> latencies.

Turns out that when we make nohz=off actually work correctly
with realtime tasks, and the scheduler tick is really disabled,
run_local_timers does not get called while the scheduler tick
is disabled, and this patch does nothing...

> +++ b/kernel/timer.c @@ -1488,6 +1488,12 @@ void
> run_local_timers(void) } #endif
> 
> +	if (time_after_eq(jiffies, base->timer_jiffies)) { +		unsigned
> long jiffies_delta = jiffies - base->timer_jiffies; +		if
> (jiffies_delta > TVR_SIZE) +			raise_softirq(TIMER_SOFTIRQ); +	} + 
> if (!base->active_timers) goto out;

I suspect we may need an alternate approach, where we
change the way __run_timers works, so it does not go
around its main loop thousands of times.

Is there any sane way we could going around this list
thousands of times?

This is especially annoying when there are no timers
pending in the first place :)

static inline void __run_timers(struct tvec_base *base)
{
        struct timer_list *timer;

        spin_lock_irq(&base->lock);
        while (time_after_eq(jiffies, base->timer_jiffies)) {
                struct list_head work_list;
                struct list_head *head = &work_list;
                int index = base->timer_jiffies & TVR_MASK;

                /*
                 * Cascade timers:
                 */
                if (!index &&
                        (!cascade(base, &base->tv2, INDEX(0))) &&
                                (!cascade(base, &base->tv3, INDEX(1))) &&
                                        !cascade(base, &base->tv4,
INDEX(2)))
                        cascade(base, &base->tv5, INDEX(3));
                ++base->timer_jiffies;
                list_replace_init(base->tv1.vec + index, &work_list);
                while (!list_empty(head)) {


- -- 
All rights reversed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJU48nfAAoJEM553pKExN6DKpoH/2Cf/nmra2uhsJzSm+FgbWgi
8EMM7TszF24Ys+JwtbdVmTonBsAXOmjJEGxvc9yknUymO3Wj6Iph4Z1cPk/9nS/v
vU7RWBiu9P6lmHfRuK+dN4w7SQnrtkOOuOLWlNSfwklVxCZasIPOsDazt/uivnpk
H3AMhGk0LduzAg1GSfJnCawnrIx/BgCu1UzHOMdE21SNZIS8z8o/MPQJ1qMbneQe
HbumMiB/0PmSmsMK9SVmYp7oEo0KliMJ+MW09Yrjfg0umz/h3TQ1aY3l5JhYK2rX
mmhlOIR4Sg720vZ6DzbDoRaJQHVY9Mc/QammpCU5ZY/f7diLzRsmm1FteNFQeAY=
=q2AN
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-02-17 23:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-30 19:52 [PATCH -rt] timer: upper bound on loops of __run_timers processing Marcelo Tosatti
2014-12-30 21:05 ` Rik van Riel
2015-01-14 17:21 ` Marcelo Tosatti
2015-01-14 17:41   ` Steven Rostedt
2015-01-26 21:12   ` Marcelo Tosatti
2015-02-17 17:24     ` Sebastian Andrzej Siewior
2015-02-17 23:08 ` Rik van Riel

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).