From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AC1A435E543 for ; Thu, 20 Nov 2025 16:50:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763657451; cv=none; b=Ynr4bqea4ZNzokMcGWX1B62cqKwwLwzTZM2j08HSx8+6jJxMvH0ivWDkBHZmlLgUFNtK1HW45GqDu6NKWcZzXODJXVf9+Hycga98ipt3C726iJalOaWy7mqxT6f/LF3UjcyUIbqwCZC1xiZa9vNPrR6W6o2NcSAfZctA81G5/mc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763657451; c=relaxed/simple; bh=/3dISRrzkGwiD5MsKW61Jau+11PrDIRIq/TlOlGWRwM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=MoaHaO/wOcB/fsowb7spTcHG7hoganB4o40Gs+8XWcgbtfWkS3e6Ko/Q/7Lbh4nwAWaY/qbR8jZWfxrPdVUz4xHQZ/bIWKlCIri7wYDXhmn5Y0ll7RFz+G1QILXJPG9Nc0g3ucAmU8s6sBlQcomChlMCsQmkawcUDBF968dW9s8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=abXSebSG; arc=none smtp.client-ip=209.85.218.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="abXSebSG" Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-b737c6c13e1so210070366b.3 for ; Thu, 20 Nov 2025 08:50:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1763657447; x=1764262247; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=mbyQEjB79q2NrxF81ZG/St/Jz+aCzHN/I1slndvSYrE=; b=abXSebSGaOqze/ljtudwgnOTowTSNt8RayPkEWCmEsV6DkLP00sGarnl9WbfH5HzzK kRhqix0eKJ8nQ4Bm8Cn+YXC9xk6caCtPrCbWQ6o4aHDzYhRvtvGj+O5Y9/h4pSrgqcT9 p0+Mgld7io14yjciK7HUP5vl/3kzo0DFAjUt8oYK4dhBdzSuFik7YpGz2AB/Uf58I27k 5sl5jYiAijWzDzv2YGl3Go6u/TibzKhepuxC4fRAwhhMc/c/qZiqZEn128cm2FnUsb+O 06oiFE7nncSYiNIHxBOG66GDhkpdhCJ3b3mBQWxp8GDl5//Xbz3yAZo5vXD/3Fxq70Ak MnTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763657447; x=1764262247; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=mbyQEjB79q2NrxF81ZG/St/Jz+aCzHN/I1slndvSYrE=; b=TzPfRIO+2MjJDRuRClOf556XZ0o9dsHsFOw+iO3vVaShz/NJS1ahlxJPOrGm75S8Op xC0b+Mf/Jj2DoPZdLD7Amwr1jz/Ip3OLAyGSgnim1wnZKTEIHzh7Z5n4szGXyGMrofFj Nd/KwM0FSdbTcRKgwnEgc1xX1wm8BI+J/Js7bfvXXE6JLme7v7jF9CoKB6CGR+1n5VE4 Ic0vjS6wPJHLa/WK5SD9ZFBZFqza6W0OOa8+WInH/5RoV8wPo1mZnU0CFz5Jqb+RqjJq WBHzkWpAtRYRu1xnyJKJU8q/SWBLQpnicBxZ1sLsBUGSP5VRXh6uL7HKYeneuAuo30xK sSFw== X-Gm-Message-State: AOJu0Ywlpzvtgb/3eGuRDup8yQpQWkk1eZMzgL3VyVciOLDn6DEnrWew 97GehZnEWqIkAi0zpmvFLGlEF+v3vYIsqlPUrUhIK6BjWm+i5CG+ls7dTwW0upV0kYs= X-Gm-Gg: ASbGnctGiW2btS2ZeyFsoNduWWMXSpMhSIbl9k/Fk7bXTBtyTjUyIsIUvnYm+Zgxjbd bNuC34AEdnoWwXaprvi/o2eEBokYvKwjl1E76VhHqOl+N92CdWfUNNjv8lTm0hg9chsuXmDejrO +y4O8cdpPWeZvtsQB3aHXx6xpyz3bg6VC77NFm5mJYEu+ugWDUmoBB+9vO6ztMLvCTKKenVSHGB n8fNSclWdDzbgo7Z/NvHuWrW2ZV1+3z2Lsjn0c4sQgZ/Zjns3oI3omVi1/xsrkSAAykQTY4TZoo vV2WglTLzQcxi0vFfYLU/xhV7bnmKOMTFzwef202l9jxaeyAGg6EwjdFshKOsF5dAHg/TB97s3h P5a6Rq8+xQPqX5eexstXoPZBKQUpRF8/dFsn7YtA8zO+x5aF23C/aqG2By9HQN0oJeCbgog9Kqc qZ5FtnYWqVKjsFEg== X-Google-Smtp-Source: AGHT+IGsPcQc0VXlK7UsphpLa+TRgi9+evqbs2zL0eEZL7L/vftBCih7aGOhmutSIOlCOY7rkv/N9Q== X-Received: by 2002:a17:906:f59b:b0:b72:af1f:af7d with SMTP id a640c23a62f3a-b7654fafc1bmr425110666b.29.1763657446932; Thu, 20 Nov 2025 08:50:46 -0800 (PST) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-b7654d3bf64sm242666666b.16.2025.11.20.08.50.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Nov 2025 08:50:46 -0800 (PST) Date: Thu, 20 Nov 2025 17:50:44 +0100 From: Petr Mladek To: Chris Down Cc: linux-kernel@vger.kernel.org, Greg Kroah-Hartman , Sergey Senozhatsky , Steven Rostedt , John Ogness , Geert Uytterhoeven , Tony Lindgren , kernel-team@fb.com Subject: Re: [PATCH v7 07/13] printk: console: Introduce sysfs interface for per-console loglevels Message-ID: References: <464d918b5497f6c7463b96b8241a65a575d932e0.1763492585.git.chris@chrisdown.name> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <464d918b5497f6c7463b96b8241a65a575d932e0.1763492585.git.chris@chrisdown.name> On Wed 2025-11-19 03:07:27, Chris Down wrote: > A sysfs interface under /sys/class/console/ is created that permits > viewing and configuring per-console attributes. This is the main > interface with which we expect users to interact with and configure > per-console loglevels. > > --- a/Documentation/admin-guide/per-console-loglevel.rst > +++ b/Documentation/admin-guide/per-console-loglevel.rst > @@ -69,3 +69,41 @@ The default value for ``kernel.console_loglevel`` comes from > ``CONFIG_CONSOLE_LOGLEVEL_DEFAULT``, or ``CONFIG_CONSOLE_LOGLEVEL_QUIET`` if > ``quiet`` is passed on the kernel command line. > > +Console attributes > +~~~~~~~~~~~~~~~~~~ > + > +Registered consoles are exposed at ``/sys/class/console``. For example, if you > +are using ``ttyS0``, the console backing it can be viewed at > +``/sys/class/console/ttyS0/``. The following files are available: > + > +* ``effective_loglevel`` (r): The effective loglevel after considering all > + loglevel authorities. For example, it shows the value of the console-specific > + loglevel when a console-specific loglevel is defined, and shows the global > + console loglevel value when the console-specific one is not defined. > + > +* ``effective_loglevel_source`` (r): The loglevel authority which resulted in > + the effective loglevel being set. The following values can be present: > + > + * ``local``: The console-specific loglevel is in effect. > + > + * ``global``: The global loglevel (``kernel.console_loglevel``) is in > + effect. Set a console-specific loglevel to override it. > + > + * ``ignore_loglevel``: ``ignore_loglevel`` was specified on the kernel > + command line or at ``/sys/module/printk/parameters/ignore_loglevel``. > + Disable it to use level controls. > + > +* ``loglevel`` (rw): The local, console-specific loglevel for this console. > + This will be in effect if no other global control overrides it. Look at > + ``effective_loglevel`` and ``effective_loglevel_source`` to verify that. > + > +Deprecated > +~~~~~~~~~~ > + > +* ``kernel.printk`` sysctl: this takes four values, setting > + ``kernel.console_loglevel``, ``kernel.default_message_loglevel``, the minimum > + console loglevel, and a fourth unused value. The interface is generally > + considered to quite confusing, doesn't perform checks on the values given, The seems to be a typo. I would use either of + "considered as quite confusing" + "considered to be quite confusing" > + and is unaware of per-console loglevel semantics. > + > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -4010,6 +4010,9 @@ static void try_enable_default_console(struct console *newcon) > if (newcon->index < 0) > newcon->index = 0; > > + newcon->level = LOGLEVEL_DEFAULT; This does not fit in this patch. A better place would be the 3rd patch which added .level into struct console. But I am not sure if we need it at all. The LOGLEVEL_DEFAULT will be set by register_console() as a fallback anyway. It is the same reason why I suggested to remove a similar code from try_enable_preferred_console(). > + newcon->classdev = NULL; This should not be needed. struct console is defined statically and this field should always be NULL before the console is registered. > + > if (console_call_setup(newcon, NULL) != 0) > return; > > diff --git a/kernel/printk/sysfs.c b/kernel/printk/sysfs.c > new file mode 100644 > index 000000000000..5252e6e04908 > --- /dev/null > +++ b/kernel/printk/sysfs.c > @@ -0,0 +1,213 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include > +#include > +#include > +#include > +#include > +#include "internal.h" > + > +static const char * > +console_effective_loglevel_source_str(const struct console *con) > +{ > + enum loglevel_source source; > + const char *str; > + int con_level; > + int cookie; > + > + cookie = console_srcu_read_lock(); > + con_level = console_srcu_read_loglevel(con); > + console_srcu_read_unlock(cookie); I think that this was discussed in v6. But I guess that we did not reach a consensus. The SRCU read lock is not need here. In fact, it is wrong because unregister_console_locked() calls device_unregister() after synchronize_srcu(&console_srcu). The sysfs API makes sure that struct console could not get removed when the interface is accessed. I suggest to create a custom wrappers for the sysfs interface. Something like: /** * console_srcu_read_loglevel - Locklessly read the console specific loglevel * when accessing the related sysfs interface * @con: struct console pointer of console to read loglevel from * * Locklessly reading @con->level provides a consistent read value because * there is at most one CPU modifying @con->level and that CPU is using only * read-modify-write operations to do so. * * Only use this function to read flags via the related sysfs interface. * The sysfs API makes sure that the structure could not disappear while * the interface is used. * * Context: Sysfs interface for the given console. * Return: The current value of the @con->level field. */ static inline int console_sysfs_read_loglevel(const struct console *con) { /* * The READ_ONCE() matches the WRITE_ONCE() when @level is modified * for registered consoles. */ return data_race(READ_ONCE(con->level)); } /** * console_sysfs_write_flags - Write the console specific loglevel via * sysfs interface. * @con: struct console pointer of console to write flags to * @flags: new flags value to write * * Only use this function to write flags via the related sysfs interface. * The sysfs API makes sure that the structure could not disappear while * the interface is used. * * Context: Any context. */ static inline void console_sysfs_write_loglevel(struct console *con, int con_level) { /* This matches the READ_ONCE() in console_srcu_read_flags(). */ WRITE_ONCE(con->level, con_level); } > + source = console_effective_loglevel_source(con_level); > + > + switch (source) { > + case LLS_IGNORE_LOGLEVEL: > + str = "ignore_loglevel"; > + break; > + case LLS_LOCAL: > + str = "local"; > + break; > + case LLS_GLOBAL: > + str = "global"; > + break; > + default: > + str = "unknown"; > + break; > + } > + > + return str; > +} > + > +static ssize_t effective_loglevel_source_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct console *con = dev_get_drvdata(dev); > + const char *str; > + > + str = console_effective_loglevel_source_str(con); > + return sysfs_emit(buf, "%s\n", str); > +} > + > +static DEVICE_ATTR_RO(effective_loglevel_source); > + > +static ssize_t effective_loglevel_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct console *con = dev_get_drvdata(dev); > + int con_level; > + int cookie; > + > + cookie = console_srcu_read_lock(); > + con_level = console_srcu_read_loglevel(con); > + console_srcu_read_unlock(cookie); Same here. > + return sysfs_emit(buf, "%d\n", console_effective_loglevel(con_level)); > +} > + > +static DEVICE_ATTR_RO(effective_loglevel); > + > +static ssize_t loglevel_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct console *con = dev_get_drvdata(dev); > + int con_level; > + int cookie; > + > + cookie = console_srcu_read_lock(); > + con_level = console_srcu_read_loglevel(con); > + console_srcu_read_unlock(cookie); and here. > + return sysfs_emit(buf, "%d\n", con_level); > +} > + > +static ssize_t loglevel_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + struct console *con = dev_get_drvdata(dev); > + ssize_t ret; > + int level; > + int cookie; > + > + ret = kstrtoint(buf, 10, &level); > + if (ret < 0) > + return ret; > + > + /* -1 means "use global loglevel" */ > + if (level == -1) > + goto out; > + > + /* > + * Reject level 0 (KERN_EMERG) - per-console loglevel must be > 0. > + * Emergency messages should go to all consoles, so they cannot be > + * filtered per-console. > + */ > + if (level == 0) > + return -ERANGE; > + > + if (console_clamp_loglevel(level) != level) > + return -ERANGE; > + > + /* > + * If the system has a minimum console loglevel set (via sysctl or > + * kernel parameter), enforce it. This prevents setting per-console > + * loglevels below the system minimum. > + */ > + if (minimum_console_loglevel > CONSOLE_LOGLEVEL_MIN && > + level < minimum_console_loglevel) > + return -ERANGE; > + > +out: > + cookie = console_srcu_read_lock(); > + WRITE_ONCE(con->level, level); > + console_srcu_read_unlock(cookie); and here. Note that we need to add the data_race() as discussed in v6. > + > + return size; > +} > + I propose to squash the following changes into this patch: diff --git a/Documentation/admin-guide/per-console-loglevel.rst b/Documentation/admin-guide/per-console-loglevel.rst index 69eede12e20f..f621a6785f81 100644 --- a/Documentation/admin-guide/per-console-loglevel.rst +++ b/Documentation/admin-guide/per-console-loglevel.rst @@ -255,7 +255,7 @@ Deprecated * ``kernel.printk`` sysctl: this takes four values, setting ``kernel.console_loglevel``, ``kernel.default_message_loglevel``, the minimum console loglevel, and a fourth unused value. The interface is generally - considered to quite confusing, doesn't perform checks on the values given, + considered to be quite confusing, doesn't perform checks on the values given, and is unaware of per-console loglevel semantics. Chris Down , 18-November-2025 diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 170fa8e14ea9..e73276880d51 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -4118,9 +4118,6 @@ static void try_enable_default_console(struct console *newcon) if (newcon->index < 0) newcon->index = 0; - newcon->level = LOGLEVEL_DEFAULT; - newcon->classdev = NULL; - if (console_call_setup(newcon, NULL) != 0) return; diff --git a/kernel/printk/sysfs.c b/kernel/printk/sysfs.c index 5252e6e04908..fc029b425809 100644 --- a/kernel/printk/sysfs.c +++ b/kernel/printk/sysfs.c @@ -6,17 +6,57 @@ #include #include "internal.h" +/** + * console_srcu_read_loglevel - Locklessly read the console specific loglevel + * when accessing the related sysfs interface + * @con: struct console pointer of console to read loglevel from + * + * Locklessly reading @con->level provides a consistent read value because + * there is at most one CPU modifying @con->level and that CPU is using only + * read-modify-write operations to do so. + * + * Only use this function to read flags via the related sysfs interface. + * The sysfs API makes sure that the structure could not disappear while + * the interface is used. + * + * Context: Sysfs interface for the given console. + * Return: The current value of the @con->level field. + */ +static inline +int console_sysfs_read_loglevel(const struct console *con) +{ + /* + * The READ_ONCE() matches the WRITE_ONCE() when @level is modified + * for registered consoles. + */ + return data_race(READ_ONCE(con->level)); +} + +/** + * console_sysfs_write_flags - Write the console specific loglevel via + * sysfs interface. + * @con: struct console pointer of console to write flags to + * @flags: new flags value to write + * + * Only use this function to write flags via the related sysfs interface. + * The sysfs API makes sure that the structure could not disappear while + * the interface is used. + * + * Context: Any context. + */ +static inline +void console_sysfs_write_loglevel(struct console *con, int con_level) +{ + /* This matches the READ_ONCE() in console_srcu_read_flags(). */ + WRITE_ONCE(con->level, con_level); +} + static const char * -console_effective_loglevel_source_str(const struct console *con) +console_effective_loglevel_source_str(int con_level) { enum loglevel_source source; const char *str; - int con_level; - int cookie; - cookie = console_srcu_read_lock(); - con_level = console_srcu_read_loglevel(con); - console_srcu_read_unlock(cookie); source = console_effective_loglevel_source(con_level); switch (source) { @@ -43,8 +83,10 @@ static ssize_t effective_loglevel_source_show(struct device *dev, { struct console *con = dev_get_drvdata(dev); const char *str; + int con_level; - str = console_effective_loglevel_source_str(con); + con_level = console_sysfs_read_loglevel(con); + str = console_effective_loglevel_source_str(con_level); return sysfs_emit(buf, "%s\n", str); } @@ -55,11 +97,8 @@ static ssize_t effective_loglevel_show(struct device *dev, { struct console *con = dev_get_drvdata(dev); int con_level; - int cookie; - cookie = console_srcu_read_lock(); - con_level = console_srcu_read_loglevel(con); - console_srcu_read_unlock(cookie); + con_level = console_sysfs_read_loglevel(con); return sysfs_emit(buf, "%d\n", console_effective_loglevel(con_level)); } @@ -70,11 +109,8 @@ static ssize_t loglevel_show(struct device *dev, struct device_attribute *attr, { struct console *con = dev_get_drvdata(dev); int con_level; - int cookie; - cookie = console_srcu_read_lock(); - con_level = console_srcu_read_loglevel(con); - console_srcu_read_unlock(cookie); + con_level = console_sysfs_read_loglevel(con); return sysfs_emit(buf, "%d\n", con_level); } @@ -84,7 +120,6 @@ static ssize_t loglevel_store(struct device *dev, struct device_attribute *attr, struct console *con = dev_get_drvdata(dev); ssize_t ret; int level; - int cookie; ret = kstrtoint(buf, 10, &level); if (ret < 0) @@ -115,9 +150,7 @@ static ssize_t loglevel_store(struct device *dev, struct device_attribute *attr, return -ERANGE; out: - cookie = console_srcu_read_lock(); - WRITE_ONCE(con->level, level); - console_srcu_read_unlock(cookie); + console_sysfs_write_loglevel(con, level); return size; } Best Regards, Petr