From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754857AbbIRWPY (ORCPT ); Fri, 18 Sep 2015 18:15:24 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:44430 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752962AbbIRWPX (ORCPT ); Fri, 18 Sep 2015 18:15:23 -0400 Date: Fri, 18 Sep 2015 15:15:21 -0700 From: Andrew Morton To: Jan Kara Cc: LKML , pmladek@suse.com, rostedt@goodmis.org, Gavin Hu , KY Srinivasan , Jan Kara Subject: Re: [PATCH 3/4] kernel: Avoid softlockups in stop_machine() during heavy printing Message-Id: <20150918151521.c5c8caa59a0e254fdd713337@linux-foundation.org> In-Reply-To: <1439998711-7013-4-git-send-email-jack@suse.com> References: <1439998711-7013-1-git-send-email-jack@suse.com> <1439998711-7013-4-git-send-email-jack@suse.com> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 19 Aug 2015 17:38:30 +0200 Jan Kara wrote: > From: Jan Kara > > When there are lots of messages accumulated in printk buffer, printing > them (especially over serial console) can take a long time (tens of > seconds). stop_machine() will effectively make all cpus spin in > multi_cpu_stop() waiting for the CPU doing printing to print all the > messages which triggers NMI softlockup watchdog and RCU stall detector > which add even more to the messages to print. Since machine doesn't do > anything (except serving interrupts) during this time, also network > connections are dropped and other disturbances may happen. > > Paper over the problem by waiting for printk buffer to be empty before > starting to stop CPUs. In theory a burst of new messages can be appended > to the printk buffer before CPUs enter multi_cpu_stop() so this isn't a 100% > solution but it works OK in practice and I'm not aware of a reasonably > simple better solution. Confused. Why don't patches 1 and 2 already fix this problem? > > ... > > @@ -2489,6 +2489,28 @@ struct tty_driver *console_device(int *index) > } > > /* > + * Wait until all messages accumulated in the printk buffer are printed to > + * console. Note that as soon as this function returns, new messages may be > + * added to the printk buffer by other CPUs. > + */ > +void console_flush(void) This doesn't seem a very good name. We already have console_cont_flush(), cont_flush(), etc. Can we think of something more specific? printk_log_buf_drain() perhaps. > +{ > + bool retry; > + unsigned long flags; > + > + while (1) { > + raw_spin_lock_irqsave(&logbuf_lock, flags); > + retry = console_seq != log_next_seq; > + raw_spin_unlock_irqrestore(&logbuf_lock, flags); Does this lock/unlock do anything useful? > + if (!retry || console_suspended) > + break; > + /* Cycle console_sem to wait for outstanding printing */ > + console_lock(); > + console_unlock(); > + } > +} > + > +/* > * Prevent further output on the passed console device so that (for example) > * serial drivers can disable console output before suspending a port, and can > * re-enable output afterwards. > diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c > index fd643d8c4b42..016d34621d2e 100644 > --- a/kernel/stop_machine.c > +++ b/kernel/stop_machine.c > @@ -21,6 +21,7 @@ > #include > #include > #include > +#include > > /* > * Structure to determine completion condition and record errors. May > @@ -543,6 +544,14 @@ int __stop_machine(int (*fn)(void *), void *data, const struct cpumask *cpus) > return ret; > } > > + /* > + * If there are lots of outstanding messages, printing them can take a > + * long time and all cpus would be spinning waiting for the printing to > + * finish thus triggering NMI watchdog, RCU lockups etc. Wait for the > + * printing here to avoid these. > + */ > + console_flush(); This is pretty pointless if num_possible_cpus==1. I'd suggest setting printk_offload_chars=0 in this case, add some early bale-out into console_flush(). Or something along those lines. And make console_flush() go away altogether if CONFIG_SMP=n - it's pointless bloat. > /* Set the initial state and stop all online cpus. */ > set_state(&msdata, MULTI_STOP_PREPARE); > return stop_cpus(cpu_online_mask, multi_cpu_stop, &msdata);