[PATCH 1/8] OProfile update

[PATCH 1/8] OProfile update

[John Levon]

The next 8 patches change the following files :

 arch/alpha/oprofile/Makefile      |    3 +
 arch/alpha/oprofile/common.c      |    2 -
 arch/i386/oprofile/Makefile       |    5 +-
 arch/i386/oprofile/init.c         |   11 ++----
 arch/i386/oprofile/nmi_int.c      |   14 +++----
 arch/i386/oprofile/timer_int.c    |   58
--------------------------------
 arch/parisc/oprofile/Makefile     |    5 +-
 arch/parisc/oprofile/init.c       |    3 -
 arch/parisc/oprofile/timer_int.c  |   58
--------------------------------
 arch/ppc64/oprofile/Makefile      |    5 +-
 arch/ppc64/oprofile/init.c        |    3 -
 arch/ppc64/oprofile/timer_int.c   |   59
---------------------------------
 arch/sparc64/oprofile/Makefile    |    5 +-
 arch/sparc64/oprofile/init.c      |    3 -
 arch/sparc64/oprofile/timer_int.c |   59
---------------------------------
 arch/x86_64/oprofile/Makefile     |    9 ++---
 drivers/oprofile/buffer_sync.c    |   67
+++++++++++++++++++++++++-------------
 drivers/oprofile/event_buffer.c   |    6 ++-
 drivers/oprofile/oprof.c          |   23 +++++++++----
 drivers/oprofile/oprofile_stats.c |    6 +--
 drivers/oprofile/oprofile_stats.h |    2 -
 drivers/oprofile/timer_int.c      |   56
+++++++++++++++++++++++++++++++
 22 files changed, 159 insertions(+), 303 deletions(-)


Convention is that error returns are negative.

diff -Naur -X dontdiff linux-cvs/arch/alpha/oprofile/common.c linux-me/arch/alpha/oprofile/common.c
--- linux-cvs/arch/alpha/oprofile/common.c	2003-04-05 18:44:20.000000000 +0100
+++ linux-me/arch/alpha/oprofile/common.c	2003-04-29 01:18:48.000000000 +0100
@@ -175,7 +175,7 @@
 	}
 
 	if (!lmodel)
-		return ENODEV;
+		return -ENODEV;
 	model = lmodel;
 
 	oprof_axp_ops.cpu_type = lmodel->cpu_type;

[PATCH 2/8] OProfile update

[John Levon]

Consolidate all the arch copies of timer_int.c into one place. Also fixes
a problem with PA-RISC not using instruction_pointer() when it should.

diff -Naur -X dontdiff linux-cvs/arch/alpha/oprofile/Makefile linux-me/arch/alpha/oprofile/Makefile
--- linux-cvs/arch/alpha/oprofile/Makefile	2003-02-23 18:27:21.000000000 +0000
+++ linux-me/arch/alpha/oprofile/Makefile	2003-04-29 01:17:51.000000000 +0100
@@ -5,7 +5,8 @@
 DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
 		oprof.o cpu_buffer.o buffer_sync.o \
 		event_buffer.o oprofile_files.o \
-		oprofilefs.o oprofile_stats.o )
+		oprofilefs.o oprofile_stats.o \
+		timer_int.o )
 
 oprofile-y				:= $(DRIVER_OBJS) common.o
 oprofile-$(CONFIG_ALPHA_GENERIC)	+= op_model_ev4.o \
diff -Naur -X dontdiff linux-cvs/arch/i386/oprofile/init.c linux-me/arch/i386/oprofile/init.c
--- linux-cvs/arch/i386/oprofile/init.c	2003-04-05 18:44:23.000000000 +0100
+++ linux-me/arch/i386/oprofile/init.c	2003-04-29 01:17:00.000000000 +0100
@@ -11,22 +11,19 @@
 #include <linux/init.h>
  
 /* We support CPUs that have performance counters like the Pentium Pro
- * with NMI mode samples. Other x86 CPUs use a simple interrupt keyed
- * off the timer interrupt, which cannot profile interrupts-disabled
- * code unlike the NMI-based code.
+ * with the NMI mode driver.
  */
  
 extern int nmi_init(struct oprofile_operations ** ops);
 extern void nmi_exit(void);
-extern void timer_init(struct oprofile_operations ** ops);
 
 int __init oprofile_arch_init(struct oprofile_operations ** ops)
 {
 #ifdef CONFIG_X86_LOCAL_APIC
-	if (!nmi_init(ops))
+	return nmi_init(ops);
+#else
+	return -ENODEV;
 #endif
-		timer_init(ops);
-	return 0;
 }
 
 
diff -Naur -X dontdiff linux-cvs/arch/i386/oprofile/Makefile linux-me/arch/i386/oprofile/Makefile
--- linux-cvs/arch/i386/oprofile/Makefile	2003-02-11 20:25:24.000000000 +0000
+++ linux-me/arch/i386/oprofile/Makefile	2003-04-29 01:11:32.000000000 +0100
@@ -3,8 +3,9 @@
 DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
 		oprof.o cpu_buffer.o buffer_sync.o \
 		event_buffer.o oprofile_files.o \
-		oprofilefs.o oprofile_stats.o )
+		oprofilefs.o oprofile_stats.o  \
+		timer_int.o )
 
-oprofile-y				:= $(DRIVER_OBJS) init.o timer_int.o
+oprofile-y				:= $(DRIVER_OBJS) init.o
 oprofile-$(CONFIG_X86_LOCAL_APIC) 	+= nmi_int.o op_model_athlon.o \
 					   op_model_ppro.o op_model_p4.o
diff -Naur -X dontdiff linux-cvs/arch/i386/oprofile/nmi_int.c linux-me/arch/i386/oprofile/nmi_int.c
--- linux-cvs/arch/i386/oprofile/nmi_int.c	2003-04-05 18:44:23.000000000 +0100
+++ linux-me/arch/i386/oprofile/nmi_int.c	2003-04-29 01:16:50.000000000 +0100
@@ -314,13 +314,13 @@
 	__u8 family = current_cpu_data.x86;
  
 	if (!cpu_has_apic)
-		return 0;
+		return -ENODEV;
  
 	switch (vendor) {
 		case X86_VENDOR_AMD:
 			/* Needs to be at least an Athlon (or hammer in 32bit mode) */
 			if (family < 6)
-				return 0;
+				return -ENODEV;
 			model = &op_athlon_spec;
 			nmi_ops.cpu_type = "i386/athlon";
 			break;
@@ -331,30 +331,30 @@
 				/* Pentium IV */
 				case 0xf:
 					if (!p4_init())
-						return 0;
+						return -ENODEV;
 					break;
 
 				/* A P6-class processor */
 				case 6:
 					if (!ppro_init())
-						return 0;
+						return -ENODEV;
 					break;
 
 				default:
-					return 0;
+					return -ENODEV;
 			}
 			break;
 #endif /* !CONFIG_X86_64 */
 
 		default:
-			return 0;
+			return -ENODEV;
 	}
 
 	init_driverfs();
 	using_nmi = 1;
 	*ops = &nmi_ops;
 	printk(KERN_INFO "oprofile: using NMI interrupt.\n");
-	return 1;
+	return 0;
 }
 
 
diff -Naur -X dontdiff linux-cvs/arch/i386/oprofile/timer_int.c linux-me/arch/i386/oprofile/timer_int.c
--- linux-cvs/arch/i386/oprofile/timer_int.c	2003-02-11 20:26:06.000000000 +0000
+++ linux-me/arch/i386/oprofile/timer_int.c	1970-01-01 01:00:00.000000000 +0100
@@ -1,58 +0,0 @@
-/**
- * @file timer_int.c
- *
- * @remark Copyright 2002 OProfile authors
- * @remark Read the file COPYING
- *
- * @author John Levon <levon@movementarian.org>
- */
-
-#include <linux/kernel.h>
-#include <linux/notifier.h>
-#include <linux/smp.h>
-#include <linux/irq.h>
-#include <linux/oprofile.h>
-#include <asm/ptrace.h>
- 
-#include "op_counter.h"
- 
-static int timer_notify(struct notifier_block * self, unsigned long val, void * data)
-{
-	struct pt_regs * regs = (struct pt_regs *)data;
-	int cpu = smp_processor_id();
-	unsigned long eip = instruction_pointer(regs);
- 
-	oprofile_add_sample(eip, !user_mode(regs), 0, cpu);
-	return 0;
-}
- 
- 
-static struct notifier_block timer_notifier = {
-	.notifier_call	= timer_notify,
-};
- 
-
-static int timer_start(void)
-{
-	return register_profile_notifier(&timer_notifier);
-}
-
-
-static void timer_stop(void)
-{
-	unregister_profile_notifier(&timer_notifier);
-}
-
-
-static struct oprofile_operations timer_ops = {
-	.start	= timer_start,
-	.stop	= timer_stop,
-	.cpu_type = "timer"
-};
-
- 
-void __init timer_init(struct oprofile_operations ** ops)
-{
-	*ops = &timer_ops;
-	printk(KERN_INFO "oprofile: using timer interrupt.\n");
-}
diff -Naur -X dontdiff linux-cvs/arch/parisc/oprofile/init.c linux-me/arch/parisc/oprofile/init.c
--- linux-cvs/arch/parisc/oprofile/init.c	2003-04-05 18:44:29.000000000 +0100
+++ linux-me/arch/parisc/oprofile/init.c	2003-04-29 01:24:09.000000000 +0100
@@ -15,8 +15,7 @@
 
 int __init oprofile_arch_init(struct oprofile_operations ** ops)
 {
-	timer_init(ops);
-	return 0;
+	return -ENODEV;
 }
 
 
diff -Naur -X dontdiff linux-cvs/arch/parisc/oprofile/Makefile linux-me/arch/parisc/oprofile/Makefile
--- linux-cvs/arch/parisc/oprofile/Makefile	2003-01-13 21:24:33.000000000 +0000
+++ linux-me/arch/parisc/oprofile/Makefile	2003-04-29 01:24:18.000000000 +0100
@@ -3,6 +3,7 @@
 DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
 		oprof.o cpu_buffer.o buffer_sync.o \
 		event_buffer.o oprofile_files.o \
-		oprofilefs.o oprofile_stats.o )
+		oprofilefs.o oprofile_stats.o \
+		timer_int.o )
 
-oprofile-y				:= $(DRIVER_OBJS) init.o timer_int.o
+oprofile-y				:= $(DRIVER_OBJS) init.o
diff -Naur -X dontdiff linux-cvs/arch/parisc/oprofile/timer_int.c linux-me/arch/parisc/oprofile/timer_int.c
--- linux-cvs/arch/parisc/oprofile/timer_int.c	2003-02-11 20:26:06.000000000 +0000
+++ linux-me/arch/parisc/oprofile/timer_int.c	1970-01-01 01:00:00.000000000 +0100
@@ -1,58 +0,0 @@
-/**
- * @file timer_int.c
- *
- * @remark Copyright 2002 OProfile authors
- * @remark Read the file COPYING
- *
- * @author John Levon <levon@movementarian.org>
- */
-
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/notifier.h>
-#include <linux/smp.h>
-#include <linux/irq.h>
-#include <linux/oprofile.h>
-#include <asm/ptrace.h>
- 
-static int timer_notify(struct notifier_block * self, unsigned long val, void * data)
-{
-	struct pt_regs * regs = (struct pt_regs *)data;
-	int cpu = smp_processor_id();
-	unsigned long pc = regs->iaoq[0];
-	int is_kernel = !user_mode(regs);
- 
-	oprofile_add_sample(pc, is_kernel, 0, cpu);
-	return 0;
-}
- 
- 
-static struct notifier_block timer_notifier = {
-	.notifier_call	= timer_notify,
-};
- 
-
-static int timer_start(void)
-{
-	return register_profile_notifier(&timer_notifier);
-}
-
-
-static void timer_stop(void)
-{
-	unregister_profile_notifier(&timer_notifier);
-}
-
-
-static struct oprofile_operations timer_ops = {
-	.start	= timer_start,
-	.stop	= timer_stop,
-	.cpu_type = "timer"
-};
-
- 
-void __init timer_init(struct oprofile_operations ** ops)
-{
-	*ops = &timer_ops;
-	printk(KERN_INFO "oprofile: using timer interrupt.\n");
-}
diff -Naur -X dontdiff linux-cvs/arch/ppc64/oprofile/init.c linux-me/arch/ppc64/oprofile/init.c
--- linux-cvs/arch/ppc64/oprofile/init.c	2003-04-05 18:44:32.000000000 +0100
+++ linux-me/arch/ppc64/oprofile/init.c	2003-04-29 01:23:33.000000000 +0100
@@ -15,8 +15,7 @@
 
 int __init oprofile_arch_init(struct oprofile_operations ** ops)
 {
-	timer_init(ops);
-	return 0;
+	return -ENODEV;
 }
 
 
diff -Naur -X dontdiff linux-cvs/arch/ppc64/oprofile/Makefile linux-me/arch/ppc64/oprofile/Makefile
--- linux-cvs/arch/ppc64/oprofile/Makefile	2003-01-02 19:32:40.000000000 +0000
+++ linux-me/arch/ppc64/oprofile/Makefile	2003-04-29 01:24:29.000000000 +0100
@@ -3,6 +3,7 @@
 DRIVER_OBJS := $(addprefix ../../../drivers/oprofile/, \
 		oprof.o cpu_buffer.o buffer_sync.o \
 		event_buffer.o oprofile_files.o \
-		oprofilefs.o oprofile_stats.o )
+		oprofilefs.o oprofile_stats.o \
+		timer_int.o )
 
-oprofile-y := $(DRIVER_OBJS) init.o timer_int.o
+oprofile-y := $(DRIVER_OBJS) init.o
diff -Naur -X dontdiff linux-cvs/arch/ppc64/oprofile/timer_int.c linux-me/arch/ppc64/oprofile/timer_int.c
--- linux-cvs/arch/ppc64/oprofile/timer_int.c	2003-02-22 07:55:49.000000000 +0000
+++ linux-me/arch/ppc64/oprofile/timer_int.c	1970-01-01 01:00:00.000000000 +0100
@@ -1,59 +0,0 @@
-/**
- * @file timer_int.c
- *
- * @remark Copyright 2002 OProfile authors
- * @remark Read the file COPYING
- *
- * @author John Levon <levon@movementarian.org>
- */
-
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/notifier.h>
-#include <linux/smp.h>
-#include <linux/irq.h>
-#include <linux/oprofile.h>
-#include <linux/profile.h>
-#include <asm/ptrace.h>
- 
-static int timer_notify(struct notifier_block * self, unsigned long val, void * data)
-{
-	struct pt_regs * regs = (struct pt_regs *)data;
-	int cpu = smp_processor_id();
-	unsigned long pc = instruction_pointer(regs);
-	int is_kernel = !user_mode(regs);
- 
-	oprofile_add_sample(pc, is_kernel, 0, cpu);
-	return 0;
-}
- 
- 
-static struct notifier_block timer_notifier = {
-	.notifier_call	= timer_notify,
-};
- 
-
-static int timer_start(void)
-{
-	return register_profile_notifier(&timer_notifier);
-}
-
-
-static void timer_stop(void)
-{
-	unregister_profile_notifier(&timer_notifier);
-}
-
-
-static struct oprofile_operations timer_ops = {
-	.start	= timer_start,
-	.stop	= timer_stop,
-	.cpu_type = "timer"
-};
-
- 
-void __init timer_init(struct oprofile_operations ** ops)
-{
-	*ops = &timer_ops;
-	printk(KERN_INFO "oprofile: using timer interrupt.\n");
-}
diff -Naur -X dontdiff linux-cvs/arch/sparc64/oprofile/init.c linux-me/arch/sparc64/oprofile/init.c
--- linux-cvs/arch/sparc64/oprofile/init.c	2003-04-05 18:44:34.000000000 +0100
+++ linux-me/arch/sparc64/oprofile/init.c	2003-04-29 01:23:09.000000000 +0100
@@ -15,8 +15,7 @@
 
 int __init oprofile_arch_init(struct oprofile_operations ** ops)
 {
-	timer_init(ops);
-	return 0;
+	return -ENODEV;
 }
 
 
diff -Naur -X dontdiff linux-cvs/arch/sparc64/oprofile/Makefile linux-me/arch/sparc64/oprofile/Makefile
--- linux-cvs/arch/sparc64/oprofile/Makefile	2002-12-09 01:58:08.000000000 +0000
+++ linux-me/arch/sparc64/oprofile/Makefile	2003-04-29 01:20:27.000000000 +0100
@@ -3,6 +3,7 @@
 DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
 		oprof.o cpu_buffer.o buffer_sync.o \
 		event_buffer.o oprofile_files.o \
-		oprofilefs.o oprofile_stats.o )
+		oprofilefs.o oprofile_stats.o \
+		timer_int.o )
 
-oprofile-y				:= $(DRIVER_OBJS) init.o timer_int.o
+oprofile-y				:= $(DRIVER_OBJS) init.o
diff -Naur -X dontdiff linux-cvs/arch/sparc64/oprofile/timer_int.c linux-me/arch/sparc64/oprofile/timer_int.c
--- linux-cvs/arch/sparc64/oprofile/timer_int.c	2003-02-20 01:03:34.000000000 +0000
+++ linux-me/arch/sparc64/oprofile/timer_int.c	1970-01-01 01:00:00.000000000 +0100
@@ -1,59 +0,0 @@
-/**
- * @file timer_int.c
- *
- * @remark Copyright 2002 OProfile authors
- * @remark Read the file COPYING
- *
- * @author John Levon <levon@movementarian.org>
- */
-
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/notifier.h>
-#include <linux/smp.h>
-#include <linux/irq.h>
-#include <linux/oprofile.h>
-#include <linux/profile.h>
-#include <asm/ptrace.h>
- 
-static int timer_notify(struct notifier_block * self, unsigned long val, void * data)
-{
-	struct pt_regs * regs = (struct pt_regs *)data;
-	int cpu = smp_processor_id();
-	unsigned long pc = instruction_pointer(regs);
-	int is_kernel = !user_mode(regs);
- 
-	oprofile_add_sample(pc, is_kernel, 0, cpu);
-	return 0;
-}
- 
- 
-static struct notifier_block timer_notifier = {
-	.notifier_call	= timer_notify,
-};
- 
-
-static int timer_start(void)
-{
-	return register_profile_notifier(&timer_notifier);
-}
-
-
-static void timer_stop(void)
-{
-	unregister_profile_notifier(&timer_notifier);
-}
-
-
-static struct oprofile_operations timer_ops = {
-	.start	= timer_start,
-	.stop	= timer_stop,
-	.cpu_type = "timer"
-};
-
- 
-void __init timer_init(struct oprofile_operations ** ops)
-{
-	*ops = &timer_ops;
-	printk(KERN_INFO "oprofile: using timer interrupt.\n");
-}
diff -Naur -X dontdiff linux-cvs/arch/x86_64/oprofile/Makefile linux-me/arch/x86_64/oprofile/Makefile
--- linux-cvs/arch/x86_64/oprofile/Makefile	2002-12-28 19:54:15.000000000 +0000
+++ linux-me/arch/x86_64/oprofile/Makefile	2003-04-29 01:19:24.000000000 +0100
@@ -9,9 +9,10 @@
 DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
 	oprof.o cpu_buffer.o buffer_sync.o \
 	event_buffer.o oprofile_files.o \
-	oprofilefs.o oprofile_stats.o )
+	oprofilefs.o oprofile_stats.o \
+	timer_int.o )
  
-oprofile-objs := $(DRIVER_OBJS) init.o timer_int.o
+oprofile-objs := $(DRIVER_OBJS) init.o
 
 oprofile-$(CONFIG_X86_LOCAL_APIC) += nmi_int.o op_model_athlon.o
  
@@ -23,11 +24,9 @@
 	@ln -sf ../../i386/oprofile/op_model_athlon.c $(obj)/op_model_athlon.c
 $(obj)/init.c: ${INCL}
 	@ln -sf ../../i386/oprofile/init.c $(obj)/init.c
-$(obj)/timer_int.c: ${INCL}
-	@ln -sf ../../i386/oprofile/timer_int.c $(obj)/timer_int.c
 $(obj)/op_counter.h: 
 	@ln -sf ../../i386/oprofile/op_counter.h $(obj)/op_counter.h
 $(obj)/op_x86_model.h:
 	@ln -sf ../../i386/oprofile/op_x86_model.h $(obj)/op_x86_model.h	
-clean-files += op_x86_model.h op_counter.h timer_int.c init.c \
+clean-files += op_x86_model.h op_counter.h init.c \
 	       op_model_athlon.c nmi_int.c
diff -Naur -X dontdiff linux-cvs/drivers/oprofile/timer_int.c linux-me/drivers/oprofile/timer_int.c
--- linux-cvs/drivers/oprofile/timer_int.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-me/drivers/oprofile/timer_int.c	2003-04-29 01:25:24.000000000 +0100
@@ -0,0 +1,56 @@
+/**
+ * @file timer_int.c
+ *
+ * @remark Copyright 2002 OProfile authors
+ * @remark Read the file COPYING
+ *
+ * @author John Levon <levon@movementarian.org>
+ */
+
+#include <linux/kernel.h>
+#include <linux/notifier.h>
+#include <linux/smp.h>
+#include <linux/irq.h>
+#include <linux/oprofile.h>
+#include <asm/ptrace.h>
+ 
+static int timer_notify(struct notifier_block * self, unsigned long val, void * data)
+{
+	struct pt_regs * regs = (struct pt_regs *)data;
+	int cpu = smp_processor_id();
+	unsigned long eip = instruction_pointer(regs);
+ 
+	oprofile_add_sample(eip, !user_mode(regs), 0, cpu);
+	return 0;
+}
+ 
+ 
+static struct notifier_block timer_notifier = {
+	.notifier_call	= timer_notify,
+};
+ 
+
+static int timer_start(void)
+{
+	return register_profile_notifier(&timer_notifier);
+}
+
+
+static void timer_stop(void)
+{
+	unregister_profile_notifier(&timer_notifier);
+}
+
+
+static struct oprofile_operations timer_ops = {
+	.start	= timer_start,
+	.stop	= timer_stop,
+	.cpu_type = "timer"
+};
+
+ 
+void __init timer_init(struct oprofile_operations ** ops)
+{
+	*ops = &timer_ops;
+	printk(KERN_INFO "oprofile: using timer interrupt.\n");
+}
diff -Naur -X dontdiff linux-cvs/drivers/oprofile/oprof.c linux-me/drivers/oprofile/oprof.c
--- linux-cvs/drivers/oprofile/oprof.c	2003-04-05 18:44:50.000000000 +0100
+++ linux-me/drivers/oprofile/oprof.c	2003-04-30 19:58:07.000000000 +0100
@@ -119,14 +119,23 @@
 }
 
  
+extern void timer_init(struct oprofile_operations ** ops);
+
+ 
 static int __init oprofile_init(void)
 {
 	int err;
 
 	/* Architecture must fill in the interrupt ops and the
-	 * logical CPU type.
+	 * logical CPU type, or we can fall back to the timer
+	 * interrupt profiler.
 	 */
 	err = oprofile_arch_init(&oprofile_ops);
+	if (err == -ENODEV) {
+		timer_init(&oprofile_ops);
+		err = 0;
+	}
+
 	if (err)
 		goto out;
 

[PATCH 3/8] OProfile update

[John Levon]

Clear up the code around  start_sem so it's more obvious, and remove a pointless
down/up pair on buffer_sem (shutdown is already synchronised in sync_stop()).

diff -Naur -X dontdiff linux-cvs/drivers/oprofile/oprof.c linux-me/drivers/oprofile/oprof.c
--- linux/drivers/oprofile/oprof.c	2003-04-30 19:58:07.000000000 +0100
+++ linux-me/drivers/oprofile/oprof.c	2003-04-29 01:16:00.000000000 +0100
@@ -28,6 +28,8 @@
 {
 	int err;
  
+	down(&start_sem);
+
 	if ((err = alloc_cpu_buffers()))
 		goto out;
 
@@ -45,7 +47,6 @@
 	if ((err = sync_start()))
 		goto out3;
 
-	down(&start_sem);
 	is_setup = 1;
 	up(&start_sem);
 	return 0;
@@ -58,6 +59,7 @@
 out1:
 	free_cpu_buffers();
 out:
+	up(&start_sem);
 	return err;
 }
 
@@ -106,22 +108,20 @@
 
 void oprofile_shutdown(void)
 {
+	down(&start_sem);
 	sync_stop();
 	if (oprofile_ops->shutdown)
 		oprofile_ops->shutdown(); 
-	/* down() is also necessary to synchronise all pending events
-	 * before freeing */
-	down(&buffer_sem);
 	is_setup = 0;
-	up(&buffer_sem);
 	free_event_buffer();
 	free_cpu_buffers();
+	up(&start_sem);
 }
 
- 
+
 extern void timer_init(struct oprofile_operations ** ops);
 
- 
+
 static int __init oprofile_init(void)
 {
 	int err;

[PATCH 4/8] OProfile update

[John Levon]

If there's are multiple tasks sleeping inside the read routine ever, this is necessary.

diff -Naur -X dontdiff linux-cvs/drivers/oprofile/event_buffer.c linux-me/drivers/oprofile/event_buffer.c
--- linux-cvs/drivers/oprofile/event_buffer.c	2002-12-21 19:18:20.000000000 +0000
+++ linux-me/drivers/oprofile/event_buffer.c	2003-04-29 01:09:35.000000000 +0100
@@ -151,11 +151,15 @@
 	if (count != max || *offset)
 		return -EINVAL;
 
-	/* wait for the event buffer to fill up with some data */
 	wait_event_interruptible(buffer_wait, atomic_read(&buffer_ready));
+
 	if (signal_pending(current))
 		return -EINTR;
 
+	/* can't currently happen */
+	if (!atomic_read(&buffer_ready))
+		return -EAGAIN;
+
 	down(&buffer_sem);
 
 	atomic_set(&buffer_ready, 0);

[PATCH 5/8] OProfile update

[John Levon]

Now we don't do buffer syncs whilst holding current's mmap_sem ever,
we can wait to hold it. Also take task_lock and try to improve the docs a bit.

diff -Naur -X dontdiff linux-cvs/drivers/oprofile/buffer_sync.c linux-me/drivers/oprofile/buffer_sync.c
--- linux-cvs/drivers/oprofile/buffer_sync.c	2003-04-05 18:44:49.000000000 +0100
+++ linux-me/drivers/oprofile/buffer_sync.c	2003-05-03 20:10:44.000000000 +0100
@@ -310,26 +337,23 @@
 /* Take the task's mmap_sem to protect ourselves from
  * races when we do lookup_dcookie().
  */
-static struct mm_struct * take_task_mm(struct task_struct * task)
+static struct mm_struct * take_tasks_mm(struct task_struct * task)
 {
-	struct mm_struct * mm = task->mm;
- 
-	/* if task->mm !NULL, mm_count must be at least 1. It cannot
-	 * drop to 0 without the task exiting, which will have to sleep
-	 * on buffer_sem first. So we do not need to mark mm_count
-	 * ourselves.
+	struct mm_struct * mm;
+       
+	/* Subtle. We don't need to keep a reference to this task's mm,
+	 * because, for the mm to be freed on another CPU, that would have
+	 * to go through the task exit notifier, which ends up sleeping
+	 * on the buffer_sem we hold, so we end up with mutual exclusion
+	 * anyway.
 	 */
+	task_lock(task);
+	mm = task->mm;
+	task_unlock(task);
+ 
 	if (mm) {
-		/* More ugliness. If a task took its mmap
-		 * sem then came to sleep on buffer_sem we
-		 * will deadlock waiting for it. So we can
-		 * but try. This will lose samples :/
-		 */
-		if (!down_read_trylock(&mm->mmap_sem)) {
-			/* FIXME: this underestimates samples lost */
-			atomic_inc(&oprofile_stats.sample_lost_mmap_sem);
-			mm = NULL;
-		}
+		/* needed to walk the task's VMAs */
+		down_read(&mm->mmap_sem);
 	}
  
 	return mm;
@@ -399,7 +423,7 @@
 				new = (struct task_struct *)s->event;
 
 				release_mm(mm);
-				mm = take_task_mm(new);
+				mm = take_tasks_mm(new);
 
 				cookie = get_exec_dcookie(mm);
 				add_user_ctx_switch(new->pid, cookie);
@@ -460,4 +484,3 @@
 	schedule_work(&sync_wq);
 	/* timer is re-added by the scheduled task */
 }
-

[PATCH 7/8] OProfile update

[John Levon]

Change the lost_mmap_sem stat to lost_no_mm, and account it.

diff -Naur -X dontdiff linux-cvs/drivers/oprofile/buffer_sync.c linux-me/drivers/oprofile/buffer_sync.c
--- linux-cvs/drivers/oprofile/buffer_sync.c	2003-04-05 18:44:49.000000000 +0100
+++ linux-me/drivers/oprofile/buffer_sync.c	2003-05-03 20:10:44.000000000 +0100
@@ -296,6 +321,8 @@
 		add_sample_entry(s->eip, s->event);
 	} else if (mm) {
 		add_us_sample(mm, s);
+	} else {
+		atomic_inc(&oprofile_stats.sample_lost_no_mm);
 	}
 }
  
diff -Naur -X dontdiff linux-cvs/drivers/oprofile/oprofile_stats.c linux-me/drivers/oprofile/oprofile_stats.c
--- linux-cvs/drivers/oprofile/oprofile_stats.c	2003-03-07 15:39:16.000000000 +0000
+++ linux-me/drivers/oprofile/oprofile_stats.c	2003-05-01 14:40:18.000000000 +0100
@@ -31,7 +31,7 @@
 		cpu_buf->sample_lost_task_exit = 0;
 	}
  
-	atomic_set(&oprofile_stats.sample_lost_mmap_sem, 0);
+	atomic_set(&oprofile_stats.sample_lost_no_mm, 0);
 	atomic_set(&oprofile_stats.event_lost_overflow, 0);
 }
 
@@ -68,8 +68,8 @@
 			&cpu_buf->sample_lost_task_exit);
 	}
  
-	oprofilefs_create_ro_atomic(sb, dir, "sample_lost_mmap_sem",
-		&oprofile_stats.sample_lost_mmap_sem);
+	oprofilefs_create_ro_atomic(sb, dir, "sample_lost_no_mm",
+		&oprofile_stats.sample_lost_no_mm);
 	oprofilefs_create_ro_atomic(sb, dir, "event_lost_overflow",
 		&oprofile_stats.event_lost_overflow);
 }
diff -Naur -X dontdiff linux-cvs/drivers/oprofile/oprofile_stats.h linux-me/drivers/oprofile/oprofile_stats.h
--- linux-cvs/drivers/oprofile/oprofile_stats.h	2002-10-16 03:26:30.000000000 +0100
+++ linux-me/drivers/oprofile/oprofile_stats.h	2003-05-01 14:36:12.000000000 +0100
@@ -13,7 +13,7 @@
 #include <asm/atomic.h>
  
 struct oprofile_stat_struct {
-	atomic_t sample_lost_mmap_sem;
+	atomic_t sample_lost_no_mm;
 	atomic_t event_lost_overflow;
 };
 

[PATCH 6/8] OProfile update

[John Levon]

We were doing del_timer_sync() on shutdown, but not ensuring that any queued work
was done as well.

diff -Naur -X dontdiff linux-cvs/drivers/oprofile/buffer_sync.c linux-me/drivers/oprofile/buffer_sync.c
--- linux-cvs/drivers/oprofile/buffer_sync.c	2003-04-05 18:44:49.000000000 +0100
+++ linux-me/drivers/oprofile/buffer_sync.c	2003-05-03 20:10:44.000000000 +0100
@@ -147,6 +170,8 @@
 	profile_event_unregister(EXIT_MMAP, &exit_mmap_nb);
 	profile_event_unregister(EXEC_UNMAP, &exec_unmap_nb);
 	del_timer_sync(&sync_timer);
+	/* timer might have queued work, make sure it's completed. */
+	flush_scheduled_work();
 }
 
  

[PATCH 8/8] OProfile update

[John Levon]

Schedule work away on an unmap, instead of calling it directly. Should result
in less lost samples, and it fixes a lock ordering problem buffer_sem <-> mmap_sem

diff -Naur -X dontdiff linux-cvs/drivers/oprofile/buffer_sync.c linux-me/drivers/oprofile/buffer_sync.c
--- linux-cvs/drivers/oprofile/buffer_sync.c	2003-04-05 18:44:49.000000000 +0100
+++ linux-me/drivers/oprofile/buffer_sync.c	2003-05-03 20:10:44.000000000 +0100
@@ -58,8 +58,8 @@
  * must concern ourselves with. First, when a task is about to
  * exit (exit_mmap()), we should process the buffer to deal with
  * any samples in the CPU buffer, before we lose the ->mmap information
- * we need. Second, a task may unmap (part of) an executable mmap,
- * so we want to process samples before that happens too
+ * we need. It is vital to get this case correct, otherwise we can
+ * end up trying to access a freed task_struct.
  */
 static int mm_notify(struct notifier_block * self, unsigned long val, void * data)
 {
@@ -67,6 +67,29 @@
 	return 0;
 }
 
+
+/* Second, a task may unmap (part of) an executable mmap,
+ * so we want to process samples before that happens too. This is merely
+ * a QOI issue not a correctness one.
+ */
+static int munmap_notify(struct notifier_block * self, unsigned long val, void * data)
+{
+	/* Note that we cannot sync the buffers directly, because we might end up
+	 * taking the the mmap_sem that we hold now inside of event_buffer_read()
+	 * on a page fault, whilst holding buffer_sem - deadlock.
+	 *
+	 * This would mean a threaded reader of the event buffer, but we should
+	 * prevent it anyway.
+	 *
+	 * Delaying the work in a context that doesn't hold the mmap_sem means
+	 * that we won't lose samples from other mappings that current() may
+	 * have. Note that either way, we lose any pending samples for what is
+	 * being unmapped.
+	 */
+	schedule_work(&sync_wq);
+	return 0;
+}
+
  
 /* We need to be told about new modules so we don't attribute to a previously
  * loaded module, or drop the samples on the floor.
@@ -92,7 +115,7 @@
 };
 
 static struct notifier_block exec_unmap_nb = {
-	.notifier_call	= mm_notify,
+	.notifier_call	= munmap_notify,
 };
 
 static struct notifier_block exit_mmap_nb = {