From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757266AbcK2Nyp (ORCPT ); Tue, 29 Nov 2016 08:54:45 -0500 Received: from mx2.suse.de ([195.135.220.15]:52920 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754945AbcK2Nyh (ORCPT ); Tue, 29 Nov 2016 08:54:37 -0500 Date: Tue, 29 Nov 2016 14:54:33 +0100 From: Petr Mladek To: Peter Zijlstra Cc: Sergey Senozhatsky , Andrew Morton , Jan Kara , Tejun Heo , Calvin Owens , Thomas Gleixner , Mel Gorman , Steven Rostedt , Ingo Molnar , linux-kernel@vger.kernel.org, Jason Wessel Subject: Re: [PATCH 1/3] printk: Fix kdb_trap_printk placement Message-ID: <20161129135433.GB21230@pathway.suse.cz> References: <20161018170830.405990950@infradead.org> <20161018171513.585962054@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161018171513.585962054@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 Tue 2016-10-18 19:08:31, Peter Zijlstra wrote: > Some people figured vprintk_emit() makes for a nice API and exported > it, bypassing the kdb trap. I think that nobody saw this problem because kdb_trap_printk was used only for a limited number of printk's. It is just a trick how to use generic functions to print messages in the kdb context, e.g. for getting backtraces. But the patch makes sense. > This still leaves vprintk_nmi() outside of the kbd reach, should that > be fixed too? It is perfectly fine. Messages from NMI context are not meant for kdb anyway. > Cc: Jason Wessel > Signed-off-by: Peter Zijlstra (Intel) > --- > kernel/printk/printk.c | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) > > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1750,6 +1750,13 @@ asmlinkage int vprintk_emit(int facility > /* cpu currently holding logbuf_lock in this function */ > static unsigned int logbuf_cpu = UINT_MAX; > > +#ifdef CONFIG_KGDB_KDB > + if (unlikely(kdb_trap_printk)) { > + r = vkdb_printf(KDB_MSGSRC_PRINTK, fmt, args); > + return r; r is not defined. It is fixed in the next patch but it breaks bisectability. Please, find below an updated patch that also includes my Reviewed-by. >>From 5adf18de45ba986ea3ae611446828238f4d65fe0 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Tue, 18 Oct 2016 19:08:31 +0200 Subject: [PATCH 1/3] printk: Fix kdb_trap_printk placement 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) Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek --- kernel/printk/printk.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index f7a55e9ff2f7..541ce7705353 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -1781,6 +1781,11 @@ asmlinkage int vprintk_emit(int facility, int level, /* cpu currently holding logbuf_lock in this function */ static unsigned int logbuf_cpu = UINT_MAX; +#ifdef CONFIG_KGDB_KDB + if (unlikely(kdb_trap_printk)) + return vkdb_printf(KDB_MSGSRC_PRINTK, fmt, args); +#endif + if (level == LOGLEVEL_SCHED) { level = LOGLEVEL_DEFAULT; in_sched = true; @@ -1923,17 +1928,7 @@ asmlinkage int printk_emit(int facility, int level, int vprintk_default(const char *fmt, va_list args) { - int r; - -#ifdef CONFIG_KGDB_KDB - if (unlikely(kdb_trap_printk)) { - r = vkdb_printf(KDB_MSGSRC_PRINTK, fmt, args); - return r; - } -#endif - r = vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, 0, fmt, args); - - return r; + return vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, 0, fmt, args); } EXPORT_SYMBOL_GPL(vprintk_default); -- 1.8.5.6