Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754330AbbFJPgp (ORCPT ); Wed, 10 Jun 2015 11:36:45 -0400 Received: from casper.infradead.org ([85.118.1.10]:60395 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754470AbbFJP3Y (ORCPT ); Wed, 10 Jun 2015 11:29:24 -0400 Date: Wed, 10 Jun 2015 17:29:17 +0200 From: Peter Zijlstra To: Petr Mladek Cc: Steven Rostedt , linux-kernel@vger.kernel.org, jkosina@suse.cz, paulmck@linux.vnet.ibm.com, Ingo Molnar , Thomas Gleixner Subject: Re: [RFC][PATCH] printk: Fixup the nmi printk mess Message-ID: <20150610152917.GI3644@twins.programming.kicks-ass.net> References: <20150610125509.GO19282@twins.programming.kicks-ass.net> <20150610143155.GD9409@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150610143155.GD9409@pathway.suse.cz> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2091 Lines: 68 On Wed, Jun 10, 2015 at 04:31:55PM +0200, Petr Mladek wrote: > > +static void __printk_nmi_flush(struct irq_work *work) > > +{ > > + struct nmi_seq_buf *s = container_of(work, struct nmi_seq_buf, work); > > + int len, last_i = 0, i = 0; > > + > > +again: > > + len = seq_buf_used(&s->seq); > > + if (!len) > > + return; > > + > > + /* Print line by line. */ > > + for (; i < len; i++) { > > + if (s->buffer[i] == '\n') { > > + print_seq_line(s, last_i, i); > > + last_i = i + 1; > > + } > > + } > > + /* Check if there was a partial line. */ > > + if (last_i < len) { > > + print_seq_line(s, last_i, len - 1); > > + pr_cont("\n"); > > + } > > + > > + /* > > + * Another NMI could have come in while we were printing > > + * check if nothing has been added to the buffer. > > + */ > > + if (cmpxchg_local(&s->seq.len, len, 0) != len) > > + goto again; > > If another NMI comes at this point, it will start filling the buffer > from the beginning. If it is fast enough, it might override the text > that we print above. How so? If the cmpxchg succeeded and len == 0, we flushed everything and are done with it, if another NMI comes in and 'overwrites' it, that's fine, right? > > +static int vprintk_nmi(const char *fmt, va_list args) > > +{ > > + struct nmi_seq_buf *s = this_cpu_ptr(&nmi_print_seq); > > + unsigned int len = seq_buf_used(&s->seq); > > + > > + irq_work_queue(&s->work); > > + seq_buf_vprintf(&s->seq, fmt, args); > > This is more critical. seq_buf_vprintf() has the following code: > > len = vsnprintf(s->buffer + s->len, s->size - s->len, fmt, args); > > The race might look like: > > CPU0 CPU1 > No, everything is strictly per cpu. > To be honest, I am not familiar with cmpxchg_local(). But I think that > it can't do much difference. The value has to be synced to the other > CPUs one day. Nope.. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/