Message ID | 20221019145600.1282823-13-john.ogness@linutronix.de |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp377938wrs; Wed, 19 Oct 2022 08:08:07 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5/SKf4SHjvjouhOGxMJZ20sXP+sZlvW20DN5+hfv8ZqgaYaV4YHqpLqygwkWtfm3b/b+0k X-Received: by 2002:a17:907:2711:b0:78e:c2a:a3fb with SMTP id w17-20020a170907271100b0078e0c2aa3fbmr7094147ejk.556.1666192087544; Wed, 19 Oct 2022 08:08:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666192087; cv=none; d=google.com; s=arc-20160816; b=KfBF/FvbtyTsTgH8C6eQWvME353jauKTsavAY7n3mekdM4uCBtb8zCkKifwcf2VHiu MEFAnC2sxPtkvuqPBdyrk+BxqUCNbHSEg7luAfZRjVzN/t+e2QyEV6GI/QHhn4R+CKCe m7bi4EpgeBGan9SasjcOb9k3AYfvrZfYHqHTeBJqQsN0YGO2uRMKk10UjMnID57XOsvs RBot7oYeniy4pTfh3VGSGQv9RPLDEyI/FntKxo6YGd3M3ae4148fVhEkK5K1kMYgXtOw R2dOaDdgcC52E9UKM5h3U+jGL3++M6vJFEEe/AvdqmsO2hTCu86YGm9aS6HCFz7JOaym mprA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:dkim-signature :dkim-signature:from; bh=jlKWc1Grue9kQfYwJIiewArVOaAkhAK4uRE7V4DMseI=; b=fEnO0KNAOT4TFm0nhT0l816biB2D3iE7NpdyYNs9qsX+5qvUHZxS6G08ytwvIApskV 24FuEGrM6CI8x632HDYtdyLg4094phlYMCI01+K9NX3roQ88oPV6vhVdmL6RadtfgHRT Oj2b+Tzjktp1RgpudsLIxsHQ7ZvFFU+ANxkjEz1jvzR6MeWI+AWpk3puIWeIW1t/vKDe ffmcOqlJb/pLIiLgXNJSD/Bjc2PRGNEcPZJmKS7X5yziG7atfnhsO6kyZ6oTFkCh/KpG 7RCAzPsQyrQyjK8BaU5tmBwI5mIXtUsI/anDGl1t/vjuYMWkLYE+13pdTNCm2SHccVPp QkIw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=Kk2lnB0D; dkim=neutral (no key) header.i=@linutronix.de header.s=2020e; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id x18-20020a05640226d200b0045cc9072f14si17690549edd.8.2022.10.19.08.07.15; Wed, 19 Oct 2022 08:08:07 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=Kk2lnB0D; dkim=neutral (no key) header.i=@linutronix.de header.s=2020e; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229767AbiJSPEX (ORCPT <rfc822;samuel.l.nystrom@gmail.com> + 99 others); Wed, 19 Oct 2022 11:04:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39086 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232052AbiJSPEC (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 19 Oct 2022 11:04:02 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C308211E46A; Wed, 19 Oct 2022 07:58:35 -0700 (PDT) From: John Ogness <john.ogness@linutronix.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1666191368; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jlKWc1Grue9kQfYwJIiewArVOaAkhAK4uRE7V4DMseI=; b=Kk2lnB0DeVDuCcQ2SWcnH8R0PgShB2CZwwsounOmf0c7TAxDw35ZfqSbSJ1PexWKje9a+0 NMoma2X+T0p0buJRG2CD0IdcQ3ox2o2rPGrDdFrPUj49Z35+fPcKgQeojLLcyw6shd+JZ9 7r6p+CtHoY10o8YGBeUNnQ0FJ053rbDPZzoW8bfvOwNdxeAh5JUrxV2//iLX8tpfhO5B/d l3lLWkkazcGkc6qDO/d6aqqwdZkfKULXqPgH+kzjyq67FPhYBElPgpY4K3E1YW+mL8gUT4 aXnp3oFg8O4sBrVcA7QxClrsqr+RzG3dLrE+NIR5PRiZa07ARtMvihKFdcmmsw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1666191368; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jlKWc1Grue9kQfYwJIiewArVOaAkhAK4uRE7V4DMseI=; b=Hrgpan7bNf/oEJbZZM6hsAcz+zCVZiCrz6fuzTkbXs/R1ESYYwgaiON5lh57jBdACO27c+ UT3EpQHIZr7qOrCg== To: Petr Mladek <pmladek@suse.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org>, Steven Rostedt <rostedt@goodmis.org>, Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org, Jason Wessel <jason.wessel@windriver.com>, Daniel Thompson <daniel.thompson@linaro.org>, Douglas Anderson <dianders@chromium.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Jiri Slaby <jirislaby@kernel.org>, kgdb-bugreport@lists.sourceforge.net, linux-serial@vger.kernel.org Subject: [PATCH printk v2 12/38] tty: serial: kgdboc: use console_is_enabled() Date: Wed, 19 Oct 2022 17:01:34 +0206 Message-Id: <20221019145600.1282823-13-john.ogness@linutronix.de> In-Reply-To: <20221019145600.1282823-1-john.ogness@linutronix.de> References: <20221019145600.1282823-1-john.ogness@linutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,INVALID_DATE_TZ_ABSURD, RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747129034255327588?= X-GMAIL-MSGID: =?utf-8?q?1747129034255327588?= |
Series |
reduce console_lock scope
|
|
Commit Message
John Ogness
Oct. 19, 2022, 2:55 p.m. UTC
Replace (console->flags & CON_ENABLED) usage with console_is_enabled().
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
drivers/tty/serial/kgdboc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Wed, Oct 19, 2022 at 05:01:34PM +0206, John Ogness wrote: > Replace (console->flags & CON_ENABLED) usage with console_is_enabled(). > > Signed-off-by: John Ogness <john.ogness@linutronix.de> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Wed 2022-10-19 17:01:34, John Ogness wrote: > Replace (console->flags & CON_ENABLED) usage with console_is_enabled(). > > Signed-off-by: John Ogness <john.ogness@linutronix.de> Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr
Hi, On Wed, Oct 19, 2022 at 7:56 AM John Ogness <john.ogness@linutronix.de> wrote: > > Replace (console->flags & CON_ENABLED) usage with console_is_enabled(). > > Signed-off-by: John Ogness <john.ogness@linutronix.de> > --- > drivers/tty/serial/kgdboc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c > index e76f0186c335..b17aa7e49894 100644 > --- a/drivers/tty/serial/kgdboc.c > +++ b/drivers/tty/serial/kgdboc.c > @@ -533,7 +533,7 @@ static int __init kgdboc_earlycon_init(char *opt) > console_lock(); > for_each_console(con) { > if (con->write && con->read && > - (con->flags & (CON_BOOT | CON_ENABLED)) && > + (console_is_enabled(con) || (con->flags & CON_BOOT)) && <shrug>. I guess this is OK, but it feels a little pointless. If we're still directly looking at the CON_BOOT bit in con->flags it seems weird to be accessing CON_ENABLED through a special wrapper that's marked as a `data_race`. In our case it's _not_ a data race, right, since this function continues to hold the console_lock() even at the end of the series? I personally would drop this patch but if you really want it I won't object. -Doug
Hi, On Mon, Oct 24, 2022 at 3:46 PM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Wed, Oct 19, 2022 at 7:56 AM John Ogness <john.ogness@linutronix.de> wrote: > > > > Replace (console->flags & CON_ENABLED) usage with console_is_enabled(). > > > > Signed-off-by: John Ogness <john.ogness@linutronix.de> > > --- > > drivers/tty/serial/kgdboc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c > > index e76f0186c335..b17aa7e49894 100644 > > --- a/drivers/tty/serial/kgdboc.c > > +++ b/drivers/tty/serial/kgdboc.c > > @@ -533,7 +533,7 @@ static int __init kgdboc_earlycon_init(char *opt) > > console_lock(); > > for_each_console(con) { > > if (con->write && con->read && > > - (con->flags & (CON_BOOT | CON_ENABLED)) && > > + (console_is_enabled(con) || (con->flags & CON_BOOT)) && > > <shrug>. I guess this is OK, but it feels a little pointless. If we're > still directly looking at the CON_BOOT bit in con->flags it seems > weird to be accessing CON_ENABLED through a special wrapper that's > marked as a `data_race`. In our case it's _not_ a data race, right, > since this function continues to hold the console_lock() even at the > end of the series? I personally would drop this patch but if you > really want it I won't object. I realized that my statement isn't quite true. It actually only holds console_list_lock() even at the end of the series. Still, it seems weird that we're declaring the `data_race` on CON_ENABLED but not CON_BOOT ?
On 2022-10-24, Doug Anderson <dianders@chromium.org> wrote: > It actually only holds console_list_lock() even at the end of the > series. Still, it seems weird that we're declaring the `data_race` on > CON_ENABLED but not CON_BOOT ? You are correct that it is not a data race (because of the console_list_lock being held.) Petr has suggested adding a separate function for non-data-race callers. For v3 I will do this and use it here, probably called console_is_enabled_locked(). Usually CON_ENABLED is the only flag that is interesting during normal operation. The kgdboc case is an exception. I thought about if we should have console_flags() and console_flags_locked() to be able to handle general con->flags access. console_flags() would be marked with data_race(), console_flags_locked() would use lockdep to ensure the console_list_lock is held. But I would also like to have the _is_enabled special variant because how we check if a console is enabled will be different for the atomic consoles. I would like to hide those details in the _is_enabled implementation. John Ogness
On 2022-10-24, Doug Anderson <dianders@chromium.org> wrote: > It actually only holds console_list_lock() even at the end of the > series. Still, it seems weird that we're declaring the `data_race` on > CON_ENABLED but not CON_BOOT ? For my upcoming v3 I decided to drop this patch and will keep the existing direct reading of @flags. Instead of this patch, for v3 the comment will additionally mention why @flags is allowed to be directly read: /* * Hold the console_lock to guarantee that no consoles are * unregistered until the kgdboc_earlycon setup is complete. * Trapping the exit() callback relies on exit() not being * called until the trap is setup. This also allows safe * traversal of the console list and race-free reading of @flags. */ John Ogness
On Fri 2022-11-04 17:29:15, John Ogness wrote: > On 2022-10-24, Doug Anderson <dianders@chromium.org> wrote: > > It actually only holds console_list_lock() even at the end of the > > series. Still, it seems weird that we're declaring the `data_race` on > > CON_ENABLED but not CON_BOOT ? > > For my upcoming v3 I decided to drop this patch and will keep the > existing direct reading of @flags. Instead of this patch, for v3 the > comment will additionally mention why @flags is allowed to be directly > read: > > /* > * Hold the console_lock to guarantee that no consoles are ^^^^^^^^^^^^ > * unregistered until the kgdboc_earlycon setup is complete. My understanding is that this is synchronized by console_list_lock. Or do I miss something? > * Trapping the exit() callback relies on exit() not being > * called until the trap is setup. This also allows safe > * traversal of the console list and race-free reading of @flags. > */ Best Regards, Petr
On 2022-11-07, Petr Mladek <pmladek@suse.com> wrote: >> /* >> * Hold the console_lock to guarantee that no consoles are > ^^^^^^^^^^^^ >> * unregistered until the kgdboc_earlycon setup is complete. > > My understanding is that this is synchronized by console_list_lock. > Or do I miss something? Yes, in the end the comment will say console_list_lock. At this point in the series, the console_lock is still used. The comment is updated later (as in patch 34 of the v2 series). John
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c index e76f0186c335..b17aa7e49894 100644 --- a/drivers/tty/serial/kgdboc.c +++ b/drivers/tty/serial/kgdboc.c @@ -533,7 +533,7 @@ static int __init kgdboc_earlycon_init(char *opt) console_lock(); for_each_console(con) { if (con->write && con->read && - (con->flags & (CON_BOOT | CON_ENABLED)) && + (console_is_enabled(con) || (con->flags & CON_BOOT)) && (!opt || !opt[0] || strcmp(con->name, opt) == 0)) break; }