lkml.org 
[lkml]   [2005]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] enable/disable profiling via proc/sysctl
    Hi Andrew,

    The fixed patch is attached. I hope, it's ok this time.

    Thanks,


    On 6/27/05, Andrew Morton <akpm@osdl.org> wrote:
    > Levent Serinol <lserinol@gmail.com> wrote:
    > >
    > > 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.
    >
    >
    > > --- include/linux/sysctl.h.org 2005-06-13 16:05:17.000000000 +0300
    > > +++ include/linux/sysctl.h 2005-06-25 15:05:06.000000000 +0300
    >
    > Patches should be in `patch -p1' form, please. See
    > http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt
    >
    > > +static int prof_on = 0;
    > > +static int prof_booton = 0;
    >
    > There's no need to explicitly initialise these.
    >
    > > +#ifdef CONFIG_SMP
    > > +static int remove_hash_tables(void)
    > > +{
    > > + int cpu;
    > > +
    > > + smp_mb();
    > > + on_each_cpu(profile_nop, NULL, 0, 1);
    >
    > Why?
    >
    > > + 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);
    > > + }
    > > + }
    >
    > 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.
    >
    > > +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;
    > > +
    > > + 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.
    >
    > > + }
    > > + break;
    >
    > }
    > break;
    >
    > > + 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.
    >
    > > + 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;
    > > + case CPU_PROFILING:printk(KERN_INFO
    > > + "kernel profiling enabled (shift: %ld)\n",
    > > + prof_shift);
    > > + break;
    > > + }
    > > + break;
    > > + }
    > > + }
    > > + return 0;
    > > +}
    >
    > Documentation/CodingStyle is your friend ;)
    >
    > > --- kernel/sysctl.c.org 2005-06-13 16:05:23.000000000 +0300
    > > +++ kernel/sysctl.c 2005-06-26 02:06:23.000000000 +0300
    > > ...
    > > +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.
    >


    --

    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 15:06:23.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 15:06:23.000000000 +0300
    +++ linux-2.6.12-rc6/kernel/profile.c 2005-06-27 15:05:07.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,104 @@ out_cleanup:
    #define create_hash_tables() ({ 0; })
    #endif

    +#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;
    +}
    +#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();
    + 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;
    + 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();
    + 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 +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);
    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 15:06:23.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 }
    };
    \
     
     \ /
      Last update: 2005-06-27 14:18    [W:0.062 / U:180.460 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site