| Message ID | alpine.DEB.2.10.1403181053110.23935@nuc (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 18 Mar 2014 10:54:06 -0500 (CDT) Christoph Lameter <cl@linux.com> wrote: > On Tue, 18 Mar 2014, Grygorii Strashko wrote: > > > Any way, I can boot and console works fine with your change :) > > Thanks. > > Ok here is the properly formatted patch: > > > Subject: preemption_checks: Avoid snprintf before checking error conditions > > snprintf can cause hangs. This is weird. How the heck can snprintf() fail if called too early? All it does is shuffle chars around in memory. The only external dependency I'm seeing is a WARN_ON() which presumably didn't trigger anyway. I'm suspecting a misdiagnosis here. Otherwise, we seriously need to fix snprintf(), not work around it! Also, what does "before checking error conditions" refer to? Does this mean you know why snprintf() failed?? > Move the string processing into the function > so that the string operations only occur when necessary after the > conditions have been checked. > > Tested-by: Grygorii Strashko <grygorii.strashko@ti.com> Grygorii, thanks for testing linux-next on unusual machines - it's most helpful. > Signed-off-by: Christoph Lameter <cl@linux.com>
On 03/18/2014 11:37 PM, Andrew Morton wrote:> On Tue, 18 Mar 2014 10:54:06 -0500 (CDT) Christoph Lameter <cl@linux.com> wrote: > >> On Tue, 18 Mar 2014, Grygorii Strashko wrote: >> >>> Any way, I can boot and console works fine with your change :) >>> Thanks. >> >> Ok here is the properly formatted patch: >> >> >> Subject: preemption_checks: Avoid snprintf before checking error conditions >> >> snprintf can cause hangs. > > This is weird. How the heck can snprintf() fail if called too early? > All it does is shuffle chars around in memory. The only external > dependency I'm seeing is a WARN_ON() which presumably didn't trigger > anyway. > > I'm suspecting a misdiagnosis here. Otherwise, we seriously need to > fix snprintf(), not work around it! Not sure I can run debugger fast :(, but I'll try. > > Also, what does "before checking error conditions" refer to? Does this > mean you know why snprintf() failed?? Just an assumption, May be the problem here not in snprintf, but in stack. Looks like if I reduce stack usage the issue is gone: char text[10]; > >> Move the string processing into the function >> so that the string operations only occur when necessary after the >> conditions have been checked. >> >> Tested-by: Grygorii Strashko <grygorii.strashko@ti.com> > > Grygorii, thanks for testing linux-next on unusual machines - it's most > helpful. > Regards, -grygorii
On Tue, 18 Mar 2014, Andrew Morton wrote: > > snprintf can cause hangs. > > This is weird. How the heck can snprintf() fail if called too early? > All it does is shuffle chars around in memory. The only external > dependency I'm seeing is a WARN_ON() which presumably didn't trigger > anyway. > > I'm suspecting a misdiagnosis here. Otherwise, we seriously need to > fix snprintf(), not work around it! > > Also, what does "before checking error conditions" refer to? Does this > mean you know why snprintf() failed?? No I dont. I only know that this fixes Grygorii's issues. There could be numerous arch specific per cpu setup issues going on that may impact on snprintf. If I move it behind the checks then I can avoid using snprintf.
Index: linux/lib/smp_processor_id.c =================================================================== --- linux.orig/lib/smp_processor_id.c 2014-03-18 09:36:31.330450525 -0500 +++ linux/lib/smp_processor_id.c 2014-03-18 09:36:37.822315534 -0500 @@ -7,7 +7,8 @@ #include <linux/kallsyms.h> #include <linux/sched.h> -notrace static unsigned int check_preemption_disabled(char *what) +notrace static unsigned int check_preemption_disabled(const char *what1, + const char *what2) { int this_cpu = raw_smp_processor_id(); @@ -38,8 +39,8 @@ if (!printk_ratelimit()) goto out_enable; - printk(KERN_ERR "BUG: using %s in preemptible [%08x] code: %s/%d\n", - what, preempt_count() - 1, current->comm, current->pid); + printk(KERN_ERR "BUG: using %s%s() in preemptible [%08x] code: %s/%d\n", + what1, what2, preempt_count() - 1, current->comm, current->pid); print_symbol("caller is %s\n", (long)__builtin_return_address(0)); dump_stack(); @@ -52,15 +53,12 @@ notrace unsigned int debug_smp_processor_id(void) { - return check_preemption_disabled("smp_processor_id()"); + 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); + check_preemption_disabled("__this_cpu_", op); } EXPORT_SYMBOL(__this_cpu_preempt_check);