linux-kernel - Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup

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]

Date:   Tue, 16 Jan 2018 19:10:29 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Tejun Heo <tj@...nel.org>, akpm@...ux-foundation.org,
        linux-mm@...ck.org, Cong Wang <xiyou.wangcong@...il.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Mel Gorman <mgorman@...e.de>, Michal Hocko <mhocko@...nel.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Peter Zijlstra <peterz@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Jan Kara <jack@...e.cz>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
        rostedt@...e.goodmis.org, Byungchul Park <byungchul.park@....com>,
        Pavel Machek <pavel@....cz>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup

On (01/16/18 10:36), Petr Mladek wrote:
[..]
> > unfortunately disabling preemtion in console_unlock() is a bit
> > dangerous :( we have paths that call console_unlock() exactly
> > to flush everything (not only new pending messages, but everything)
> > that is in logbuf and we cannot return from console_unlock()
> > preliminary in that case.
> 
> You are right. Just to be sure. Are you talking about replaying
> the entire log when a new console is registered? Or do you know
> about more paths?

to the best of my knowledge CON_PRINTBUFFER is the only thing that
explicitly states
  "I want everything what's in logbuf, even if it has been already
   printed on other consoles"

the rest want to have only pending messages, so we can offload
from there.

CON_PRINTBUFFER registration can happen any time. e.g. via modprobe
netconsole. we can be up and running for some time when netconsole
joins in, so that CON_PRINTBUFFER thing can be painful.

> If I get it correctly, we allow to hand off the lock even when
> replying the entire log. But you are right that we should
> enable preemption in this case because there are many messages
> even without printk() activity.

> IMHO, the best solution would be to reply the log in a
> separate process asynchronously and do not block existing
> consoles in the meantime. But I am not sure if it is worth
> the complexity. Anyway, it is a future work.
[..]
> > > int printk_kthread_func(void *data)
> > > {
> > > 	while(1) {
> > > 		 if (!pending_messaged)
> > > 			schedule();
> > > 
> > > 		if (console_trylock_spinning())
> > > 			console_unlock();
> > > 
> > > 		cond_resched();
> > > 	}
> > > }
> > 
> > overall that's very close to what I have in one of my private branches.
> > console_trylock_spinning() for some reason does not perform really
> > well on my made-up internal printk torture tests. it seems that I
> > have a much better stability (no lockups and so on) when I also let
> > printk_kthread to sleep on console_sem(). but I will look further.
> 
> I believe that it is not trivial. console_trylock_spinning() is
> tricky and the timing is important.

yes, timing seems to be very important.
 *as far as I can see from the traces on my printk torture tests*

> For example, it might be tricky if a torture test affects the normal
> workflow by many interrupts. We might need to call even more
> console_unlock() code with spinning enabled to improve the success
> ratio. Another problem is that the kthread must be scheduled on
> another CPU.

yes, I always schedule it on another CPU [if any].

> And so on. I believe that there are many more problems and areas
> for improvement.

right.

	-ss