From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752942AbcJFNJA (ORCPT ); Thu, 6 Oct 2016 09:09:00 -0400 Received: from mx2.suse.de ([195.135.220.15]:40593 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750979AbcJFNIx (ORCPT ); Thu, 6 Oct 2016 09:08:53 -0400 Date: Thu, 6 Oct 2016 15:08:49 +0200 From: Petr Mladek To: Sergey Senozhatsky Cc: Jan Kara , Andrew Morton , Tejun Heo , Calvin Owens , Thomas Gleixner , Mel Gorman , Steven Rostedt , linux-kernel@vger.kernel.org, Sergey Senozhatsky Subject: Re: [RFC][PATCHv2 3/7] printk: introduce per-cpu alt_print seq buffer Message-ID: <20161006130849.GG13369@pathway.suse.cz> References: <20160930151758.8965-1-sergey.senozhatsky@gmail.com> <20160930151758.8965-4-sergey.senozhatsky@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160930151758.8965-4-sergey.senozhatsky@gmail.com> 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 Sat 2016-10-01 00:17:54, Sergey Senozhatsky wrote: > This patch extends the idea of NMI per-cpu buffers to regions > that may cause recursive printk() calls and possible deadlocks. > diff --git a/kernel/printk/alt_printk.c b/kernel/printk/alt_printk.c > index 7178661..4bc1e7d 100644 > --- a/kernel/printk/alt_printk.c > +++ b/kernel/printk/alt_printk.c > len = atomic_read(&s->len); > > - if (len >= sizeof(s->buffer)) { > - atomic_inc(&nmi_message_lost); > + if (len >= sizeof(s->buffer)) > return 0; > - } > > /* > * Make sure that all old data have been read before the buffer was > @@ -240,6 +235,83 @@ void alt_printk_flush_on_panic(void) > alt_printk_flush(); > } > > +/* > + * Safe printk() for NMI context. It uses a per-CPU buffer to > + * store the message. NMIs are not nested, so there is always only > + * one writer running. But the buffer might get flushed from another > + * CPU, so we need to be careful. > + */ > +static int vprintk_nmi(const char *fmt, va_list args) > +{ > + struct alt_printk_seq_buf *s = this_cpu_ptr(&nmi_print_seq); > + int add; > + > + add = alt_printk_log_store(s, fmt, args); > + if (!add) > + atomic_inc(&nmi_message_lost); This would could also empty string as an error. A solution might be update alt_printk_log_store() to return -1 in case of lost log. Note that vprintk_nmi() still needs to return 0 in this case to stay compatible with printk(). > + > + return add; > +} > + > +void printk_nmi_enter(void) > +{ > + this_cpu_or(alt_printk_ctx, ALT_PRINTK_NMI_CONTEXT_MASK); > +} > + > +void printk_nmi_exit(void) > +{ > + this_cpu_and(alt_printk_ctx, ~ALT_PRINTK_NMI_CONTEXT_MASK); > +} > + > +/* > + * Lockless printk(), to avoid deadlocks should the printk() recurse > + * into itself. It uses a per-CPU buffer to store the message, just like > + * NMI. > + */ > +static int vprintk_alt(const char *fmt, va_list args) > +{ > + struct alt_printk_seq_buf *s = this_cpu_ptr(&alt_print_seq); > + > + return alt_printk_log_store(s, fmt, args); We should handle lost strings here as well. But it can be done in a followup patch. > +} > + > +/* > + * Returns with local IRQs disabled. > + * Can be preempted by NMI. > + */ > +void alt_printk_enter(void) > +{ > + unsigned long flags; > + int entry_count; > + > + local_irq_save(flags); > + if (!(this_cpu_read(alt_printk_ctx) & ALT_PRINTK_CONTEXT_MASK)) > + this_cpu_write(alt_printk_irq_flags, flags); > + this_cpu_inc(alt_printk_ctx); > +} > + > +/* > + * Restores local IRQs state saved in alt_printk_enter(). > + * Can be preempted by NMI. > + */ > +void alt_printk_exit(void) > +{ > + this_cpu_dec(alt_printk_ctx); > + if (!(this_cpu_read(alt_printk_ctx) & ALT_PRINTK_CONTEXT_MASK)) > + local_irq_restore(this_cpu_read(alt_printk_irq_flags)); > +} I will discuss this in your replay that explains the details. Anyway, it looks much easier now. Best Regards, Petr