[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9563158.18pcnM708K@nerdopolis2>
Date: Fri, 14 Mar 2025 08:52:15 -0400
From: Adam Simonelli <adamsimonelli@...il.com>
To: Petr Mladek <pmladek@...e.com>
Cc: linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
Jiri Slaby <jirislaby@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Andy Shevchenko <andy.shevchenko@...il.com>,
Steven Rostedt <rostedt@...dmis.org>,
John Ogness <john.ogness@...utronix.de>,
Sergey Senozhatsky <senozhatsky@...omium.org>
Subject:
Re: [PATCH v10 1/1] printk: Add an option to allow ttynull to be a default
console device
On Friday, March 14, 2025 5:44:43 AM EDT Petr Mladek wrote:
> On Thu 2025-03-13 20:41:04, adamsimonelli@...il.com wrote:
> > From: Adam Simonelli <adamsimonelli@...il.com>
> >
> > The new option is CONFIG_NULL_TTY_DEFAULT_CONSOLE.
> >
> > if enabled, and CONFIG_VT is disabled, ttynull will become the default
> > primary console device, based on the link order.
>
> The result is not longer based on the link order.
>
> The linking order affected the ordering the console initcalls.
> But this patch calls add_preferred_console() directly in console_init()
> before processing the init calls...
>
> I would just remove the ", based on the link order".
>
> > ttynull will be the only console device usually with this option enabled.
> > Some architectures do call add_preferred_console() which may add another
> > console though.
>
> I would add here the following line:
>
> Motivation:
>
> to clearly separate the description of the new behavior from the motivation.
>
> > Many distributions ship with CONFIG_VT enabled. On tested desktop hardware
> > if CONFIG_VT is disabled, the default console device falls back to
> > /dev/ttyS0 instead of /dev/tty.
> >
> > This could cause issues in user space, and hardware problems:
> >
> > 1. The user space issues include the case where /dev/ttyS0 is
> > disconnected, and the TCGETS ioctl, which some user space libraries use
> > as a probe to determine if a file is a tty, is called on /dev/console and
> > fails. Programs that call isatty() on /dev/console and get an incorrect
> > false value may skip expected logging to /dev/console.
> >
> > 2. The hardware issues include the case if a user has a science instrument
> > or other device connected to the /dev/ttyS0 port, and they were to upgrade
> > to a kernel that is disabling the CONFIG_VT option, kernel logs will then be
> > sent to the device connected to /dev/ttyS0 unless they edit their kernel
> > command line manually.
> >
> > The new CONFIG_NULL_TTY_DEFAULT_CONSOLE option will give users and
> > distribution maintainers an option to avoid this. Disabling CONFIG_VT and
> > enabling CONFIG_NULL_TTY_DEFAULT_CONSOLE will ensure the default kernel
> > console behavior is not dependant on hardware configuration by default, and
>
> s/dependant/dependent/
>
> > avoid unexpected new behavior on devices connected to the /dev/ttyS0 serial
> > port.
> >
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 07668433644b..9dd807717cd4 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -4277,6 +4277,11 @@ void __init console_init(void)
> > initcall_t call;
> > initcall_entry_t *ce;
> >
> > +#ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
> > + if (!console_set_on_cmdline)
> > + add_preferred_console("ttynull", 0, NULL);
>
> checkpatch.pl reports that there are used spaces instead of
> tabs in the two lines above. I suggest to use some editor which takes
> care of proper indentation, e.g. emacs or vim and run
> ./scripts/checkpatch.pl before sending pathes ;-)
>
> > +#endif
> > +
> > /* Setup the default TTY line discipline. */
> > n_tty_init();
>
> With the above changes:
>
> Reviewed-by: Petr Mladek <pmladek@...e.com>
> Tested-by: Petr Mladek <pmladek@...e.com>
>
> There is no need to resend this patch unless there are other comments.
> I could make the changes when committing the patch.
>
> Best Regards,
> Petr
>
Thanks! I think I will resend though, just so I can get it right
Powered by blists - more mailing lists