Re: [PATCH] enable/disable profiling via proc/sysctl

[Levent Serinol <lserinol@gmail.com>]

Hi William,

 You're right. As you suggest I reverted memory barrier part. Also,
added memory barriers while allocating and freeing memory.

Can you confirm that if it's okay now or missed anything else ?

PS. I couldn't figure out how to reverse function of hotcpu_notifier.
That's why I used  register_cpu_notifier and #ifdefs.



On 6/27/05, William Lee Irwin III <wli@holomorphy.com> wrote:
> On Mon, Jun 27, 2005 at 03:13:13PM +0300, Levent Serinol wrote:
> > +#ifdef CONFIG_SMP
> > +int remove_hash_tables(void)
> > +{
> > +     int cpu;
> > +
> > +
> > +     for_each_online_cpu(cpu) {
> > +             struct page *page;
> > +
> > +             if (per_cpu(cpu_profile_hits, cpu)[0]) {
> > +                     page = virt_to_page(per_cpu(cpu_profile_hits, cpu)[0]);
> > +                     per_cpu(cpu_profile_hits, cpu)[0] = NULL;
> > +                     __free_page(page);
> > +             }
> > +             if (per_cpu(cpu_profile_hits, cpu)[1]) {
> > +                     page = virt_to_page(per_cpu(cpu_profile_hits, cpu)[1]);
> > +                     per_cpu(cpu_profile_hits, cpu)[1] = NULL;
> > +                     __free_page(page);
> > +             }
> > +     }
> > +     return -1;
> 
> Big problem. Timer interrupts can be firing while this is in progress.
> The reason for profile_nop() is so that the code I have running during
> timer interrupts (protected by local_irq_save()) runs to completion,
> where it will afterward discover the flags or variables set to updated
> states. You'll probably have to add more (e.g. memory barriers) since
> you can tolerate less once you're dynamically allocating and freeing etc.
> 
> 
> On Mon, Jun 27, 2005 at 03:13:13PM +0300, Levent Serinol wrote:
> > @@ -560,7 +668,11 @@ static int __init create_proc_profile(vo
> >               return 0;
> >       entry->proc_fops = &proc_profile_operations;
> >       entry->size = (1+prof_len) * sizeof(atomic_t);
> > -     hotcpu_notifier(profile_cpu_callback, 0);
> > +#ifdef CONFIG_HOTPLUG_CPU
> > +     register_cpu_notifier(&profile_cpu_notifier);
> > +#endif
> > +     profile_params[0] = prof_on;
> > +     profile_params[1] = prof_shift;
> >       return 0;
> >  }
> >  module_init(create_proc_profile);
> 
> hotcpu_notifier() is there to avoid the #ifdef; you shouldn't need to
> rearrange that.
> 
> Anyway, just keep fixing it up. There should be a few more after this.
> 
> 
> -- wli
> 


-- 

Stay out of the road, if you want to grow old. 
~ Pink Floyd ~.

diff -uprN -X dontdiff linux-2.6.12-rc6.orig/include/linux/profile.h linux-2.6.12-rc6/include/linux/profile.h
--- linux-2.6.12-rc6.orig/include/linux/profile.h	2005-03-02 09:38:08.000000000 +0200
+++ linux-2.6.12-rc6/include/linux/profile.h	2005-06-27 10:19:43.000000000 +0300
@@ -6,14 +6,21 @@
 #include <linux/kernel.h>
 #include <linux/config.h>
 #include <linux/init.h>
+#include <linux/sysctl.h>
 #include <linux/cpumask.h>
 #include <asm/errno.h>
 
 #define CPU_PROFILING	1
 #define SCHED_PROFILING	2
 
+struct ctl_table;
+struct file;
 struct proc_dir_entry;
 struct pt_regs;
+extern int profile_params[];
+int profile_sysctl_handler(ctl_table *table, int write,
+               struct file *file, void __user *buffer, size_t
+*length, loff_t *ppos);
 
 /* init basic kernel profiler */
 void __init profile_init(void);
diff -uprN -X dontdiff linux-2.6.12-rc6.orig/include/linux/sysctl.h linux-2.6.12-rc6/include/linux/sysctl.h
--- linux-2.6.12-rc6.orig/include/linux/sysctl.h	2005-06-27 18:25:13.000000000 +0300
+++ linux-2.6.12-rc6/include/linux/sysctl.h	2005-06-27 10:15:54.000000000 +0300
@@ -136,6 +136,7 @@ enum
 	KERN_UNKNOWN_NMI_PANIC=66, /* int: unknown nmi panic flag */
 	KERN_BOOTLOADER_TYPE=67, /* int: boot loader type */
 	KERN_RANDOMIZE=68, /* int: randomize virtual address space */
+	KERN_PROFILE=69, /* int: profile on/off */
 };
 
 
diff -uprN -X dontdiff linux-2.6.12-rc6.orig/kernel/profile.c linux-2.6.12-rc6/kernel/profile.c
--- linux-2.6.12-rc6.orig/kernel/profile.c	2005-06-27 18:25:13.000000000 +0300
+++ linux-2.6.12-rc6/kernel/profile.c	2005-06-27 18:23:47.000000000 +0300
@@ -21,7 +21,6 @@
 #include <linux/mm.h>
 #include <linux/cpumask.h>
 #include <linux/cpu.h>
-#include <linux/profile.h>
 #include <linux/highmem.h>
 #include <asm/sections.h>
 #include <asm/semaphore.h>
@@ -37,9 +36,12 @@ struct profile_hit {
 /* Oprofile timer tick hook */
 int (*timer_hook)(struct pt_regs *);
 
+struct semaphore prof_sem;
+int profile_params[2] = {0, 0};
 static atomic_t *prof_buffer;
 static unsigned long prof_len, prof_shift;
 static int prof_on;
+static int prof_booton;
 static cpumask_t prof_cpu_mask = CPU_MASK_ALL;
 #ifdef CONFIG_SMP
 static DEFINE_PER_CPU(struct profile_hit *[2], cpu_profile_hits);
@@ -74,12 +76,14 @@ __setup("profile=", profile_setup);
 
 void __init profile_init(void)
 {
+	init_MUTEX(&prof_sem);
 	if (!prof_on) 
 		return;
  
 	/* only text is profiled */
 	prof_len = (_etext - _stext) >> prof_shift;
 	prof_buffer = alloc_bootmem(prof_len*sizeof(atomic_t));
+	prof_booton = 1;
 }
 
 /* Profile event notifications */
@@ -367,6 +371,12 @@ static int __devinit profile_cpu_callbac
 	}
 	return NOTIFY_OK;
 }
+static struct notifier_block profile_cpu_notifier =
+{
+         .notifier_call = profile_cpu_callback,
+         .priority = 0,
+};
+
 #endif /* CONFIG_HOTPLUG_CPU */
 #else /* !CONFIG_SMP */
 #define profile_flip_buffers()		do { } while (0)
@@ -500,11 +510,11 @@ static struct file_operations proc_profi
 };
 
 #ifdef CONFIG_SMP
-static void __init profile_nop(void *unused)
+static void profile_nop(void *unused)
 {
 }
 
-static int __init create_hash_tables(void)
+int create_hash_tables(void)
 {
 	int cpu;
 
@@ -548,6 +558,113 @@ out_cleanup:
 #define create_hash_tables()			({ 0; })
 #endif
 
+#ifdef CONFIG_SMP
+int remove_hash_tables(void)
+{
+	int cpu;
+
+
+	prof_on = 0;
+	smp_mb();
+	on_each_cpu(profile_nop, NULL, 0, 1);
+
+	for_each_online_cpu(cpu) {
+		struct page *page;
+
+		if (per_cpu(cpu_profile_hits, cpu)[0]) {
+			page = virt_to_page(per_cpu(cpu_profile_hits, cpu)[0]);
+			per_cpu(cpu_profile_hits, cpu)[0] = NULL;
+			__free_page(page);
+		}
+		if (per_cpu(cpu_profile_hits, cpu)[1]) {
+			page = virt_to_page(per_cpu(cpu_profile_hits, cpu)[1]);
+			per_cpu(cpu_profile_hits, cpu)[1] = NULL;
+			__free_page(page);
+		}
+	}
+	return -1;
+}
+#else
+#define remove_hash_tables()			({ 0; })
+#endif
+
+int profile_sysctl_handler(ctl_table *table, int write, struct file *file, 
+		void __user *buffer, size_t *length, loff_t *ppos)
+{
+	int err;
+	struct proc_dir_entry *entry;
+
+
+	down(&prof_sem);
+
+	if (prof_booton && write) 
+		goto out;
+
+	err=proc_dointvec(table, write, file, buffer, length, ppos);
+	if (err < 0 || !write)
+		goto out;
+
+	prof_shift = profile_params[1];
+
+	if (!profile_params[0]) {
+		if (prof_on) {
+			prof_on = 0;
+			remove_proc_entry("profile",NULL);
+#ifdef CONFIG_HOTPLUG_CPU
+			unregister_cpu_notifier(&profile_cpu_notifier);
+#endif
+			remove_hash_tables();
+			smp_mb();
+			on_each_cpu(profile_nop, NULL, 0, 1);
+			vfree(prof_buffer);
+			printk(KERN_INFO "kernel profiling disabled\n");
+		}
+	}
+
+	if ((profile_params[0]==SCHED_PROFILING) || (profile_params[0]==CPU_PROFILING)) {
+		if (prof_on)
+			goto out;
+        	prof_len = (_etext - _stext) >> prof_shift;
+		smp_mb();
+        	prof_buffer = vmalloc(prof_len*sizeof(atomic_t));
+		if (!prof_buffer)
+			goto out;
+		if (create_hash_tables()) {
+			vfree(prof_buffer);
+			goto out;
+		}
+		prof_on = profile_params[0];
+		if (!(entry = create_proc_entry("profile", S_IWUSR | S_IRUGO, NULL))) {
+			remove_hash_tables();
+			smp_mb();
+			on_each_cpu(profile_nop, NULL, 0, 1);
+			vfree(prof_buffer);
+			goto out;
+		}
+		entry->proc_fops = &proc_profile_operations;
+		entry->size = (1+prof_len) * sizeof(atomic_t);
+#ifdef CONFIG_HOTPLUG_CPU
+		register_cpu_notifier(&profile_cpu_notifier);
+#endif
+		profile_discard_flip_buffers();
+        	memset(prof_buffer, 0, prof_len * sizeof(atomic_t));
+			switch(prof_on) {
+			case SCHED_PROFILING:
+				printk(KERN_INFO "kernel schedule profiling enabled (shift: %ld)\n",
+						prof_shift);
+				break;
+			case CPU_PROFILING:
+				printk(KERN_INFO "kernel profiling enabled (shift: %ld)\n",
+						prof_shift);
+				break;
+			} 
+	}
+
+out:
+	up(&prof_sem);
+	return 0;
+}
+
 static int __init create_proc_profile(void)
 {
 	struct proc_dir_entry *entry;
@@ -560,7 +677,11 @@ static int __init create_proc_profile(vo
 		return 0;
 	entry->proc_fops = &proc_profile_operations;
 	entry->size = (1+prof_len) * sizeof(atomic_t);
-	hotcpu_notifier(profile_cpu_callback, 0);
+#ifdef CONFIG_HOTPLUG_CPU
+	register_cpu_notifier(&profile_cpu_notifier);
+#endif
+	profile_params[0] = prof_on;
+	profile_params[1] = prof_shift;
 	return 0;
 }
 module_init(create_proc_profile);
diff -uprN -X dontdiff linux-2.6.12-rc6.orig/kernel/sysctl.c linux-2.6.12-rc6/kernel/sysctl.c
--- linux-2.6.12-rc6.orig/kernel/sysctl.c	2005-06-27 18:25:13.000000000 +0300
+++ linux-2.6.12-rc6/kernel/sysctl.c	2005-06-27 10:19:07.000000000 +0300
@@ -21,6 +21,7 @@
 #include <linux/config.h>
 #include <linux/module.h>
 #include <linux/mm.h>
+#include <linux/profile.h>
 #include <linux/swap.h>
 #include <linux/slab.h>
 #include <linux/sysctl.h>
@@ -642,7 +643,15 @@ static ctl_table kern_table[] = {
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
-
+	{
+		.ctl_name       = KERN_PROFILE,
+		.procname       = "profile",
+		.data           = &profile_params,
+		.maxlen         = 2*sizeof(int),
+		.mode           = 0644,
+		.proc_handler   = &profile_sysctl_handler,
+		.strategy       = &sysctl_intvec,
+	},
 	{ .ctl_name = 0 }
 };