Re: [PATCH] printk: Enough to disable preemption in printk deferred context

From: Petr Mladek
Date: Wed Apr 19 2023 - 08:03:41 EST


On Wed 2023-04-19 11:11:52, John Ogness wrote:
> On 2023-04-19, Petr Mladek <pmladek@xxxxxxxx> wrote:
> > it is safe to interrupt one writer now. The preemption still
> > has to be disabled because the deferred context is CPU specific.
>
> Really it is enough to disable migration.

True. But it gets too far to my taste. As you describe below.
It affects all printk's on the CPU.

Sigh, even the enabled intrrupts might be questionable. For example,
when the iterrupt is from a watchdog and want's to report a stall.

> We need to keep an eye on the usage of this function. By allowing
> interrupts and preemption, it means that other printk's on that CPU will
> also be deferred if the context interrupted within the deferred block.

A solution would be to make this more clear in the comment.
Something like:

/*
* The printk_deferred_enter/exit macros are available only as a hack.
* They define a per-CPU context where all printk console printing is
* deferred because it might cause a deadlock otherwise.
*
* The API user is responsible for calling the corresponding enter/exit
* pair on the same CPU. It is highly recommended to use them only in
* a context with interrupts disabled. Otherwise, other unrelated
* printk() calls might be deferred when they interrupt/preempt
* the deferred code section.
*/

Another solution would be to stay on the "safe" side and keep the
comment as is or even enforce disabling interrupts by the API.

I would personally just improve the comment. It is good to describe
the situation correctly. We could always add restrictions when
there are problems in practice.

Best Regards,
Petr