mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: linux-kernel@vger.kernel.org, Petr Mladek <pmladek@suse.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [RFC/PATCH] printk: Fix preferred console selection with multiple matches
Date: Wed, 11 Dec 2019 15:02:02 +1100	[thread overview]
Message-ID: <38b543cb91e936d7bd9f8885e585dd55032d83a4.camel@kernel.crashing.org> (raw)
In-Reply-To: <20191211020149.GN88619@google.com>

On Wed, 2019-12-11 at 11:01 +0900, Sergey Senozhatsky wrote:
> On (19/12/11 09:26), Benjamin Herrenschmidt wrote:
> 

> As far as I know, ->match() does not only match but also does ->setup().
> If we have two console list entries that match (one via aliasing and one
> via exact match) then the console driver is setup twice. Do all console
> drivers handle it? [double setup]

I don't think it's an issue but I may be wrong. I had a quick look
at some of the drivers and I don't really see why they would break but
I couldn't look at them all and I might be mistaken.

We could skip setup if the console is already enabled but I would
advise against that since the two calls might have different options
(the firmware baud rate could be different from the command line one
for example) and we want the options for the last one.

> If we could perform simple alias matching, without ->setup() call, and
> exact matching (strcmp()), and then, if newcon would match two entries,
> we would pick up the last matching entry and configure newcon only once.
> 
> This changes the order, tho.

Walking the array backwards might just be what we want actually for the
case at hand, but of course if some platforms or driver call
add_preferred_console *after* the command line parsing, then it will
break those.

Simple alias matching would require re-working all the match()
callbacks. That said I think it was a mistake to begin with to have
them include setup(). Those should have remained separate.

What about a compromise here:

Instead of walking the array and testing for preferred_console as we do
so, we first test the array entry pointed to by preferred_console
(doing both match & setup as today) and if that doesn't work, fallback
to our existing mechanism ?

It would be a first step. It wouldn't fix all cases but would be
something reasonable to backport.

Another approach woudl be to pass to add_preferred_console an argument
"bool user_specified" which we would use to set a CON_USER flag.

We could then do a two-pass lookup of the array where we first only
match user specified entries, then match the rest.

> [..]
> >  - Another match that is marked preferred_console, in which case in
> > addition to being enabled, the newly registered console will also be
> > made the default console (ie, first in the list with CONSDEV set). This
> > is actually what we want ! IE. The console matches the last specified
> > one on the command line.
> 
> Well, it still looks to me that what you want is to "ignore alias
> match and prefer exact match".

We don't want to ignore the alias match. But we do want to prefer the
exact match. We still want to keep the fallback to the alias match.

Cheers,
Ben.



  reply	other threads:[~2019-12-11  4:03 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-10  0:57 Benjamin Herrenschmidt
2019-12-10  8:01 ` Sergey Senozhatsky
2019-12-10 22:26   ` Benjamin Herrenschmidt
2019-12-11  2:01     ` Sergey Senozhatsky
2019-12-11  4:02       ` Benjamin Herrenschmidt [this message]
2019-12-11  5:35         ` Sergey Senozhatsky
2019-12-11 12:53         ` Petr Mladek
2019-12-10  9:15 ` Petr Mladek
2019-12-10 22:39   ` Benjamin Herrenschmidt
2019-12-11  9:17     ` Petr Mladek
2019-12-12  1:23       ` Sergey Senozhatsky
2019-12-16  0:09       ` Benjamin Herrenschmidt
2019-12-19  9:50         ` Petr Mladek
2019-12-12  0:35   ` Benjamin Herrenschmidt
2019-12-12  9:09     ` Petr Mladek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=38b543cb91e936d7bd9f8885e585dd55032d83a4.camel@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome