public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Martin Schwidefsky <schwidefsky@de.ibm.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org,
	Rob van der Heij <rvdheij@gmail.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Ingo Molnar <mingo@elte.hu>, john stultz <johnstul@us.ibm.com>,
	Andi Kleen <andi@firstfloor.org>
Subject: Re: [patch 0/2] NOHZ vs. profile/oprofile v2
Date: Mon, 22 Jun 2009 16:26:31 +0200	[thread overview]
Message-ID: <20090622162631.4b4dcee4@skybase> (raw)
In-Reply-To: <alpine.LFD.2.00.0906092252150.3351@localhost.localdomain>

On Tue, 9 Jun 2009 22:52:51 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:

> > version 2 of the profile patches. The only change is the in_interrupt()
> > fix in tick_nohz_stop_idle(). I would like to know how to proceed with
> > the issue.
> > Andy, do you still prefer to handle the old style profiler analog to
> > the oprofile patch? If yes I would drop patch #1 and extend patch #2
> > with another tick_nohz_disable().  
> 
> Any update on this one ?

A solution to this problem should go upstream soon, no? How about this
patch, it uses the tick_nohz_disable/tick_nohz_enable mechanic for 
oprofile and the old style kernel profiler. Good enough ?

---
Subject: [PATCH] keep on ticking if a profiler is active

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

On a NOHZ system with oprofile or the old style kernel profiler enabled
the timer tick should not be stopped when a cpu goes idle. Currently
a maximum of 1 tick is accounted if a cpu sleeps for a longer period of
time. This does bad things to the percentages in the profiler output.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 drivers/oprofile/oprof.c |    3 +++
 include/linux/tick.h     |    4 ++++
 kernel/profile.c         |    4 ++++
 kernel/time/tick-sched.c |   27 ++++++++++++++++++++++++++-
 4 files changed, 37 insertions(+), 1 deletion(-)

diff -urpN linux-2.6/drivers/oprofile/oprof.c linux-2.6-patched/drivers/oprofile/oprof.c
--- linux-2.6/drivers/oprofile/oprof.c	2009-06-10 05:05:27.000000000 +0200
+++ linux-2.6-patched/drivers/oprofile/oprof.c	2009-06-22 11:26:50.000000000 +0200
@@ -12,6 +12,7 @@
 #include <linux/init.h>
 #include <linux/oprofile.h>
 #include <linux/moduleparam.h>
+#include <linux/tick.h>
 #include <asm/mutex.h>
 
 #include "oprof.h"
@@ -103,6 +104,7 @@ int oprofile_start(void)
 	if (oprofile_started)
 		goto out;
 
+	tick_nohz_disable(1);
 	oprofile_reset_stats();
 
 	if ((err = oprofile_ops.start()))
@@ -123,6 +125,7 @@ void oprofile_stop(void)
 		goto out;
 	oprofile_ops.stop();
 	oprofile_started = 0;
+	tick_nohz_enable();
 	/* wake up the daemon to read what remains */
 	wake_up_buffer_waiter();
 out:
diff -urpN linux-2.6/include/linux/tick.h linux-2.6-patched/include/linux/tick.h
--- linux-2.6/include/linux/tick.h	2009-06-22 11:26:26.000000000 +0200
+++ linux-2.6-patched/include/linux/tick.h	2009-06-22 11:26:50.000000000 +0200
@@ -119,6 +119,8 @@ extern void tick_nohz_stop_sched_tick(in
 extern void tick_nohz_restart_sched_tick(void);
 extern ktime_t tick_nohz_get_sleep_length(void);
 extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
+extern void tick_nohz_enable(void);
+extern void tick_nohz_disable(int wakeup);
 # else
 static inline void tick_nohz_stop_sched_tick(int inidle) { }
 static inline void tick_nohz_restart_sched_tick(void) { }
@@ -129,6 +131,8 @@ static inline ktime_t tick_nohz_get_slee
 	return len;
 }
 static inline u64 get_cpu_idle_time_us(int cpu, u64 *unused) { return -1; }
+static inline void tick_nohz_enable(void) { }
+static inline void tick_nohz_disable(int wakeup) { }
 # endif /* !NO_HZ */
 
 #endif
diff -urpN linux-2.6/kernel/profile.c linux-2.6-patched/kernel/profile.c
--- linux-2.6/kernel/profile.c	2009-06-22 11:26:26.000000000 +0200
+++ linux-2.6-patched/kernel/profile.c	2009-06-22 11:26:50.000000000 +0200
@@ -24,6 +24,7 @@
 #include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
+#include <linux/tick.h>
 #include <asm/sections.h>
 #include <asm/irq_regs.h>
 #include <asm/ptrace.h>
@@ -97,6 +98,8 @@ int profile_setup(char *str)
 		printk(KERN_INFO "kernel profiling enabled (shift: %ld)\n",
 			prof_shift);
 	}
+	if (prof_on)
+		tick_nohz_disable(0);
 	return 1;
 }
 __setup("profile=", profile_setup);
@@ -582,6 +585,7 @@ static int create_hash_tables(void)
 	return 0;
 out_cleanup:
 	prof_on = 0;
+	tick_nohz_enable();
 	smp_mb();
 	on_each_cpu(profile_nop, NULL, 1);
 	for_each_online_cpu(cpu) {
diff -urpN linux-2.6/kernel/time/tick-sched.c linux-2.6-patched/kernel/time/tick-sched.c
--- linux-2.6/kernel/time/tick-sched.c	2009-06-22 11:26:26.000000000 +0200
+++ linux-2.6-patched/kernel/time/tick-sched.c	2009-06-22 11:26:50.000000000 +0200
@@ -124,6 +124,30 @@ static int __init setup_tick_nohz(char *
 
 __setup("nohz=", setup_tick_nohz);
 
+/*
+ * NO HZ currently disabled ?
+ */
+static atomic_t tick_nohz_disable_counter = ATOMIC_INIT(0);
+
+void tick_nohz_enable(void)
+{
+	atomic_dec(&tick_nohz_disable_counter);
+}
+EXPORT_SYMBOL_GPL(tick_nohz_enable);
+
+static void __tick_nohz_disable(void *dummy)
+{
+}
+
+void tick_nohz_disable(int wakeup)
+{
+	if (atomic_inc_return(&tick_nohz_disable_counter) == 1)
+		if (wakeup)
+			/* Wake up all cpus to make them start ticking. */
+			smp_call_function(__tick_nohz_disable, NULL, 0);
+}
+EXPORT_SYMBOL_GPL(tick_nohz_disable);
+
 /**
  * tick_nohz_update_jiffies - update jiffies when idle was interrupted
  *
@@ -276,7 +300,8 @@ void tick_nohz_stop_sched_tick(int inidl
 	next_jiffies = get_next_timer_interrupt(last_jiffies);
 	delta_jiffies = next_jiffies - last_jiffies;
 
-	if (rcu_needs_cpu(cpu) || printk_needs_cpu(cpu))
+	if (rcu_needs_cpu(cpu) || printk_needs_cpu(cpu) ||
+	    atomic_read(&tick_nohz_disable_counter) > 0)
 		delta_jiffies = 1;
 	/*
 	 * Do not stop the tick, if we are only one off


-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

  parent reply	other threads:[~2009-06-22 14:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-03 15:22 [patch 0/2] NOHZ vs. profile/oprofile v2 Martin Schwidefsky
2009-06-03 15:22 ` [patch 1/2] idle profile hits with NOHZ Martin Schwidefsky
2009-06-03 15:22 ` [patch 2/2] keep on ticking if oprofile is active Martin Schwidefsky
2009-06-09 20:52 ` [patch 0/2] NOHZ vs. profile/oprofile v2 Thomas Gleixner
2009-06-10 17:33   ` Martin Schwidefsky
2009-06-22 14:26   ` Martin Schwidefsky [this message]
2009-06-22 14:41     ` Ingo Molnar
2009-06-22 14:59       ` Martin Schwidefsky
2009-06-22 15:05         ` Ingo Molnar
2009-06-22 15:18           ` Martin Schwidefsky
2009-06-22 15:29             ` Ingo Molnar
2009-06-22 15:36               ` Martin Schwidefsky
2009-06-22 15:40                 ` Ingo Molnar
2009-06-22 16:37                   ` Martin Schwidefsky
2009-06-24 16:51                   ` Martin Schwidefsky
2009-06-24 17:47                     ` Ingo Molnar
2010-03-02 13:57                     ` Aaro Koskinen
2010-03-02 15:01                       ` Martin Schwidefsky
2010-03-02 15:08                         ` Thomas Gleixner
2010-03-02 15:25                           ` Martin Schwidefsky
2010-03-02 15:38                           ` Robert Richter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090622162631.4b4dcee4@skybase \
    --to=schwidefsky@de.ibm.com \
    --cc=andi@firstfloor.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=johnstul@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rvdheij@gmail.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox