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: 9691675 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 95AEC601A1 for ; Fri, 21 Apr 2017 01:57:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 86A422236A for ; Fri, 21 Apr 2017 01:57:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7AAE828478; Fri, 21 Apr 2017 01:57:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 29BD12236A for ; Fri, 21 Apr 2017 01:57:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1034489AbdDUB5e (ORCPT ); Thu, 20 Apr 2017 21:57:34 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:34420 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1034486AbdDUB53 (ORCPT ); Thu, 20 Apr 2017 21:57:29 -0400 Received: by mail-io0-f194.google.com with SMTP id h41so23878763ioi.1; Thu, 20 Apr 2017 18:57:29 -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=loh0DUJxkchVc/OGVnK/lIHWyd4j7HMJEdy3LO2Iu8BOQMG9AMRqvG2PoeF3msNmMZ Lg8ZVwD9Y9g0Dw5GTnUVJeRHh3mcHZp6xHhIo2GT56c91xxvhs/kbNxk5L5fKd17wKwB q8FHoVsT5uEt5IOFWCyGrSpRtYKMMlyj09PPSY2dRmMS5vlIV5pA1ClH9mCYhIKY+OZU LErAlgl2FKH1zQHtJboKSY3U4bPzxiAZ7SjIJdCni1S+22CW4JH3aV76R5Dd7Mnt69x9 EogcDJUCOoUGw0b0QchsEcczj5tXknaqJl8hJkP1nN15GTnIfKM2SSzh6P91BQXfxl5I f05g== X-Gm-Message-State: AN3rC/6N9MpUZlS6UfbHvPK3gGaGojjL0yWt2TEWPbpxJ9+1HeIgBXOS Cce7hwW4uoYHAQ== 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 Cc: Sergey Senozhatsky , Steven Rostedt , Andrew Morton , Peter Zijlstra , Russell King , Daniel Thompson , Jiri Kosina , Ingo Molnar , Thomas Gleixner , Chris Metcalf , 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, Jan Kara , Ralf Baechle , Benjamin Herrenschmidt , Martin Schwidefsky , David Miller 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) Sender: linux-sh-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sh@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 // --- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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