All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Slaby <jslaby@suse.cz>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Ingo Molnar <mingo@redhat.com>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org
Subject: Re: [PATCH 1/2] x86: dumpstack, use pr_cont
Date: Tue, 6 Oct 2015 18:16:30 +0200	[thread overview]
Message-ID: <5613F3DE.4010406@suse.cz> (raw)
In-Reply-To: <874mi4m1gs.fsf@rasmusvillemoes.dk>

On 10/06/2015, 06:00 PM, Rasmus Villemoes wrote:
>> --- a/arch/x86/kernel/dumpstack.c
>> +++ b/arch/x86/kernel/dumpstack.c
>> @@ -260,18 +260,18 @@ int __die(const char *str, struct pt_regs *regs, long err)
>>  	printk(KERN_DEFAULT
>>  	       "%s: %04lx [#%d] ", str, err & 0xffff, ++die_counter);
>>  #ifdef CONFIG_PREEMPT
>> -	printk("PREEMPT ");
>> +	pr_cont("PREEMPT ");
>>  #endif
>>  #ifdef CONFIG_SMP
>> -	printk("SMP ");
>> +	pr_cont("SMP ");
>>  #endif
>>  #ifdef CONFIG_DEBUG_PAGEALLOC
>> -	printk("DEBUG_PAGEALLOC ");
>> +	pr_cont("DEBUG_PAGEALLOC");
> 
> cosmetic: this lost a space.

Oh, good catch. This was left from the times when DEBUG_PAGEALLOC was
last. Now KASAN is, but I added a space to both of them locally for the
time being. Anyway:

> May I suggest 
> 
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index 9c30acfadae2..b473a47d1851 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -257,21 +257,23 @@ int __die(const char *str, struct pt_regs *regs, long err)
>  	unsigned short ss;
>  	unsigned long sp;
>  #endif
> -	printk(KERN_DEFAULT
> -	       "%s: %04lx [#%d] ", str, err & 0xffff, ++die_counter);
> +	static const char build_flags[] = ""
>  #ifdef CONFIG_PREEMPT
> -	printk("PREEMPT ");
> +	" PREEMPT"
>  #endif
>  #ifdef CONFIG_SMP
> -	printk("SMP ");
> +	" SMP"
>  #endif
>  #ifdef CONFIG_DEBUG_PAGEALLOC
> -	printk("DEBUG_PAGEALLOC ");
> +	" DEBUG_PAGEALLOC"
>  #endif
>  #ifdef CONFIG_KASAN
> -	printk("KASAN");
> +	" KASAN"
>  #endif
> -	printk("\n");
> +		;
> +	printk(KERN_DEFAULT
> +	       "%s: %04lx [#%d]%s\n", str, err & 0xffff, ++die_counter,
> +		build_flags);
>  	if (notify_die(DIE_OOPS, str, regs, err,
>  			current->thread.trap_nr, SIGSEGV) == NOTIFY_STOP)
>  		return 1;
> 
> instead, so that there's only one printk call and the pr_cont issue goes
> away?

I like this. So if you resend as a proper patch, I will definitely ack
it. But it's up to the maintainers which one they prefer.

thanks,
-- 
js
suse labs

  reply	other threads:[~2015-10-06 16:16 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-06 12:29 [PATCH 1/2] x86: dumpstack, use pr_cont Jiri Slaby
2015-10-06 12:29 ` [PATCH 2/2] x86: text_poke, check if text_mutex is held Jiri Slaby
2015-10-06 16:00 ` [PATCH 1/2] x86: dumpstack, use pr_cont Rasmus Villemoes
2015-10-06 16:16   ` Jiri Slaby [this message]
2015-10-06 21:05     ` [PATCH 1/2] linux/kconfig.h: generalize IS_ENABLED logic Rasmus Villemoes
2015-10-06 21:05       ` [PATCH 2/2] x86: dumpstack: eliminate some #ifdefs Rasmus Villemoes
2015-10-07  7:03         ` Ingo Molnar
2016-03-26 20:40         ` [PATCH] x86: dumpstack: combine some printks Rasmus Villemoes
2016-04-01  6:37           ` [tip:x86/debug] x86/dumpstack: Combine some printk()s tip-bot for Rasmus Villemoes
2015-10-07  6:57       ` [PATCH 1/2] linux/kconfig.h: generalize IS_ENABLED logic Ingo Molnar
2015-10-07  8:15       ` Michal Marek
2015-10-07 21:33         ` Rasmus Villemoes
2015-10-08 11:40           ` Michal Marek
2015-11-16  8:00       ` using IS_ENABLED(CONFIG_xyz) effectively Vineet Gupta
2015-11-16  8:00         ` Vineet Gupta
2015-11-16  8:00         ` Vineet Gupta
2015-11-16  8:28         ` Geert Uytterhoeven
2015-11-16  8:28           ` Geert Uytterhoeven
2015-11-16  8:35           ` Vineet Gupta
2015-11-16  8:35             ` Vineet Gupta
2015-11-16  8:52             ` Arnd Bergmann
2015-11-16  8:52               ` Arnd Bergmann
2015-11-16  8:52               ` Arnd Bergmann
2015-11-16 10:03               ` Vineet Gupta
2015-11-16 10:03                 ` Vineet Gupta
  -- strict thread matches above, loose matches on Subject: below --
2014-01-27  9:28 [PATCH 1/2] x86: dumpstack, use pr_cont Jiri Slaby

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5613F3DE.4010406@suse.cz \
    --to=jslaby@suse.cz \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.