From patchwork Fri Apr 21 01:57:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergey Senozhatsky X-Patchwork-Id: 753073 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3w8JpG4zTfz9s2s for ; Fri, 21 Apr 2017 11:59:50 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="gqK7PxX5"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3w8JpG3x4kzDqN6 for ; Fri, 21 Apr 2017 11:59:50 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="gqK7PxX5"; dkim-atps=neutral X-Original-To: linuxppc-dev@lists.ozlabs.org Delivered-To: linuxppc-dev@lists.ozlabs.org Received: from mail-io0-x243.google.com (mail-io0-x243.google.com [IPv6:2607:f8b0:4001:c06::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3w8JlZ3lK5zDqM1 for ; Fri, 21 Apr 2017 11:57:30 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="gqK7PxX5"; dkim-atps=neutral Received: by mail-io0-x243.google.com with SMTP id x86so23848111ioe.3 for ; Thu, 20 Apr 2017 18:57:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=zXzGkQ468VLkD7+e4+98kA4OPf9sbUvaxTxkSEfpoGg=; b=gqK7PxX5puLBS3Qvvn3tDSgwXKAOpZjcOCArprqFnqPjv8JFd3gxKf+kKKjQi5jdxW eosT72sJugBNXmDnCrkkeeLzZFg3bMqCPxdT64jYxLFzF2VEd5E2mAUed7oqkxtIvBOQ dC5AlZtCVo6B2lvgVgi0zg5y3DwtnW+IoPb7Go6B/tJa+0VqZOIp+71KASbFFvTa/hz4 zWXi7Mh/cB7vMHd3VrfANHPFzBEedkZcM+SS2B1Yv6zzT+cyu1GOnTknx80SGFoAkkUz dR4XTJUvPvDBLd8lZpszdoeHkRwpTUeypooE6W8couzjBBFFxMGCGks9WCCa3Ojfx0lB fL0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=zXzGkQ468VLkD7+e4+98kA4OPf9sbUvaxTxkSEfpoGg=; b=t0+XePuOgnf2vfq6hZ7q47KJQ7wQluiHM0QkEZhMITE45nysiHb9WYT1WfHR+vrUj7 bdHq5GQTtffQGRqF0xph8vP0u7B/JwaPvp0LGEegkKb3l2/ljfwYEch+R2szLWtbd9Dj I99mcc/gXy37F6DFFEwUTalVEvspXgk2cR7d2ueueJAOn5kp1/hYWKdU9fcxmulm5TWT wOZsFXvYFIJHoOs3IQKiH4LlDCjK0gA0s0O3wjE8t1HKLEOHMSPiVYvOCzSUGhe6DiUU kh7pXD/h7rgjROTiDXWZNRs+RVWtdm5TMKd0A5siE3XVm2f/+0bvphEaosXMN7zxsVOa vkxA== X-Gm-Message-State: AN3rC/6zM+NIPdZxpFMAWKg0FTSZrqFhdtKxz20xUatRZixS/jPazSfn CI21TNB1UGzH6g== X-Received: by 10.98.201.209 with SMTP id l78mr10249894pfk.13.1492739848681; Thu, 20 Apr 2017 18:57:28 -0700 (PDT) Received: from localhost ([175.223.39.25]) by smtp.gmail.com with ESMTPSA id e7sm6053528pgc.17.2017.04.20.18.57.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 20 Apr 2017 18:57:27 -0700 (PDT) Date: Fri, 21 Apr 2017 10:57:25 +0900 From: Sergey Senozhatsky To: Petr Mladek Subject: Re: [PATCH v5 1/4] printk/nmi: generic solution for safe printk in NMI Message-ID: <20170421015724.GA586@jagdpanzerIV.localdomain> References: <1461239325-22779-1-git-send-email-pmladek@suse.com> <1461239325-22779-2-git-send-email-pmladek@suse.com> <20170419131341.76bc7634@gandalf.local.home> <20170420033112.GB542@jagdpanzerIV.localdomain> <20170420131154.GL3452@pathway.suse.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170420131154.GL3452@pathway.suse.cz> User-Agent: Mutt/1.8.2 (2017-04-18) X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-mips@linux-mips.org, Jan Kara , Sergey Senozhatsky , linux-sh@vger.kernel.org, Peter Zijlstra , Chris Metcalf , sparclinux@vger.kernel.org, linux-s390@vger.kernel.org, Daniel Thompson , x86@kernel.org, Ingo Molnar , Russell King , adi-buildroot-devel@lists.sourceforge.net, Steven Rostedt , Thomas Gleixner , linux-arm-kernel@lists.infradead.org, Jiri Kosina , linux-cris-kernel@axis.com, linux-kernel@vger.kernel.org, Ralf Baechle , Martin Schwidefsky , Andrew Morton , linuxppc-dev@lists.ozlabs.org, David Miller Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" Hello, On (04/20/17 15:11), Petr Mladek wrote: [..] > Good analyze. I would summarize it that we need to be careful of: > > + logbug_lock > + PRINTK_SAFE_CONTEXT > + locks used by console drivers > > The first two things are easy to check. Except that a check for logbuf_lock > might produce false negatives. The last check is very hard. > > > so at the moment what I can think of is something like > > > > -- check this_cpu_read(printk_context) in NMI prink > > > > -- if we are NOT in printk_safe on this CPU, then do printk_deferred() > > and bypass `nmi_print_seq' buffer > > I would add also a check for logbuf_lock. > > -- if we are in printk_safe > > -- well... bad luck... have a bigger buffer. > > Yup, we do the best effort while still trying to stay on the safe > side. > > I have cooked up a patch based on this. It uses printk_deferred() > in NMI when it is safe. Note that console_flush_on_panic() will > try to get them on the console when a kdump is not generated. > I believe that it will help Steven. OK. I need to look more at the patch. It does more than I'd expected/imagined. [..] > void printk_nmi_enter(void) > { > - this_cpu_or(printk_context, PRINTK_NMI_CONTEXT_MASK); > + /* > + * The size of the extra per-CPU buffer is limited. Use it > + * only when really needed. > + */ > + if (this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK || > + raw_spin_is_locked(&logbuf_lock)) { > + this_cpu_or(printk_context, PRINTK_NMI_CONTEXT_MASK); > + } else { > + this_cpu_or(printk_context, PRINTK_NMI_DEFERRED_CONTEXT_MASK); > + } > } well... the logbuf_lock can temporarily be locked from another CPU. I'd say that spin_is_locked() has better chances for false positive than this_cpu_read(printk_context). because this_cpu_read(printk_context) depends only on this CPU state, while spin_is_locked() depends on all CPUs. and the idea with this_cpu_read(printk_context) was that we check if the logbuf_lock was locked from this particular CPU. I agree that this_cpu_read(printk_context) covers slightly more than logbuf_lock scope, so we may get positive this_cpu_read(printk_context) with unlocked logbuf_lock, but I don't tend to think that it's a big problem. wouldn't something as simple as below do the trick? // absolutely and completely untested // diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c index 033e50a7d706..c7477654c5b1 100644 --- a/kernel/printk/printk_safe.c +++ b/kernel/printk/printk_safe.c @@ -303,7 +303,10 @@ static int vprintk_nmi(const char *fmt, va_list args) { struct printk_safe_seq_buf *s = this_cpu_ptr(&nmi_print_seq); - return printk_safe_log_store(s, fmt, args); + if (this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK) + return printk_safe_log_store(s, fmt, args); + + return vprintk_emit(0, LOGLEVEL_SCHED, NULL, 0, fmt, args); } -ss