From: Petr Mladek <pmladek@suse.com>
To: yalin wang <yalin.wang2010@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Peter Zijlstra <peterz@infradead.org>,
Steven Rostedt <rostedt@goodmis.org>,
Russell King <rmk+kernel@arm.linux.org.uk>,
Daniel Thompson <daniel.thompson@linaro.org>,
Jiri Kosina <jkosina@suse.com>, Ingo Molnar <mingo@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
linux-kernel <linux-kernel@vger.kernel.org>,
x86@kernel.org, linux-arm-kernel@lists.infradead.org,
adi-buildroot-devel@lists.sourceforge.net,
linux-cris-kernel@axis.com, linux-mips@linux-mips.org,
linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org,
linux-sh@vger.kernel.org, sparclinux@vger.kernel.org
Subject: Re: [PATCH v2 5/5] printk/nmi: Increase the size of the temporary buffer
Date: Mon, 7 Dec 2015 15:16:46 +0100 [thread overview]
Message-ID: <20151207141646.GF20935@pathway.suse.cz> (raw)
In-Reply-To: <81211733-2484-40A9-9D4A-644AA27FBC73@gmail.com>
On Mon 2015-11-30 08:42:04, yalin wang wrote:
>
> > On Nov 27, 2015, at 19:09, Petr Mladek <pmladek@suse.com> wrote:
> >
> > Testing has shown that the backtrace sometimes does not fit
> > into the 4kB temporary buffer that is used in NMI context.
> >
> > The warnings are gone when I double the temporary buffer size.
> >
> > Note that this problem existed even in the x86-specific
> > implementation that was added by the commit a9edc8809328
> > ("x86/nmi: Perform a safe NMI stack trace on all CPUs").
> > Nobody noticed it because it did not print any warnings.
> >
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
> > ---
> > kernel/printk/nmi.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/printk/nmi.c b/kernel/printk/nmi.c
> > index 8af1e4016719..6111644d5f01 100644
> > --- a/kernel/printk/nmi.c
> > +++ b/kernel/printk/nmi.c
> > @@ -42,7 +42,7 @@ atomic_t nmi_message_lost;
> > 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) -
> > + unsigned char buffer[2 * PAGE_SIZE - sizeof(atomic_t) -
> > sizeof(struct irq_work)];
> > };
> >
>
> why not define like this:
>
> union {
> struct {atomic_t len;
> struct irq_work work;
> }
> unsigned char buffer[PAGE_SIZE * 2] ;
> }
>
> we can make sure the union is 2 PAGE_SIZE .
IMHO, this would add more confusion. It would just move the
computation somewhere else. The union will have 2*PAGE_SIZE
but the beginning of "buffer" will be shared with "len"
and "work". Therefore, we would need to skip the beginning
of the buffer when storing the data. By other words, we still
will be able to use only (sizeof(buffer) - sizeof(atomic_t) -
sizeof(struct irq_work)] of the "buffer".
Or did I miss something, please?
Best Regards,
Petr
prev parent reply other threads:[~2015-12-07 14:16 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-27 11:09 [PATCH v2 0/5] Cleaning printk stuff in NMI context Petr Mladek
2015-11-27 11:09 ` [PATCH v2 1/5] printk/nmi: Generic solution for safe printk in NMI Petr Mladek
2015-11-27 11:49 ` kbuild test robot
2015-11-27 15:38 ` Petr Mladek
2015-12-01 23:24 ` Jiri Kosina
2015-12-04 16:57 ` Petr Mladek
2016-03-17 19:35 ` Andrew Morton
2016-03-18 10:03 ` Petr Mladek
2015-11-27 12:02 ` kbuild test robot
2015-11-27 14:26 ` Max Filippov
2015-11-30 14:25 ` Petr Mladek
2015-12-02 2:45 ` Michael Ellerman
2015-12-04 15:29 ` Petr Mladek
2015-11-27 11:09 ` [PATCH v2 2/5] printk/nmi: Use IRQ work only when ready Petr Mladek
2015-11-27 11:09 ` [PATCH v2 3/5] printk/nmi: Try hard to print Oops message in NMI context Petr Mladek
2015-12-01 23:44 ` Russell King - ARM Linux
2015-12-04 15:27 ` Petr Mladek
2015-12-04 17:12 ` Russell King - ARM Linux
2015-12-07 15:48 ` David Laight
2015-12-08 14:49 ` Petr Mladek
2015-12-08 11:21 ` Petr Mladek
2015-11-27 11:09 ` [PATCH v2 4/5] printk/nmi: Warn when some message has been lost " Petr Mladek
2015-11-27 11:09 ` [PATCH v2 5/5] printk/nmi: Increase the size of the temporary buffer Petr Mladek
2015-11-30 16:42 ` yalin wang
2015-12-02 16:20 ` David Laight
2015-12-04 15:47 ` Petr Mladek
2015-12-07 14:16 ` Petr Mladek [this message]
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=20151207141646.GF20935@pathway.suse.cz \
--to=pmladek@suse.com \
--cc=adi-buildroot-devel@lists.sourceforge.net \
--cc=akpm@linux-foundation.org \
--cc=daniel.thompson@linaro.org \
--cc=jkosina@suse.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-cris-kernel@axis.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@linux-mips.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rmk+kernel@arm.linux.org.uk \
--cc=rostedt@goodmis.org \
--cc=sparclinux@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
--cc=yalin.wang2010@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).