| Date | Tue, 4 Mar 2014 14:27:27 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH 05/48] percpu: Add preemption checks to __this_cpu ops |
| |
On Fri, 14 Feb 2014 14:18:46 -0600 Christoph Lameter <cl@linux.com> wrote:
> [Patch depends on another patch in this series that introduces raw_cpu_ops] > > We define a check function in order to avoid trouble with the > include files. Then the higher level __this_cpu macros are > modified to invoke the preemption check. > > --- linux.orig/lib/smp_processor_id.c 2014-01-30 14:40:50.936519233 -0600 > +++ linux/lib/smp_processor_id.c 2014-01-30 14:40:50.936519233 -0600 > @@ -7,7 +7,7 @@ > #include <linux/kallsyms.h> > #include <linux/sched.h> > > -notrace unsigned int debug_smp_processor_id(void) > +notrace static unsigned int check_preemption_disabled(char *what) > { > int this_cpu = raw_smp_processor_id(); > > @@ -38,9 +38,9 @@ > if (!printk_ratelimit()) > goto out_enable; > > - printk(KERN_ERR "BUG: using smp_processor_id() in preemptible [%08x] " > - "code: %s/%d\n", > - preempt_count() - 1, current->comm, current->pid); > + printk(KERN_ERR "BUG: using %s in preemptible [%08x] code: %s/%d\n", > + what, preempt_count() - 1, current->comm, current->pid); > + > print_symbol("caller is %s\n", (long)__builtin_return_address(0)); > dump_stack();
I wonder if there's any point in printing __builtin_return_address. Doesn't dump_stack() tell us the same thing?
> @@ -50,5 +50,17 @@ > return this_cpu; > } > > +notrace unsigned int debug_smp_processor_id(void) > +{ > + return check_preemption_disabled("smp_processor_id()"); > +} > EXPORT_SYMBOL(debug_smp_processor_id); > > +notrace void __this_cpu_preempt_check(const char *op) > +{ > + char text[40]; > + > + snprintf(text, sizeof(text), "__this_cpu_%s()", op); > + check_preemption_disabled(text); > +}
I'd like to see a comment here telling scared readers why this can never overflow text[].
|