linux-kernel - Re: [PATCH] printk: stop spining waiter when console resume to flush prb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YJkIK9cyHr46UAFP@alley>
Date:   Mon, 10 May 2021 12:17:15 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <senozhatsky@...omium.org>
Cc:     Luo Jiaxing <luojiaxing@...wei.com>, sergey.senozhatsky@...il.com,
        rostedt@...dmis.org, john.ogness@...utronix.de,
        linux-kernel@...r.kernel.org, linuxarm@...wei.com,
        bobo.shaobowang@...wei.com
Subject: Re: [PATCH] printk: stop spining waiter when console resume to flush
 prb

On Mon 2021-05-10 17:26:45, Sergey Senozhatsky wrote:
> On (21/05/07 18:36), Petr Mladek wrote:
> Well, the alternative patch set just gives everyone an API that selectively
> downgrades printk() to pre-console_sem_owner age: when console_unlock()
> would never handover the printing duty. It'll take no time until this
> function will pop up somewhere where we don't want it to be used.
> 
> E.g.
> 
> 	rcu_read_lock();
> 	...
> 	console_unlock_preemptible();
> 	...
> 	rcu_read_unlock();
> 
> lockdep_assert_preemption_enabled() is not entirely reliable - it
> depends on __lockdep_enabled, provided that system in question has
> CONFIG_PROVE_LOCKING set.

I understand the concern. I think that I would be able to sleep with
this. I believe that such problems would be discovered soon.

Also I doubt that it would spread that quickly. It is the same as
with printk_deferred(). It is just a workaround for a problem
that only few people are aware of.

If it is still a concern, we could make it static and block
any patches that would make it public.

But it does not make sense to fight over this now.
I am not sure that console_unlock_preemptible() is worth it after all.
Luo has to prove that it might fix a real life problem.

> It queues the work IF we have pending messages AND there are NO active
> console_sem waiters spinning on consolse_sem waiting for us to handover
> the printing duty. And IRQ shall write to consoles only N messages out
> of possibly M pending messages (M > N). N, obviously, should be small,
> e.g. 42 lines: if after 42 printed lines we didn't handover printing
> to another context then - queue another IRQ work and iret. But it keeps
> the console_owner mechanism enabled.

I am sorry but I am not going to continue in this discussion. Many
printk() designs have been discussed in the past and this is just
repeating the same again and again.

The current plan is to move console work to kthreads (separate
preemptive context). Using IRQ is a complete opposite way.

There is always the fight between getting the messages out as soon
as possible and the risk of breaking the system (softlockups,
deadlocks).

The kthread approach reduces the risk of system breakage to a bare
minimum. The price is that some messages might never reach console.
There is finally a consensus to give it a try. If it fails, we might
start looking for alternatives again.

Best Regards,
Petr