From mboxrd@z Thu Jan 1 00:00:00 1970 From: Petr Mladek Date: Fri, 04 Dec 2015 15:29:24 +0000 Subject: Re: [PATCH v2 1/5] printk/nmi: Generic solution for safe printk in NMI Message-Id: <20151204152924.GB20935@pathway.suse.cz> List-Id: References: <1448622572-16900-1-git-send-email-pmladek@suse.com> <1448622572-16900-2-git-send-email-pmladek@suse.com> <1449024316.11810.6.camel@ellerman.id.au> In-Reply-To: <1449024316.11810.6.camel@ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-arm-kernel@lists.infradead.org On Wed 2015-12-02 13:45:16, Michael Ellerman wrote: > On Fri, 2015-11-27 at 12:09 +0100, Petr Mladek wrote: > > > printk() takes some locks and could not be used a safe way in NMI > > context. > > > > The chance of a deadlock is real especially when printing > > stacks from all CPUs. This particular problem has been addressed > > on x86 by the commit a9edc8809328 ("x86/nmi: Perform a safe NMI stack > > trace on all CPUs"). > > ... > > > diff --git a/kernel/printk/nmi.c b/kernel/printk/nmi.c > > new file mode 100644 > > index 000000000000..3989e13a0021 > > --- /dev/null > > +++ b/kernel/printk/nmi.c > > @@ -0,0 +1,200 @@ > > ... > > > + > > +struct nmi_seq_buf { > > + atomic_t len; /* length of written data */ > > + struct irq_work work; /* IRQ work that flushes the buffer */ > > + unsigned char buffer[PAGE_SIZE - sizeof(atomic_t) - > > + sizeof(struct irq_work)]; > > +}; > > +static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq); > > > PAGE_SIZE isn't always 4K. > > On typical powerpc systems this will give you 128K, and on some 512K, which is > probably not what we wanted. Good point! > The existing code just did: > > #define NMI_BUF_SIZE 4096 I will change this to 8192. The 4kB were not enough in some cases. Best Regards, Petr From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753539AbbLDP3b (ORCPT ); Fri, 4 Dec 2015 10:29:31 -0500 Received: from mx2.suse.de ([195.135.220.15]:54291 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753641AbbLDP31 (ORCPT ); Fri, 4 Dec 2015 10:29:27 -0500 Date: Fri, 4 Dec 2015 16:29:24 +0100 From: Petr Mladek To: Michael Ellerman Cc: Andrew Morton , linux-mips@linux-mips.org, Daniel Thompson , Jiri Kosina , linux-cris-kernel@axis.com, linux-s390@vger.kernel.org, Peter Zijlstra , x86@kernel.org, linux-kernel@vger.kernel.org, Steven Rostedt , adi-buildroot-devel@lists.sourceforge.net, Ingo Molnar , linux-sh@vger.kernel.org, sparclinux@vger.kernel.org, Russell King , Thomas Gleixner , linuxppc-dev@lists.ozlabs.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 1/5] printk/nmi: Generic solution for safe printk in NMI Message-ID: <20151204152924.GB20935@pathway.suse.cz> References: <1448622572-16900-1-git-send-email-pmladek@suse.com> <1448622572-16900-2-git-send-email-pmladek@suse.com> <1449024316.11810.6.camel@ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1449024316.11810.6.camel@ellerman.id.au> 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 Wed 2015-12-02 13:45:16, Michael Ellerman wrote: > On Fri, 2015-11-27 at 12:09 +0100, Petr Mladek wrote: > > > printk() takes some locks and could not be used a safe way in NMI > > context. > > > > The chance of a deadlock is real especially when printing > > stacks from all CPUs. This particular problem has been addressed > > on x86 by the commit a9edc8809328 ("x86/nmi: Perform a safe NMI stack > > trace on all CPUs"). > > ... > > > diff --git a/kernel/printk/nmi.c b/kernel/printk/nmi.c > > new file mode 100644 > > index 000000000000..3989e13a0021 > > --- /dev/null > > +++ b/kernel/printk/nmi.c > > @@ -0,0 +1,200 @@ > > ... > > > + > > +struct nmi_seq_buf { > > + atomic_t len; /* length of written data */ > > + struct irq_work work; /* IRQ work that flushes the buffer */ > > + unsigned char buffer[PAGE_SIZE - sizeof(atomic_t) - > > + sizeof(struct irq_work)]; > > +}; > > +static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq); > > > PAGE_SIZE isn't always 4K. > > On typical powerpc systems this will give you 128K, and on some 512K, which is > probably not what we wanted. Good point! > The existing code just did: > > #define NMI_BUF_SIZE 4096 I will change this to 8192. The 4kB were not enough in some cases. Best Regards, Petr From mboxrd@z Thu Jan 1 00:00:00 1970 From: pmladek@suse.com (Petr Mladek) Date: Fri, 4 Dec 2015 16:29:24 +0100 Subject: [PATCH v2 1/5] printk/nmi: Generic solution for safe printk in NMI In-Reply-To: <1449024316.11810.6.camel@ellerman.id.au> References: <1448622572-16900-1-git-send-email-pmladek@suse.com> <1448622572-16900-2-git-send-email-pmladek@suse.com> <1449024316.11810.6.camel@ellerman.id.au> Message-ID: <20151204152924.GB20935@pathway.suse.cz> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed 2015-12-02 13:45:16, Michael Ellerman wrote: > On Fri, 2015-11-27 at 12:09 +0100, Petr Mladek wrote: > > > printk() takes some locks and could not be used a safe way in NMI > > context. > > > > The chance of a deadlock is real especially when printing > > stacks from all CPUs. This particular problem has been addressed > > on x86 by the commit a9edc8809328 ("x86/nmi: Perform a safe NMI stack > > trace on all CPUs"). > > ... > > > diff --git a/kernel/printk/nmi.c b/kernel/printk/nmi.c > > new file mode 100644 > > index 000000000000..3989e13a0021 > > --- /dev/null > > +++ b/kernel/printk/nmi.c > > @@ -0,0 +1,200 @@ > > ... > > > + > > +struct nmi_seq_buf { > > + atomic_t len; /* length of written data */ > > + struct irq_work work; /* IRQ work that flushes the buffer */ > > + unsigned char buffer[PAGE_SIZE - sizeof(atomic_t) - > > + sizeof(struct irq_work)]; > > +}; > > +static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq); > > > PAGE_SIZE isn't always 4K. > > On typical powerpc systems this will give you 128K, and on some 512K, which is > probably not what we wanted. Good point! > The existing code just did: > > #define NMI_BUF_SIZE 4096 I will change this to 8192. The 4kB were not enough in some cases. Best Regards, Petr