From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754725AbcB2LSY (ORCPT ); Mon, 29 Feb 2016 06:18:24 -0500 Received: from mail-pa0-f52.google.com ([209.85.220.52]:35419 "EHLO mail-pa0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753521AbcB2LSU (ORCPT ); Mon, 29 Feb 2016 06:18:20 -0500 Date: Mon, 29 Feb 2016 20:19:37 +0900 From: Sergey Senozhatsky To: Petr Mladek Cc: Sergey Senozhatsky , Sergey Senozhatsky , Andrew Morton , Jan Kara , Peter Zijlstra , Steven Rostedt , Ingo Molnar , linux-kernel@vger.kernel.org Subject: Re: [PATCH] printk/nmi: restore printk_func in nmi_panic Message-ID: <20160229111937.GA356@swordfish> References: <1456457840-1059-1-git-send-email-sergey.senozhatsky@gmail.com> <20160226145717.GZ12548@pathway.suse.cz> <20160227021944.GA1621@swordfish> <20160229103141.GL3305@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160229103141.GL3305@pathway.suse.cz> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (02/29/16 11:31), Petr Mladek wrote: > > > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > > > > index f4fa2b2..3ee33d5 100644 > > > > --- a/include/linux/kernel.h > > > > +++ b/include/linux/kernel.h > > > > @@ -469,10 +469,12 @@ do { \ > > > > cpu = raw_smp_processor_id(); \ > > > > old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, cpu); \ > > > > \ > > > > - if (old_cpu == PANIC_CPU_INVALID) \ > > > > + if (old_cpu == PANIC_CPU_INVALID) { \ > > > > + printk_nmi_exit(); \ > > > > > > This might end up in a deadlock that printk_nmi() wanted to avoid. > > > > aha, I see. > > > > > I think about a compromise. We should try to get the messages > > > out only when kdump is not enabled. > > > > can we zap_locks() if we are on nmi_panic()->panic()->console_flush_on_panic() path? > > That is the problem. zap_locks() is not a solution. > > First, it handles only lockbuf_lock and console_sem. There are other > locks used by particular consoles that might cause a deadlock. yes, well, that's true for panic() in general. we can't take care of all of the locks that possibly could have been in busy state at the time CPU received STOP_IPI or entered panic(). we can't even safely iterate all of the consoles and call ->reset() on them (provided that such struct console callback will ever be implemented) because smp_send_stop() is free to leave some CPUs online. > Second, re-initializing locks is dangerous of its own. If they are > released by some other CPU that is still running, you might end up > in a deadlock because of a double release. In fact, I think that it > actually increases the risk. If there are more than 2 CPUs than > it is more likely that a printk is running on another CPU than > on the current one. panic calls debug_locks_off(), so chances *seem* to be lower. hm... we are (sort of) on the safe side if we know that smp_send_stop() has stopped all the CPUs but panic cpu; so zap_locks() is safe (to some extent of course) when we know that num_online_cpus() == 1 in console_flush_on_panic(). -ss