From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5E328C433E0 for ; Tue, 23 Mar 2021 09:47:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2181B619CA for ; Tue, 23 Mar 2021 09:47:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230046AbhCWJqu (ORCPT ); Tue, 23 Mar 2021 05:46:50 -0400 Received: from mx2.suse.de ([195.135.220.15]:35828 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230047AbhCWJqj (ORCPT ); Tue, 23 Mar 2021 05:46:39 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1616492797; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=9bkP1bfooCaeb3dEPDdMPWKx/OWcMw/3cK8FHR3F0cA=; b=SNkwL7lnmQufoyUrenTcr4Cz0rKu3g4hAXNc5Gnu1OA1EsXxaWErkiVzQudObaZ6ipueWG C/F9pI2jDwwImja16SCp1CbctbaRzSxQGCk4ly/CQ8gOkjudE5+BX7cJKVTP5lr/fECou5 kt2LbudWEnyqFZYcRaSltZkvcdhKSmU= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 84607AC3E; Tue, 23 Mar 2021 09:46:37 +0000 (UTC) Date: Tue, 23 Mar 2021 10:46:31 +0100 From: Petr Mladek To: John Ogness Cc: Sergey Senozhatsky , Steven Rostedt , Thomas Gleixner , linux-kernel@vger.kernel.org, Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , Eric Biederman , Nicholas Piggin , Christophe Leroy , Alistair Popple , Jordan Niethe , Peter Zijlstra , =?iso-8859-1?Q?C=E9dric?= Le Goater , Andrew Morton , Kees Cook , Yue Hu , Alexey Kardashevskiy , Rafael Aquini , Tiezhu Yang , "Guilherme G. Piccoli" , "Paul E. McKenney" , linuxppc-dev@lists.ozlabs.org, kexec@lists.infradead.org Subject: Re: [PATCH next v1 2/3] printk: remove safe buffers Message-ID: References: <20210316233326.10778-1-john.ogness@linutronix.de> <20210316233326.10778-3-john.ogness@linutronix.de> <87k0pzmoao.fsf@jogness.linutronix.de> <87ft0mg8a0.fsf@jogness.linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ft0mg8a0.fsf@jogness.linutronix.de> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 2021-03-22 22:58:47, John Ogness wrote: > On 2021-03-22, Petr Mladek wrote: > > On Mon 2021-03-22 12:16:15, John Ogness wrote: > >> On 2021-03-21, Sergey Senozhatsky wrote: > >> >> @@ -369,7 +70,10 @@ __printf(1, 0) int vprintk_func(const char *fmt, va_list args) > >> >> * Use the main logbuf even in NMI. But avoid calling console > >> >> * drivers that might have their own locks. > >> >> */ > >> >> - if ((this_cpu_read(printk_context) & PRINTK_NMI_DIRECT_CONTEXT_MASK)) { > >> >> + if (this_cpu_read(printk_context) & > >> >> + (PRINTK_NMI_DIRECT_CONTEXT_MASK | > >> >> + PRINTK_NMI_CONTEXT_MASK | > >> >> + PRINTK_SAFE_CONTEXT_MASK)) { > >> > > >> But I suppose I could switch > >> the 1 printk_nmi_direct_enter() user to printk_nmi_enter() so that > >> PRINTK_NMI_DIRECT_CONTEXT_MASK can be removed now. I would do this in a > >> 4th patch of the series. > > > > Yes, please unify the PRINTK_NMI_CONTEXT. One is enough. > > Agreed. (But I'll go even further. See below.) > > > I wonder if it would make sense to go even further at this stage. > > What is possible? > > > > 1. We could get rid of printk_nmi_enter()/exit() and > > PRINTK_NMI_CONTEXT completely already now. It is enough > > to check in_nmi() in printk_func(). > > > > Agreed. in_nmi() within vprintk_emit() is enough to detect if the > console code should be skipped: > > if (!in_sched && !in_nmi()) { > ... > } Well, we also need to make sure that the irq work is scheduled to call console later. We should keep this dicision in printk_func(). I mean to replace the current if (this_cpu_read(printk_context) & (PRINTK_NMI_DIRECT_CONTEXT_MASK | PRINTK_NMI_CONTEXT_MASK | PRINTK_SAFE_CONTEXT_MASK)) { with /* * Avoid calling console drivers in recursive printk() * and in NMI context. */ if (this_cpu_read(printk_context) || in_nmi() { That said, I am not sure how this fits your further rework. I do not want to complicate it too much. I am just afraid that the discussion about console rework might take some time. And this would remove some complexity before we started the more complicated or controversial changes. > > 2. I thought about unifying printk_safe_enter()/exit() and > > printk_enter()/exit(). They both count recursion with > > IRQs disabled, have similar name. But they are used > > different way. > > > > But better might be to rename printk_safe_enter()/exit() to > > console_enter()/exit() or to printk_deferred_enter()/exit(). > > It would make more clear what it does now. And it might help > > to better distinguish it from the new printk_enter()/exit(). > > > > I am not sure if it is worth it. > > I am also not sure if it is worth the extra "noise" just to give the > function a more appropriate name. The plan is to remove it completely > soon anyway. My vote is to leave the name as it is. OK, let's keep printk_safe() name. It was just an idea. I wrote it primary to sort my thoughts. Best Regards, Petr