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
| ||
|
Date: Wed, 22 Mar 2017 17:40:58 +0100 From: Petr Mladek <pmladek@...e.com> To: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com> Cc: Steven Rostedt <rostedt@...dmis.org>, Andrew Morton <akpm@...ux-foundation.org>, Linus Torvalds <torvalds@...ux-foundation.org>, Peter Zijlstra <peterz@...radead.org>, "Rafael J . Wysocki" <rjw@...ysocki.net>, linux-kernel@...r.kernel.org, Sergey Senozhatsky <sergey.senozhatsky@...il.com>, Jan Kara <jack@...e.cz> Subject: Re: [RFC][PATCH 1/4] printk: introduce printing kernel thread On Mon 2017-03-06 21:45:51, Sergey Senozhatsky wrote: > This patch introduces a dedicated printing kernel thread - printk_kthread. > The main purpose of this kthread is to offload printing to a non-atomic > and always scheduleable context, which eliminates 4) and makes 1)-3) less > critical. printk() now just appends log messages to the kernel log buffer > and wake_up()s printk_kthread instead of locking console_sem and calling > into potentially unsafe console_unlock(). > > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > +/* > + * This disables printing offloading and instead attempts > + * to do the usual console_trylock()->console_unlock(). > + * > + * Note, this does not stop the printk_kthread if it's already > + * printing logbuf messages. > + */ > +void console_printing_thread_off(void) > +{ > + printk_kthread_disable++; > + barrier(); > +} > + > +/* This re-enables printk_kthread offloading. */ > +void console_printing_thread_on(void) > +{ > + barrier(); > + printk_kthread_disable--; > +} I really like that these functions are re-entrant. It will make our life much easier. Just a small nitpicking. I would prefer to use the name console_printk_kthread_off()/on(). I was several times confused by "printing_thread" when searching the sources. The common sub-string "printk_kthread" for all the related stuff would make my life easier ;-) Just an idea. printk() and printk_deferred() behave the same way when the kthread is enabled. I wonder if we should make it more explicit by using names like: printk_deferred_mode_disabled++; printk_deferred_mode_off(); printk_deferred_mode_on(); Also it is an already know term and a more generic name. This API is used globally while the kthread is an implementation detail. The offloading might be done another way in the future. Finally, I think about using this variable instead of the ugly LOGLEVEL_SCHED. The catch is that LOGLEVEL_SCHED forces the deferred mode while these functions force the opposite. I just think loudly. I wonder what is better to help people understand the code in the future. Otherwise, the patch looks fine to me. Well, there are some related things discussed in the other patches that might affect it. Best Regards, Petr
Powered by blists - more mailing lists