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;
Date:   Fri, 30 Sep 2022 11:24:19 +0200
From:   Petr Mladek <pmladek@suse.com>
To:     John Ogness <john.ogness@linutronix.de>
Cc:     Sergey Senozhatsky <senozhatsky@chromium.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH printk 06/18] printk: Protect [un]register_console() with
 a mutex
Message-ID: <Yza1wxVcH2bsITcs@alley>
References: <20220924000454.3319186-1-john.ogness@linutronix.de>
 <20220924000454.3319186-7-john.ogness@linutronix.de>
 <YzMT27FVllY3u05k@alley>
 <87mtajkqvu.fsf@jogness.linutronix.de>
 <YzW9ExRVjv6PzvWR@alley>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YzW9ExRVjv6PzvWR@alley>
Precedence: bulk

On Thu 2022-09-29 17:43:17, Petr Mladek wrote:
> On Thu 2022-09-29 01:48:29, John Ogness wrote:
> > On 2022-09-27, Petr Mladek <pmladek@suse.com> wrote:
> > > Hmm, the new mutex is really nasty. It has very strange semantic.
> > > It makes the locking even more complicated.
> > 
> > We are working to replace the BKL-console_lock with new separate clearly
> > defined mechanisms.
> >
> > The new mutex provides full synchronization for list changes as well as
> > changes to items of that list. (Really console->flags is the only change
> > to items of the list.)

We should actually make the the reading of console->flags safe under
srcu_read_lock(). It would allow to use the SRCU walk by all the
readers.

> > For some places in the code it is very clear that the console_lock can
> > be completely replaced (either with srcu or the new mutex). For other
> > places, it is not yet clear why the console_lock is being used and so
> > both console_lock and mutex are used.
> 
> One important and tricky location is console_trylock() in
> vprintk_emit(). And the related for_each_console() called from
> console_unlock()->console_flush_all().
> 
> It is the legacy mode that tries to print to the consoles immediately.
> I am not sure if we could _ever_ remove this mode.
> 
> I would really like to avoid state where we have two locks (semaphore
> and mutex) serializing the same thing (console list).

That said, I could imagine implementing console_lock() so that it
would be implemented by mutex when the legacy mode is disabled and
semaphore when it is allowed.

You were talking about command-line option that would allow to
disable the legacy mode on production RT systems. And I guess
that you added mutex because it behaves better on RT.

Also I could imagine using console_list_lock() as a wrapper
to console_lock(). It might help to distinguish locations where
the list is traversed and where the console_lock() is used for
another reason. I mean to remove the big-kernel-lock character
of the console_lock().

You know, the more locks we have, the bigger is the risk of
deadlocks, and the more hacks would be needed in
console_flush_on_panic(). And I am afraid
that console_lock() will be with us for many years and
maybe forever.

Does it make any sense, please?

Best Regards,
Petr