* [PATCH 0/2 v2] speeding up cpu_up() @ 2015-05-08 6:37 Len Brown 2015-05-08 6:37 ` [PATCH 1/2] x86: replace cpu_up hard-coded mdelay with variable udelay Len Brown 0 siblings, 1 reply; 11+ messages in thread From: Len Brown @ 2015-05-08 6:37 UTC (permalink / raw) To: x86, linux-pm, linux-kernel Thanks for testing, Aravind, Borislav, I went with Ingo's suggestion to made this a quirk. However, I went with a white-list instead of a black-list, because fewer comparisons are needed for modern processors. Families are added infrequently, and finally, we get the option to stick something other than 0 in the table if it we need to. Let me know if I got the AMD family #'s right. thanks, -Len Brown, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] x86: replace cpu_up hard-coded mdelay with variable udelay 2015-05-08 6:37 [PATCH 0/2 v2] speeding up cpu_up() Len Brown @ 2015-05-08 6:37 ` Len Brown 2015-05-08 6:37 ` [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay Len Brown 0 siblings, 1 reply; 11+ messages in thread From: Len Brown @ 2015-05-08 6:37 UTC (permalink / raw) To: x86, linux-pm, linux-kernel; +Cc: Len Brown From: Len Brown <len.brown@intel.com> Replace the hard-coded mdelay(10) in cpu_up() to a variable udelay. Add a boot-time override, "cpu_init_udelay=N" Default unchanged at 10ms on all systems. Signed-off-by: Len Brown <len.brown@intel.com> --- Documentation/kernel-parameters.txt | 6 ++++++ arch/x86/kernel/smpboot.c | 28 +++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index bfcb1a6..0a16309 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -737,6 +737,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted. cpuidle.off=1 [CPU_IDLE] disable the cpuidle sub-system + cpu_init_udelay=N + [X86] Delay for N microsec between assert and de-assert + of APIC INIT to start processors. This delay occurs + on every CPU online, such as boot, and resume from suspend. + Default: 10000 + cpcihp_generic= [HW,PCI] Generic port I/O CompactPCI driver Format: <first_slot>,<last_slot>,<port>,<enum_bit>[,<debug>] diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index febc6aa..76734f4 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -517,6 +517,32 @@ void __inquire_remote_apic(int apicid) } /* + * The Multiprocessor Specification 1.4 (1997) example code suggests + * that there should be a 10ms delay between the BSP asserting INIT + * and de-asserting INIT, when starting a remote processor. + * But that slows boot and resume on modern processors, which include + * many cores and don't require that delay. + * + * cmdline "init_cpu_udelay=" is available to specify this delay. + */ +#define UDELAY_10MS_DEFAULT 10000 + +static unsigned int init_udelay = UDELAY_10MS_DEFAULT; + +static int __init cpu_init_udelay(char *str) +{ + unsigned int new_udelay; + + get_option(&str, &new_udelay); + pr_debug("cpu_init_udelay=%d, was %d", new_udelay, init_udelay); + init_udelay = new_udelay; + + return 0; +} + +early_param("cpu_init_udelay", cpu_init_udelay); + +/* * Poke the other CPU in the eye via NMI to wake it up. Remember that the normal * INIT, INIT, STARTUP sequence will reset the chip hard for us, and this * won't ... remember to clear down the APIC, etc later. @@ -586,7 +612,7 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip) pr_debug("Waiting for send to finish...\n"); send_status = safe_apic_wait_icr_idle(); - mdelay(10); + udelay(init_udelay); pr_debug("Deasserting INIT\n"); -- 2.4.0.rc1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay 2015-05-08 6:37 ` [PATCH 1/2] x86: replace cpu_up hard-coded mdelay with variable udelay Len Brown @ 2015-05-08 6:37 ` Len Brown 2015-05-08 7:51 ` Ingo Molnar 2015-05-08 8:32 ` Borislav Petkov 0 siblings, 2 replies; 11+ messages in thread From: Len Brown @ 2015-05-08 6:37 UTC (permalink / raw) To: x86, linux-pm, linux-kernel; +Cc: Len Brown From: Len Brown <len.brown@intel.com> Modern processor familes are on a white-list to remove the costly cpu_init_udelay 10000. Unknown processor families get the traditional 10ms delay in cpu_up(). This seemed more efficient than forcing modern processors to exhaustively search a black-list having all the old processor families that should have a 10ms delay. For not only are new processor familes infrequently added, the white list also allows a delay other than 0, if needed. Signed-off-by: Len Brown <len.brown@intel.com> --- arch/x86/kernel/smpboot.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index 76734f4..34a08ff 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -76,6 +76,7 @@ #include <asm/i8259.h> #include <asm/realmode.h> #include <asm/misc.h> +#include <asm/cpu_device_id.h> /* State of each CPU */ DEFINE_PER_CPU(int, cpu_state) = { 0 }; @@ -521,14 +522,44 @@ void __inquire_remote_apic(int apicid) * that there should be a 10ms delay between the BSP asserting INIT * and de-asserting INIT, when starting a remote processor. * But that slows boot and resume on modern processors, which include - * many cores and don't require that delay. - * + * many cores and don't require that delay. Here we default to the + * legacy delay, but quirk new processors to skip the delay. * cmdline "init_cpu_udelay=" is available to specify this delay. */ #define UDELAY_10MS_DEFAULT 10000 static unsigned int init_udelay = UDELAY_10MS_DEFAULT; +static const struct x86_cpu_id init_udelay_ids[] = { + { X86_VENDOR_INTEL, 0x6, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, + { X86_VENDOR_AMD, 0x16, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, + { X86_VENDOR_AMD, 0x15, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, + { X86_VENDOR_AMD, 0x14, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, + { X86_VENDOR_AMD, 0x12, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, + { X86_VENDOR_AMD, 0x11, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, + { X86_VENDOR_AMD, 0x10, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, + { X86_VENDOR_AMD, 0xF, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, + {} +}; +MODULE_DEVICE_TABLE(x86cpu, init_udelay_ids); + +static void __init smp_quirk_init_udelay(void) +{ + const struct x86_cpu_id *id; + unsigned int new_udelay; + + id = x86_match_cpu(init_udelay_ids); + if (id == NULL) + return; /* if no match, keep default */ + + if (init_udelay != UDELAY_10MS_DEFAULT) + return; /* if cmdline changed from default, leave it alone */ + + new_udelay = (unsigned long) id->driver_data; + pr_debug("cpu_init_udelay quirk to %d, was %d", new_udelay, init_udelay); + init_udelay = new_udelay; +} + static int __init cpu_init_udelay(char *str) { unsigned int new_udelay; @@ -1196,6 +1227,8 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus) uv_system_init(); set_mtrr_aps_delayed_init(); + + smp_quirk_init_udelay(); } void arch_enable_nonboot_cpus_begin(void) -- 2.4.0.rc1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay 2015-05-08 6:37 ` [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay Len Brown @ 2015-05-08 7:51 ` Ingo Molnar 2015-05-08 8:23 ` Borislav Petkov 2015-05-08 8:32 ` Borislav Petkov 1 sibling, 1 reply; 11+ messages in thread From: Ingo Molnar @ 2015-05-08 7:51 UTC (permalink / raw) To: Len Brown; +Cc: x86, linux-pm, linux-kernel, Len Brown, Borislav Petkov * Len Brown <lenb@kernel.org> wrote: > From: Len Brown <len.brown@intel.com> > > Modern processor familes are on a white-list to remove > the costly cpu_init_udelay 10000. Unknown processor families > get the traditional 10ms delay in cpu_up(). > > This seemed more efficient than forcing modern processors > to exhaustively search a black-list having all the old > processor families that should have a 10ms delay. > For not only are new processor familes infrequently added, > the white list also allows a delay other than 0, if needed. > static unsigned int init_udelay = UDELAY_10MS_DEFAULT; > > +static const struct x86_cpu_id init_udelay_ids[] = { > + { X86_VENDOR_INTEL, 0x6, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > + { X86_VENDOR_AMD, 0x16, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > + { X86_VENDOR_AMD, 0x15, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > + { X86_VENDOR_AMD, 0x14, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > + { X86_VENDOR_AMD, 0x12, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > + { X86_VENDOR_AMD, 0x11, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > + { X86_VENDOR_AMD, 0x10, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > + { X86_VENDOR_AMD, 0xF, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > + {} > +}; So since especially AMD likes to iterate the family upwards, why not make this a simple open ended check: if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL && boot_cpu_data.x86 >= 6 || boot_cpu_data.x86_vendor == X86_VENDOR_AMD && boot_cpu_data.x86 >= 15) { ... 0 delay ... } ... which is much smaller and more future proof? Thanks, Ingo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay 2015-05-08 7:51 ` Ingo Molnar @ 2015-05-08 8:23 ` Borislav Petkov 0 siblings, 0 replies; 11+ messages in thread From: Borislav Petkov @ 2015-05-08 8:23 UTC (permalink / raw) To: Ingo Molnar; +Cc: Len Brown, x86, linux-pm, linux-kernel, Len Brown On Fri, May 08, 2015 at 09:51:11AM +0200, Ingo Molnar wrote: > > +static const struct x86_cpu_id init_udelay_ids[] = { > > + { X86_VENDOR_INTEL, 0x6, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > > + { X86_VENDOR_AMD, 0x16, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > > + { X86_VENDOR_AMD, 0x15, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > > + { X86_VENDOR_AMD, 0x14, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > > + { X86_VENDOR_AMD, 0x12, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > > + { X86_VENDOR_AMD, 0x11, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > > + { X86_VENDOR_AMD, 0x10, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > > + { X86_VENDOR_AMD, 0xF, X86_MODEL_ANY, X86_FEATURE_ANY, 0 }, > > + {} > > +}; > > So since especially AMD likes to iterate the family upwards, why not > make this a simple open ended check: > > if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL && > boot_cpu_data.x86 >= 6 || > boot_cpu_data.x86_vendor == X86_VENDOR_AMD && > boot_cpu_data.x86 >= 15) { > > ... 0 delay ... > } > > ... which is much smaller and more future proof? I was about to say that... -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay 2015-05-08 6:37 ` [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay Len Brown 2015-05-08 7:51 ` Ingo Molnar @ 2015-05-08 8:32 ` Borislav Petkov 2015-05-08 18:15 ` Len Brown 1 sibling, 1 reply; 11+ messages in thread From: Borislav Petkov @ 2015-05-08 8:32 UTC (permalink / raw) To: Len Brown; +Cc: x86, linux-pm, linux-kernel, Len Brown On Fri, May 08, 2015 at 02:37:55AM -0400, Len Brown wrote: > From: Len Brown <len.brown@intel.com> > > Modern processor familes are on a white-list to remove > the costly cpu_init_udelay 10000. Unknown processor families > get the traditional 10ms delay in cpu_up(). > > This seemed more efficient than forcing modern processors > to exhaustively search a black-list having all the old > processor families that should have a 10ms delay. > For not only are new processor familes infrequently added, > the white list also allows a delay other than 0, if needed. > > Signed-off-by: Len Brown <len.brown@intel.com> > --- > arch/x86/kernel/smpboot.c | 37 +++++++++++++++++++++++++++++++++++-- > 1 file changed, 35 insertions(+), 2 deletions(-) ... > +static void __init smp_quirk_init_udelay(void) > +{ > + const struct x86_cpu_id *id; > + unsigned int new_udelay; > + > + id = x86_match_cpu(init_udelay_ids); > + if (id == NULL) > + return; /* if no match, keep default */ > + > + if (init_udelay != UDELAY_10MS_DEFAULT) > + return; /* if cmdline changed from default, leave it alone */ > + > + new_udelay = (unsigned long) id->driver_data; > + pr_debug("cpu_init_udelay quirk to %d, was %d", new_udelay, init_udelay); Can we make this printk(KERN_DEBUG please? I'd like to be able to slap "debug" on the command line and not recompile the kernel. And no, dyndbg="file smpboot.c +p" or whatever the syntax is, simply doesn't scale if I want to see all debug messages from early boot. Thanks. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay 2015-05-08 8:32 ` Borislav Petkov @ 2015-05-08 18:15 ` Len Brown 2015-05-08 18:27 ` Borislav Petkov ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Len Brown @ 2015-05-08 18:15 UTC (permalink / raw) To: Borislav Petkov; +Cc: X86 ML, Linux PM list, linux-kernel, Len Brown On Fri, May 8, 2015 at 4:32 AM, Borislav Petkov <bp@alien8.de> wrote: >> + pr_debug("cpu_init_udelay quirk to %d, was %d", new_udelay, init_udelay); > > Can we make this printk(KERN_DEBUG please? > > I'd like to be able to slap "debug" on the command line and not > recompile the kernel. And no, dyndbg="file smpboot.c +p" or whatever the > syntax is, simply doesn't scale if I want to see all debug messages from > early boot. I agree with you. Further, you made me think about it, and while it was helpful when I wrote the patch, I don't think any printk() is needed upstream. see v3 just sent. Len Brown, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay 2015-05-08 18:15 ` Len Brown @ 2015-05-08 18:27 ` Borislav Petkov 2015-05-09 7:22 ` Ingo Molnar 2015-05-09 7:24 ` Ingo Molnar 2 siblings, 0 replies; 11+ messages in thread From: Borislav Petkov @ 2015-05-08 18:27 UTC (permalink / raw) To: Len Brown; +Cc: X86 ML, Linux PM list, linux-kernel, Len Brown On Fri, May 08, 2015 at 02:15:42PM -0400, Len Brown wrote: > I agree with you. Further, you made me think about it, and while it > was helpful when I wrote the patch, I don't think any printk() is > needed upstream. Yeah, you're right. If a user did override it with cpu_init_udelay, it'll be in /proc/cmdline. And dmesg, and so on... > see v3 just sent. Looks ok. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay 2015-05-08 18:15 ` Len Brown 2015-05-08 18:27 ` Borislav Petkov @ 2015-05-09 7:22 ` Ingo Molnar 2015-05-09 7:24 ` Ingo Molnar 2 siblings, 0 replies; 11+ messages in thread From: Ingo Molnar @ 2015-05-09 7:22 UTC (permalink / raw) To: Len Brown; +Cc: Borislav Petkov, X86 ML, Linux PM list, linux-kernel, Len Brown * Len Brown <lenb@kernel.org> wrote: > On Fri, May 8, 2015 at 4:32 AM, Borislav Petkov <bp@alien8.de> wrote: > > >> + pr_debug("cpu_init_udelay quirk to %d, was %d", new_udelay, init_udelay); > > > > Can we make this printk(KERN_DEBUG please? > > > > I'd like to be able to slap "debug" on the command line and not > > recompile the kernel. And no, dyndbg="file smpboot.c +p" or > > whatever the syntax is, simply doesn't scale if I want to see all > > debug messages from early boot. Ugh, so I see we have grown this gem some time ago: #if defined(CONFIG_DYNAMIC_DEBUG) /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */ #define pr_debug(fmt, ...) \ dynamic_pr_debug(fmt, ##__VA_ARGS__) I didn't even realize it's there and it happend 6 years ago, in a very unintuitively titled commit: 346e15beb534 driver core: basic infrastructure for per-module dynamic debug messages So in what way does that title tell us that all pr_debug() calls are redirected away if CONFIG_DYNAMIC_DEBUG is enabled (which distros do)? So could we instead either add a dyndbg=all variant, or make 'debug' trigger all dynamic_pr_debug() messages? Because this redirection breaks the whole pr_*() abstraction rather fundamentally, dyndebug stealing pr_debug() and hiding debug messages when the user specifically asked for them via 'debug' is pretty nasty IMHO ... Thanks, Ingo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay 2015-05-08 18:15 ` Len Brown 2015-05-08 18:27 ` Borislav Petkov 2015-05-09 7:22 ` Ingo Molnar @ 2015-05-09 7:24 ` Ingo Molnar 2015-05-09 8:04 ` Borislav Petkov 2 siblings, 1 reply; 11+ messages in thread From: Ingo Molnar @ 2015-05-09 7:24 UTC (permalink / raw) To: Len Brown Cc: Borislav Petkov, X86 ML, Linux PM list, linux-kernel, Len Brown, Jason Baron, Linus Torvalds, Andrew Morton, Greg Kroah-Hartman (Resending my reply with more dyn-debug folks Cc:-ed) * Len Brown <lenb@kernel.org> wrote: > On Fri, May 8, 2015 at 4:32 AM, Borislav Petkov <bp@alien8.de> wrote: > > >> + pr_debug("cpu_init_udelay quirk to %d, was %d", new_udelay, init_udelay); > > > > Can we make this printk(KERN_DEBUG please? > > > > I'd like to be able to slap "debug" on the command line and not > > recompile the kernel. And no, dyndbg="file smpboot.c +p" or > > whatever the syntax is, simply doesn't scale if I want to see all > > debug messages from early boot. Ugh, so I see we have grown this gem some time ago: #if defined(CONFIG_DYNAMIC_DEBUG) /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */ #define pr_debug(fmt, ...) \ dynamic_pr_debug(fmt, ##__VA_ARGS__) I didn't even realize it's there and it happend 6 years ago, in a very unintuitively titled commit: 346e15beb534 driver core: basic infrastructure for per-module dynamic debug messages So in what way does that title tell us that all pr_debug() calls are redirected away if CONFIG_DYNAMIC_DEBUG is enabled (which distros do)? So could we instead either add a dyndbg=all variant, or make 'debug' trigger all dynamic_pr_debug() messages? Because this redirection breaks the whole pr_*() abstraction rather fundamentally, dyndebug stealing pr_debug() and hiding debug messages when the user specifically asked for them via 'debug' is pretty nasty IMHO ... Thanks, Ingo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay 2015-05-09 7:24 ` Ingo Molnar @ 2015-05-09 8:04 ` Borislav Petkov 0 siblings, 0 replies; 11+ messages in thread From: Borislav Petkov @ 2015-05-09 8:04 UTC (permalink / raw) To: Ingo Molnar Cc: Len Brown, X86 ML, Linux PM list, linux-kernel, Len Brown, Jason Baron, Linus Torvalds, Andrew Morton, Greg Kroah-Hartman, Steven Rostedt On Sat, May 09, 2015 at 09:22:38AM +0200, Ingo Molnar wrote: > > * Len Brown <lenb@kernel.org> wrote: > > > On Fri, May 8, 2015 at 4:32 AM, Borislav Petkov <bp@alien8.de> wrote: > > > > >> + pr_debug("cpu_init_udelay quirk to %d, was %d", new_udelay, init_udelay); > > > > > > Can we make this printk(KERN_DEBUG please? > > > > > > I'd like to be able to slap "debug" on the command line and not > > > recompile the kernel. And no, dyndbg="file smpboot.c +p" or > > > whatever the syntax is, simply doesn't scale if I want to see all > > > debug messages from early boot. > > Ugh, so I see we have grown this gem some time ago: > > #if defined(CONFIG_DYNAMIC_DEBUG) > /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */ > #define pr_debug(fmt, ...) \ > dynamic_pr_debug(fmt, ##__VA_ARGS__) > > I didn't even realize it's there and it happend 6 years ago, in a very > unintuitively titled commit: > > 346e15beb534 driver core: basic infrastructure for per-module dynamic debug messages > > So in what way does that title tell us that all pr_debug() calls are > redirected away if CONFIG_DYNAMIC_DEBUG is enabled Yeah, this thing made pr_debug() completely unusable. > (which distros do)? /me checks. SUSE does, at least. > So could we instead either add a dyndbg=all variant, or make 'debug' > trigger all dynamic_pr_debug() messages? Not only that: #elif defined(DEBUG) #define pr_debug(fmt, ...) \ printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) now, AFAIU, this means if CONFIG_DYNAMIC_DEBUG=n, I still have to have DEBUG defined in order to see debug output. Because debug= sets only console loglevel to CONSOLE_LOGLEVEL_DEBUG (debug_kernel() in main.c). So lemme do some experiments: $ make arch/x86/kernel/smpboot.i I'm looking at the first function in there - smpboot_setup_warm_reset_vector() - which has some pr_debug statements: --- static inline __attribute__((always_inline)) __attribute__((no_instrument_function)) void smpboot_setup_warm_reset_vector(unsigned long start_eip) { unsigned long flags; do { do { ({ unsigned long __dummy; typeof(flags) __dummy2; (void)(&__dummy == &__dummy2); 1; }); flags = _raw_spin_lock_irqsave(spinlock_check(&rtc_lock)); } while (0); } while (0); rtc_cmos_write(0xa, 0xf); spin_unlock_irqrestore(&rtc_lock, flags); __native_flush_tlb(); no_printk("\001" "7" "smpboot" ": " "1.\n"); ^^^^^^^^ --- Yeah, so that's not issuing anything. Now, if I do #define DEBUG BUT! I have to remember to put it *before* all include statements in that file so that printk.h can see it defined, then I see the printk() call. --- static inline __attribute__((always_inline)) __attribute__((no_instrument_function)) void smpboot_setup_warm_reset_vector(unsigned long start_eip) { unsigned long flags; do { do { ({ unsigned long __dummy; typeof(flags) __dummy2; (void)(&__dummy == &__dummy2); 1; }); flags = _raw_spin_lock_irqsave(spinlock_check(&rtc_lock)); } while (0); } while (0); rtc_cmos_write(0xa, 0xf); spin_unlock_irqrestore(&rtc_lock, flags); __native_flush_tlb(); printk("\001" "7" "smpboot" ": " "1.\n"); ^^^^^^ --- So great, I have a printk there but I have to rebuild the kernel. I can then just as well add my own debug statements, even use trace_printk for that matter... So even a dyndbg=all won't work if you haven't defined CONFIG_DYNAMIC_DEBUG in your config. So pr_debug() gets optimized away in the normal case but if we want to always build in important debug statements and turn them on without building the kernel, the easiest thing to do is to switch to good ole printk(KERN_DEBUG Hell, we can even do an x86-specific define: x86_debug() or so, which would also denote that this is a debug statement which should stay built-in and it gives important information. All the early SMP bootstrap code for example would use that. And so on and so on... > Because this redirection breaks the whole pr_*() abstraction rather > fundamentally, dyndebug stealing pr_debug() and hiding debug messages > when the user specifically asked for them via 'debug' is pretty nasty > IMHO ... Yeah, I think we should do our own which doesn't interfere with pr_* -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-05-09 8:04 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-08 6:37 [PATCH 0/2 v2] speeding up cpu_up() Len Brown 2015-05-08 6:37 ` [PATCH 1/2] x86: replace cpu_up hard-coded mdelay with variable udelay Len Brown 2015-05-08 6:37 ` [PATCH 2/2] x86: speed cpu_up by quirking cpu_init_udelay Len Brown 2015-05-08 7:51 ` Ingo Molnar 2015-05-08 8:23 ` Borislav Petkov 2015-05-08 8:32 ` Borislav Petkov 2015-05-08 18:15 ` Len Brown 2015-05-08 18:27 ` Borislav Petkov 2015-05-09 7:22 ` Ingo Molnar 2015-05-09 7:24 ` Ingo Molnar 2015-05-09 8:04 ` Borislav Petkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).