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: 9691677 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 AB3A1601A1 for ; Fri, 21 Apr 2017 01:57:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9B84C28478 for ; Fri, 21 Apr 2017 01:57:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8E9012848D; Fri, 21 Apr 2017 01:57:55 +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=-1.9 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3F52E28478 for ; Fri, 21 Apr 2017 01:57:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=NczsowMI3Yndt3nBeI9fKsg85H3nwRMFNic7aryNtKs=; b=Z2XrUTPjZOKFUS ZM3sGA32fLfNewZXfSunfPHWocPMp2tw8TrJorXY4VE4az3zn8EZH7GBIbtOh4tduRI1A72i7Iqga UZfUbiEztmN7rPMHWf/zX+Lvt0xqvMpZw1grS019Byt3YK/+hpBraB29BvZ2yVUOGdMWXBOCo++5W IUpUaqJwNBzHxE2/x/92nqGcqMMtTAvGSOlbAtD3MEKgtrEXXukRGVEczMn1uIKQzIl+vqA0hths+ UtvRjP5TxVd66dR4TxOvSv6KxV0jWfdj1iwOqnwpbjKbIwJ3Ex7uivkUNOAAJrVIetzLN5JNOzXlo tSlqKtLGkAVmFzCCZLwA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1d1NpY-0003OI-Ts; Fri, 21 Apr 2017 01:57:52 +0000 Received: from mail-io0-x244.google.com ([2607:f8b0:4001:c06::244]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1d1NpV-0003ML-NA for linux-arm-kernel@lists.infradead.org; Fri, 21 Apr 2017 01:57:51 +0000 Received: by mail-io0-x244.google.com with SMTP id k87so23703720ioi.0 for ; 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=WBcnuEC2Cw+jiIZuohgsRgFm+ctteop6+nXSnXtCxmzSMH3N+cPiWowDd1l/Bvk2hJ wTQ6UhUs/zlzafL+eGwsHFqrD+EtHoBwVX1JZMtaMgxmy3DDrKxFFekwX2DRwIS0C/e/ 2KMoZ8XLqDMhEBte81defke8vR1/DLBm+nWWvQmTj4C0UBfH6cjBbuOfUsx4aQtTGuJY SCyWQdTB743embu46z5VEEbl2diOzu59xyAVU0ocGzS8jU3fznWU2msYB8kcB3bbDp+c i+yFeMIAAabtbIQmAHSDTQQMNKqGaqnsxqUBMbilBVr5Au7KsZnvAmTUCFAWzaRwaVA6 cXyg== X-Gm-Message-State: AN3rC/5sXk1IVn7bStThcb5bKnELfMavCeZvNzCWkdJ5TQ75ZXigxxFU 8oniTfMOjyAULw== 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-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170420_185749_808884_7D2893F1 X-CRM114-Status: GOOD ( 23.75 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: 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 , Benjamin Herrenschmidt , 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 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.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 // 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