x86-tip.today: flood of WARNING: at include/linux/sched.h:2238 run_posix_cpu_timers+0xea/0x79f()

x86-tip.today: flood of WARNING: at include/linux/sched.h:2238 run_posix_cpu_timers+0xea/0x79f()

[Mike Galbraith]

Greetings,

Futzing with kerneltop this morning, I fired up amarok to give it
something light to monitor while I hunt missing events.  Something bad
happened, and when I finally (SysRq-E took minutes) got control of box
back, dmesg was full of a steady stream of the below.

That warning can be kinda unfriendly.


[ 9964.600004] ------------[ cut here ]------------
[ 9964.600006] WARNING: at include/linux/sched.h:2238 run_posix_cpu_timers+0xea/0x79f()
[ 9964.600008] Hardware name: MS-7502
[ 9964.600009] Modules linked in: snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device nfsd lockd nfs_acl auth_rpcgss sunrpc exportfs cpufreq_conservative cpufreq_userspace cpufreq_powersave acpi_cpufreq ip_tables ip6_tables microcode nls_iso8859_1 nls_cp437 vfat fat fuse loop dm_mod hid_pl hid_cypress hid_zpff hid_gyration hid_sony hid_samsung hid_microsoft hid_tmff hid_monterey hid_ezkey snd_hda_codec_realtek hid_a4tech hid_logitech ff_memless hid_cherry snd_hda_intel hid_sunplus snd_hda_codec firewire_ohci hid_petalynx firewire_core snd_hwdep snd_pcm hid_belkin snd_timer usbhid hid_chicony usb_storage snd crc_itu_t soundcore rtc_cmos ohci1394 libusual i2c_i801 hid rtc_core ieee1394 sr_mod rtc_lib e1000e snd_page_alloc cdrom intel_agp i2c_core button sg ehci_hcd uhci_hcd sd_mod usbcore edd ext3 mbcache jbd fan ahci libata scsi_mod thermal processor
[ 9964.600051] Pid: 10057, comm: amarokapp Tainted: G        W  2.6.29-tip-smp #126
[ 9964.600052] Call Trace:
[ 9964.600054]  <IRQ>  [<ffffffff80237ee3>] warn_slowpath+0xd3/0x10f
[ 9964.600060]  [<ffffffff802c1513>] ? bio_free+0x4d/0x52
[ 9964.600063]  [<ffffffff8022dc6a>] ? enqueue_task_fair+0x148/0x198
[ 9964.600067]  [<ffffffff8022be89>] ? enqueue_task+0x64/0x6f
[ 9964.600070]  [<ffffffff8022bf87>] ? activate_task+0x22/0x2a
[ 9964.600073]  [<ffffffff802347fd>] ? try_to_wake_up+0x24b/0x25d
[ 9964.600076]  [<ffffffff8023481c>] ? default_wake_function+0xd/0xf
[ 9964.600079]  [<ffffffff8024ae07>] ? autoremove_wake_function+0x11/0x38
[ 9964.600083]  [<ffffffff8022c51f>] ? __wake_up_common+0x46/0x76
[ 9964.600086]  [<ffffffff8022cce2>] ? __wake_up+0x43/0x50
[ 9964.600089]  [<ffffffff8024bfa7>] run_posix_cpu_timers+0xea/0x79f
[ 9964.600093]  [<ffffffff80279155>] ? perf_counter_task_tick+0x8f/0x94
[ 9964.600096]  [<ffffffff80240ff2>] update_process_times+0x57/0x60
[ 9964.600099]  [<ffffffff80254e22>] tick_sched_timer+0x6b/0x95
[ 9964.600102]  [<ffffffff8024d290>] __run_hrtimer+0x51/0x85
[ 9964.600105]  [<ffffffff8024dc92>] hrtimer_interrupt+0xe7/0x141
[ 9964.600108]  [<ffffffff8021db51>] smp_apic_timer_interrupt+0x80/0x98
[ 9964.600111]  [<ffffffff8020c993>] apic_timer_interrupt+0x13/0x20
[ 9964.600113]  <EOI> <4>---[ end trace 27d069ba1cae7828 ]---
[ 9964.636004] ------------[ cut here ]------------

Re: x86-tip.today: flood of WARNING: at include/linux/sched.h:2238 run_posix_cpu_timers+0xea/0x79f()

[Ingo Molnar]

* Mike Galbraith <efault@gmx.de> wrote:

> Greetings,
> 
> Futzing with kerneltop this morning, I fired up amarok to give it
> something light to monitor while I hunt missing events.  Something bad
> happened, and when I finally (SysRq-E took minutes) got control of box
> back, dmesg was full of a steady stream of the below.
> 
> That warning can be kinda unfriendly.
> 
> 
> [ 9964.600004] ------------[ cut here ]------------
> [ 9964.600006] WARNING: at include/linux/sched.h:2238 run_posix_cpu_timers+0xea/0x79f()

ah, that's Peter's stuff:

 4cd4c1b: timers: split process wide cpu clocks/timers
 32bd671: signal: re-add dead task accumulation stats.

The discussion with Oleg resulted in the conclusion i 
think that the warning is spurious and should be removed.

I've commited the minimal fix below to tip:master. There's
more fixes needed too but this should solve the warning
flood at least.

	Ingo

------------------------>
>From 7d8e23df69820e6be42bcc41d441f4860e8c76f7 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Fri, 6 Feb 2009 14:57:51 +0100
Subject: [PATCH] timers: split process wide cpu clocks/timers, remove spurious warning

Mike Galbraith reported that the new warning in thread_group_cputimer()
triggers en masse with Amarok running.

Oleg Nesterov observed:

  Can't fastpath_timer_check()->thread_group_cputimer() have the
  false warning too? Suppose we had the timer, then posix_cpu_timer_del()
  removes this timer, but task_cputime_zero(&sig->cputime_expires) still
  not true.

Remove the spurious debug warning.

Reported-by: Mike Galbraith <efault@gmx.de>
Explained-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/linux/sched.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 082d761..7939291 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2208,8 +2208,6 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
 	struct thread_group_cputimer *cputimer = &tsk->signal->cputimer;
 	unsigned long flags;
 
-	WARN_ON(!cputimer->running);
-
 	spin_lock_irqsave(&cputimer->lock, flags);
 	*times = cputimer->cputime;
 	spin_unlock_irqrestore(&cputimer->lock, flags);

Re: x86-tip.today: flood of WARNING: at include/linux/sched.h:2238 run_posix_cpu_timers+0xea/0x79f()

[Peter Zijlstra]

On Fri, 2009-02-06 at 15:01 +0100, Ingo Molnar wrote:
> * Mike Galbraith <efault@gmx.de> wrote:
> 
> > Greetings,
> > 
> > Futzing with kerneltop this morning, I fired up amarok to give it
> > something light to monitor while I hunt missing events.  Something bad
> > happened, and when I finally (SysRq-E took minutes) got control of box
> > back, dmesg was full of a steady stream of the below.
> > 
> > That warning can be kinda unfriendly.
> > 
> > 
> > [ 9964.600004] ------------[ cut here ]------------
> > [ 9964.600006] WARNING: at include/linux/sched.h:2238 run_posix_cpu_timers+0xea/0x79f()
> 
> ah, that's Peter's stuff:
> 
>  4cd4c1b: timers: split process wide cpu clocks/timers
>  32bd671: signal: re-add dead task accumulation stats.
> 
> The discussion with Oleg resulted in the conclusion i 
> think that the warning is spurious and should be removed.
> 
> I've commited the minimal fix below to tip:master. There's
> more fixes needed too but this should solve the warning
> flood at least.

I think this is the 'minimal' paranoia version.

---
Subject: timer: cleanup the clock/timer separation
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Tue Feb 10 15:46:29 CET 2009

To decrease the chance of a missed enable, always enable the timer when we
sample it, we'll always disable it when we find that there are no active timers
in the jiffy tick.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 include/linux/sched.h     |    1 +
 kernel/posix-cpu-timers.c |   42 ++++++++++++++----------------------------
 2 files changed, 15 insertions(+), 28 deletions(-)

Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h
+++ linux-2.6/include/linux/sched.h
@@ -2237,6 +2237,7 @@ void thread_group_cputimer(struct task_s
 	unsigned long flags;
 
 	spin_lock_irqsave(&cputimer->lock, flags);
+	cputimer->running = 1;
 	*times = cputimer->cputime;
 	spin_unlock_irqrestore(&cputimer->lock, flags);
 }
Index: linux-2.6/kernel/posix-cpu-timers.c
===================================================================
--- linux-2.6.orig/kernel/posix-cpu-timers.c
+++ linux-2.6/kernel/posix-cpu-timers.c
@@ -488,7 +488,7 @@ void posix_cpu_timers_exit_group(struct 
 {
 	struct task_cputime cputime;
 
-	thread_group_cputime(tsk, &cputime);
+	thread_group_cputimer(tsk, &cputime);
 	cleanup_timers(tsk->signal->cpu_timers,
 		       cputime.utime, cputime.stime, cputime.sum_exec_runtime);
 }
@@ -507,29 +507,6 @@ static void clear_dead_task(struct k_iti
 }
 
 /*
- * Enable the process wide cpu timer accounting.
- *
- * serialized using ->sighand->siglock
- */
-static void start_process_timers(struct task_struct *tsk)
-{
-	tsk->signal->cputimer.running = 1;
-	barrier();
-}
-
-/*
- * Release the process wide timer accounting -- timer stops ticking when
- * nobody cares about it.
- *
- * serialized using ->sighand->siglock
- */
-static void stop_process_timers(struct task_struct *tsk)
-{
-	tsk->signal->cputimer.running = 0;
-	barrier();
-}
-
-/*
  * Insert the timer on the appropriate list before any timers that
  * expire later.  This must be called with the tasklist_lock held
  * for reading, and interrupts disabled.
@@ -549,9 +526,6 @@ static void arm_timer(struct k_itimer *t
 	BUG_ON(!irqs_disabled());
 	spin_lock(&p->sighand->siglock);
 
-	if (!CPUCLOCK_PERTHREAD(timer->it_clock))
-		start_process_timers(p);
-
 	listpos = head;
 	if (CPUCLOCK_WHICH(timer->it_clock) == CPUCLOCK_SCHED) {
 		list_for_each_entry(next, head, entry) {
@@ -1021,6 +995,19 @@ static void check_thread_timers(struct t
 	}
 }
 
+static void stop_process_timers(struct task_struct *tsk)
+{
+	struct thread_group_cputimer *cputimer = &tsk->signal->cputimer;
+	unsigned long flags;
+
+	if (!cputimer->running)
+		return;
+
+	spin_lock_irqsave(&cputimer->lock);
+	cputimer->running = 0;
+	spin_unlock_irqrestore(&cputimer->lock);
+}
+
 /*
  * Check for any per-thread CPU timers that have fired and move them
  * off the tsk->*_timers list onto the firing list.  Per-thread timers
@@ -1427,7 +1414,6 @@ void set_process_cpu_timer(struct task_s
 	struct list_head *head;
 
 	BUG_ON(clock_idx == CPUCLOCK_SCHED);
-	start_process_timers(tsk);
 	cpu_timer_sample_group(clock_idx, tsk, &now);
 
 	if (oldval) {

[PATCH] timers: fix TIMER_ABSTIME for process wide cpu timers

[Peter Zijlstra]

While going through the man-pages Michael prepared for the posix timer
interface I realized that posix timers do expose the absolute time value
through the use of TIMER_ABSTIME.

---
Subject: timers: fix TIMER_ABSTIME for process wide cpu timers
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Wed Feb 11 11:30:27 CET 2009

The POSIX timer interface allows for absolute time expiry values through the
TIMER_ABSTIME flag, therefore we have to synchronize the timer to the clock
every time we start it.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 include/linux/sched.h     |   13 +------------
 kernel/posix-cpu-timers.c |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 12 deletions(-)

Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h
+++ linux-2.6/include/linux/sched.h
@@ -2229,18 +2229,7 @@ static inline int spin_needbreak(spinloc
  * Thread group CPU time accounting.
  */
 void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times);
-
-static inline
-void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
-{
-	struct thread_group_cputimer *cputimer = &tsk->signal->cputimer;
-	unsigned long flags;
-
-	spin_lock_irqsave(&cputimer->lock, flags);
-	cputimer->running = 1;
-	*times = cputimer->cputime;
-	spin_unlock_irqrestore(&cputimer->lock, flags);
-}
+void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times);
 
 static inline void thread_group_cputime_init(struct signal_struct *sig)
 {
Index: linux-2.6/kernel/posix-cpu-timers.c
===================================================================
--- linux-2.6.orig/kernel/posix-cpu-timers.c
+++ linux-2.6/kernel/posix-cpu-timers.c
@@ -261,6 +261,40 @@ out:
 	rcu_read_unlock();
 }
 
+static void update_gt_cputime(struct task_cputime *a, struct task_cputime *b)
+{
+	if (cputime_gt(b->utime, a->utime))
+		a->utime = b->utime;
+
+	if (cputime_gt(b->stime, a->stime))
+		a->stime = b->stime;
+
+	if (b->sum_exec_runtime > a->sum_exelt rec_runtime)
+		a->sum_exec_runtime = b->sum_exec_runtime;
+}
+
+void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
+{
+	struct thread_group_cputimer *cputimer = &tsk->signal->cputimer;
+	struct task_cputime sum;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cputimer->lock, flags);
+	if (!cputimer->running) {
+		cputimer->running = 1;
+		/*
+		 * The POSIX timer interface allows for absolute time expiry
+		 * values through the TIMER_ABSTIME flag, therefore we have
+		 * to synchronize the timer to the clock every time we start
+		 * it.
+		 */
+		thread_group_cputime(tsk, &sum);
+		update_gt_cputime(&cputimer->cputime, &sum);
+	}
+	*times = cputimer->cputime;
+	spin_unlock_irqrestore(&cputimer->lock, flags);
+}
+
 /*
  * Sample a process (thread group) clock for the given group_leader task.
  * Must be called with tasklist_lock held for reading.

Re: [PATCH] timers: fix TIMER_ABSTIME for process wide cpu timers

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> While going through the man-pages Michael prepared for the posix timer
> interface I realized that posix timers do expose the absolute time value
> through the use of TIMER_ABSTIME.
> 
> ---
> Subject: timers: fix TIMER_ABSTIME for process wide cpu timers
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Wed Feb 11 11:30:27 CET 2009
> 
> The POSIX timer interface allows for absolute time expiry values through the
> TIMER_ABSTIME flag, therefore we have to synchronize the timer to the clock
> every time we start it.
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Applied to tip:timers/urgent, thanks Peter!

	Ingo

Re: x86-tip.today: flood of WARNING: at include/linux/sched.h:2238 run_posix_cpu_timers+0xea/0x79f()

[Ingo Molnar]

* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> On Fri, 2009-02-06 at 15:01 +0100, Ingo Molnar wrote:
> > * Mike Galbraith <efault@gmx.de> wrote:
> > 
> > > Greetings,
> > > 
> > > Futzing with kerneltop this morning, I fired up amarok to give it
> > > something light to monitor while I hunt missing events.  Something bad
> > > happened, and when I finally (SysRq-E took minutes) got control of box
> > > back, dmesg was full of a steady stream of the below.
> > > 
> > > That warning can be kinda unfriendly.
> > > 
> > > 
> > > [ 9964.600004] ------------[ cut here ]------------
> > > [ 9964.600006] WARNING: at include/linux/sched.h:2238 run_posix_cpu_timers+0xea/0x79f()
> > 
> > ah, that's Peter's stuff:
> > 
> >  4cd4c1b: timers: split process wide cpu clocks/timers
> >  32bd671: signal: re-add dead task accumulation stats.
> > 
> > The discussion with Oleg resulted in the conclusion i 
> > think that the warning is spurious and should be removed.
> > 
> > I've commited the minimal fix below to tip:master. There's
> > more fixes needed too but this should solve the warning
> > flood at least.
> 
> I think this is the 'minimal' paranoia version.

applied to tip:timers/urgent, thanks Peter!

	Ingo