enable/disable profiling via proc/sysctl
PATCH 1/1 enable/disable profiling via proc/sysctl
This patch enables controlling kernel profiling through proc/sysctl inferface.
With this patch profiling will be available without rebooting the machine (especially for
production servers) with some drawbacks of vmalloc(tlb). So, bootime algorithm part is left unchanged for anyone who wishes to use profiling as usual without tlb drawback by rebooting the machine.
PS. This patch is against 2.6.12-rc6
linux/sysctl.h renamed from linux/sysctl.h.org +1 -0
@@ -136,6 +136,7 @@ enumKERN_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 */ };
linux/profile.h renamed from linux/profile.h.org +6 -0
@@ -6,14 +6,20 @@#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 ; +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 );
profile.c renamed from profile.c.org +105 -3
146 lines not on this page · raw message
sysctl.c renamed from sysctl.c.org +11 -1
@@ -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>
@@ -65,6 +66,7 @@ extern int min_free_kbytes;extern int printk_ratelimit_jiffies;extern int printk_ratelimit_burst;extern int pid_max_min, pid_max_max; +extern int profile_params[];#if defined(CONFIG_X86_LOCAL_APIC ) && defined(CONFIG_X86 )int unknown_nmi_panic;
@@ -642,7 +644,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 } };
linux/sysctl.h
Quoted 12 lines · Levent Serinol
This patch enables controlling kernel profiling through proc/sysctl inferface.
With this patch profiling will be available without rebooting the machine (especially for
production servers) with some drawbacks of vmalloc(tlb). So, bootime algorithm part is left unchanged for anyone who wishes to use profiling as usual without tlb drawback by rebooting the machine.linux/sysctl.h renamed from linux/sysctl.h.org +0
-0
Patches should be in `patch -p1' form, please. See
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt
linux/sysctl.h
Quoted 2 lines
linux/sysctl.h renamed from linux/sysctl.h.org +2
-0+static int prof_on =0 ; +static int prof_booton =0 ;
There's no need to explicitly initialise these.
linux/sysctl.h
Quoted 7 lines
linux/sysctl.h renamed from linux/sysctl.h.org +7
-0+#ifdef CONFIG_SMP +static int remove_hash_tables (void ) +{ +int cpu; + +smp_mb (); +on_each_cpu (profile_nop,NULL ,0 ,1 );
Why?
Quoted 4 lines
linux/sysctl.h renamed from linux/sysctl.h.org +4
-0+for_each_online_cpu (cpu) { +struct page *page; + +if (per_cpu (cpu_profile_hits, cpu)[0 ]) {
linux/sysctl.h
Quoted 10 lines
linux/sysctl.h renamed from linux/sysctl.h.org +10
-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); + } + }
Can this race against itself? If two cpus run the sysctl at the same time? We seem to have lock_kernel() coverage. It's be nice to do something firmer.
Quoted 4 lines
linux/sysctl.h renamed from linux/sysctl.h.org +4
-0+int profile_sysctl_handler (ctl_table *table,int write, +struct file *file,void __user *buffer,size_t *length,loff_t *ppos) +{ +int err;
linux/sysctl.h
Quoted 10 lines
linux/sysctl.h renamed from linux/sysctl.h.org +10
-0+struct proc_dir_entry *entry; + +if (prof_booton && write)return 0 ; + err=proc_dointvec (table, write, file, buffer, length, ppos); +if ((err >=0 ) && write) { + prof_shift = profile_params[1 ]; +switch (profile_params[0 ]) + { +case 0 : +if (prof_on) {
Coding style is all over the place here, as well as in most of the rest of the patch.
if (prof_booton && write)
return 0;
err = proc_dointvec(table, write, file, buffer, length, ppos);
if (err >= 0 && write) {
prof_shift = profile_params[1];
switch (profile_params[0])
{
case 0:
if (prof_on) {Every line was changed there...
Also, doing multiple returns per function is unpopular, although the very-early
if (foo) return <something>;
right at the top of the function is OK. You can use
if (err < 0 || !write) goto out;
to save a tab stop.
linux/sysctl.h
Quoted 2 lines
linux/sysctl.h renamed from linux/sysctl.h.org +2
-0+ } +break ;
} break;
linux/sysctl.h
Quoted 1 line
linux/sysctl.h renamed from linux/sysctl.h.org +1
-0+case SCHED_PROFILING ||CPU_PROFILING :
eh? I'm surprised the compiler swallowed that. I guess it's the same as `case 1:'. Looks like a bug though.
Quoted 27 lines
linux/sysctl.h renamed from linux/sysctl.h.org +27
-0+if (prof_on)return -1 ; + prof_len = (_etext - _stext) >> prof_shift; + prof_buffer =vmalloc (prof_len*sizeof (atomic_t)); +if (!prof_buffer)return (-ENOMEM ); +if (create_hash_tables ()) { +vfree (prof_buffer); +return -1 ; + } + prof_on = profile_params[0 ]; +if (!(entry =create_proc_entry ("profile" ,S_IWUSR |S_IRUGO ,NULL ))) { +remove_hash_tables (); +vfree (prof_buffer); +return 0 ; + } + 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 ;
linux/sysctl.h
Quoted 10 lines
linux/sysctl.h renamed from linux/sysctl.h.org +10
-0+case CPU_PROFILING :printk (KERN_INFO +"kernel profiling enabled (shift: %ld)\n" , + prof_shift); +break ; + } +break ; + } + } +return 0 ; +}
Documentation/CodingStyle is your friend ;)
sysctl.c extern int min_free_kbytes; · line 69
Quoted 4 lines
sysctl.c renamed from sysctl.c.org +1
-0+extern int profile_params[];
Try to place this declaration in a header.
What locking protects prof_boot_on()? lock_kernel() won't be sufficient because we're doing sleeping allocations in here.
I suspect it would be best to whap a semaphore around the whole thing.
Hi Andrew,
The fixed patch is attached. I hope, it's ok this time.
Thanks,
Quoted 158 lines · Andrew Morton
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300
Quoted 12 lines · Levent Serinol
This patch enables controlling kernel profiling through proc/sysctl inferface.
With this patch profiling will be available without rebooting the machine (especially for
production servers) with some drawbacks of vmalloc(tlb). So, bootime algorithm part is left unchanged for anyone who wishes to use profiling as usual without tlb drawback by rebooting the machine.linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300 renamed from linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300 +0
-0Patches should be in `patch -p1' form, please. See
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txtlinux/sysctl.h 2005-06-25 15:05:06.000000000 +0300
Quoted 2 lines
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300 renamed from linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300 +2
-0+static int prof_on = 0; +static int prof_booton = 0;There's no need to explicitly initialise these.
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300
Quoted 7 lines
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300 renamed from linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300 +7
-0+#ifdef CONFIG_SMP +static int remove_hash_tables(void) +{ + int cpu; + + smp_mb(); + on_each_cpu(profile_nop, NULL, 0, 1);Why?
Quoted 4 lines
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300 renamed from linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300 +4
-0+ for_each_online_cpu(cpu) { + struct page *page; + + if (per_cpu(cpu_profile_hits, cpu)[0]) {linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300
Quoted 10 lines
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300 renamed from linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300 +10
-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); + } + }Can this race against itself? If two cpus run the sysctl at the same time? We seem to have
lock_kernel()coverage. It's be nice to do something firmer.Quoted 4 lines
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300 renamed from linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300 +4
-0+int profile_sysctl_handler(ctl_table *table, int write, + struct file *file, void __user *buffer, size_t *length, loff_t *ppos) +{ + int err;linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300
Quoted 10 lines
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300 renamed from linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300 +10
-0+ struct proc_dir_entry *entry; + + if (prof_booton && write) return 0; + err=proc_dointvec(table, write, file, buffer, length, ppos); + if ((err >= 0) && write) { + prof_shift = profile_params[1]; + switch(profile_params[0]) + { + case 0: + if (prof_on) {Coding style is all over the place here, as well as in most of the rest of the patch.
if (prof_booton && write) return 0; err = proc_dointvec(table, write, file, buffer, length, ppos); if (err >= 0 && write) { prof_shift = profile_params[1]; switch (profile_params[0]) { case 0: if (prof_on) {Every line was changed there...
Also, doing multiple returns per function is unpopular, although the very-early
if (foo) return <something>;right at the top of the function is OK. You can use
if (err < 0 || !write) goto out;to save a tab stop.
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300
Quoted 2 lines
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300 renamed from linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300 +2
-0+ } + break;} break;Quoted 1 line
+ case
SCHED_PROFILING||CPU_PROFILING:eh? I'm surprised the compiler swallowed that. I guess it's the same as `case 1:'. Looks like a bug though.
Quoted 27 lines
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300 renamed from linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300 +27
-0+ if (prof_on) return -1; + prof_len = (_etext - _stext) >> prof_shift; + prof_buffer = vmalloc(prof_len*sizeof(atomic_t)); + if (!prof_buffer) return(-ENOMEM); + if (create_hash_tables()) { + vfree(prof_buffer); + return -1; + } + prof_on = profile_params[0]; + if (!(entry = create_proc_entry("profile", S_IWUSR | S_IRUGO, NULL))) { + remove_hash_tables(); + vfree(prof_buffer); + return 0; + } + 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;linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300
Quoted 10 lines
linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300 renamed from linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300 +10
-0+ case CPU_PROFILING:printk(KERN_INFO + "kernel profiling enabled (shift: %ld)\n", + prof_shift); + break; + } + break; + } + } + return 0; +}
Documentation/CodingStyleis your friend ;)sysctl.c 2005-06-26 02:06:23.000000000 +0300
Quoted 4 lines
sysctl.c 2005-06-26 02:06:23.000000000 +0300 renamed from sysctl.c.org 2005-06-13 16:05:23.000000000 +0300 +1
-0+extern int profile_params[];Try to place this declaration in a header.
What locking protects
prof_boot_on()?lock_kernel()won't be sufficient because we're doing sleeping allocations in here.I suspect it would be best to whap a semaphore around the whole thing.
Signature · 2 lines
Stay out of the road, if you want to grow old. ~ Pink Floyd ~.
include/linux/profile.h +7 -0
@@ -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 );
include/linux/sysctl.h +1 -0
@@ -136,6 +136,7 @@ enumKERN_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 */ };
kernel/profile.c +116 -4
174 lines not on this page · raw message
kernel/sysctl.c +10 -1
@@ -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 } };
Quoted 11 lines · Levent Serinol
kernel/profile.c +11
-0+#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 ]);
kernel/profile.c out_cleanup: · line 581
Quoted 10 lines
kernel/profile.c +10
-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.
kernel/profile.c static int __init create_proc_profile(vo · line 678
Quoted 13 lines · Levent Serinol
kernel/profile.c +5
-1@@ -560,7 +668,11 @@ static int __init create_proc_profile(voreturn 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
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.
Quoted 54 lines · William Lee Irwin III
Quoted 11 lines · Levent Serinol
(quoted diff) +11
-0+#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]);Review
Quoted 10 lines
(quoted diff) +10
-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 bylocal_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.Review static int __init create_proc_profile(vo · line 678
Quoted 13 lines · Levent Serinol
(quoted diff) +5
-1@@ -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
Signature · 2 lines
Stay out of the road, if you want to grow old. ~ Pink Floyd ~.
include/linux/profile.h +7 -0
@@ -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 );
include/linux/sysctl.h +1 -0
@@ -136,6 +136,7 @@ enumKERN_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 */ };
kernel/profile.c +125 -4
183 lines not on this page · raw message
kernel/sysctl.c +10 -1
@@ -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 } };