From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751364AbdJENit (ORCPT ); Thu, 5 Oct 2017 09:38:49 -0400 Received: from mx2.suse.de ([195.135.220.15]:34172 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751280AbdJENis (ORCPT ); Thu, 5 Oct 2017 09:38:48 -0400 Date: Thu, 5 Oct 2017 15:38:44 +0200 From: Petr Mladek To: Peter Zijlstra Cc: sergey.senozhatsky@gmail.com, linux-kernel@vger.kernel.org, rostedt@goodmis.org, mingo@kernel.org, tglx@linutronix.de, Jason Wessel Subject: Re: [PATCH 1/3] printk: Fix kdb_trap_printk placement Message-ID: <20171005133844.GA16068@pathway.suse.cz> References: <20170928121823.430053219@infradead.org> <20170928122513.328354788@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170928122513.328354788@infradead.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu 2017-09-28 14:18:24, Peter Zijlstra wrote: > Some people figured vprintk_emit() makes for a nice API and exported > it, bypassing the kdb trap. > > This still leaves vprintk_nmi() outside of the kbd reach, should that > be fixed too? > > Cc: Jason Wessel > Signed-off-by: Peter Zijlstra (Intel) > --- > kernel/printk/printk.c | 18 ++++++------------ > 1 file changed, 6 insertions(+), 12 deletions(-) > > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1811,6 +1811,11 @@ asmlinkage int vprintk_emit(int facility > int printed_len; > bool in_sched = false; > > +#ifdef CONFIG_KGDB_KDB > + if (unlikely(kdb_trap_printk && kdb_printf_cpu < 0)) > + return vkdb_printf(KDB_MSGSRC_PRINTK, fmt, args); > +#endif Hmm, this will get called also from scheduler and timer code via printk_deferred(). I am afraid that it is not safe. If I get it correctly, vkdb_printf() might call printk() when kdb_printf_cpu are set to a real CPU number. Then we will fall through and try to call consoles. Fortunately, I think that we do not need this patch at all. vkdb_printf() is called here only when kdb_trap_printk is set. It is used the following way: static void kdb_dumpregs(struct pt_regs *regs) { [...] kdb_trap_printk++; show_regs(regs); kdb_trap_printk--; [...] } or static int kdb_ftdump(int argc, const char **argv) { [...] kdb_trap_printk++; ftrace_dump_buf(skip_lines, cpu_file); kdb_trap_printk--; [...] } It looks like a nasty hack to reuse an existing code that calls printk(). The aim is to get the output of these printk's on the kdb console instead of the log buffer and other consoles. Note that kdb_dumpregs(), kdb_ftdump() implement kdb commands that might be called from the kdb console. If these commands are always called from normal context then we do not need to care of NMI and other special printk variants. Or can the kdb console commands be called in NMI context? Best Regards, Petr