linux-kernel - Re: [RFC][PATCH v8 1/2] printk: Make printk() completely async

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: <20160401093602.GB503@swordfish>
Date:	Fri, 1 Apr 2016 18:36:02 +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>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jan Kara <jack@...e.com>, Tejun Heo <tj@...nel.org>,
	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	linux-kernel@...r.kernel.org,
	Byungchul Park <byungchul.park@....com>,
	Jan Kara <jack@...e.cz>
Subject: Re: [RFC][PATCH v8 1/2] printk: Make printk() completely async

Hello Petr,

On (04/01/16 10:59), Petr Mladek wrote:
[..]
> CPU0					CPU1
> 
> printk()
> 
>   if (printk_kthread)
>   # fails and need_flush_console
>   # stays false
> 
> 					init_printk_kthread()
> 					  # put printk_thread into
> 					  # run queue
> 					  printk_kthread = ...;
> 
>   if (!in_panic && printk_kthread)
>     wake_up_process(printk_kthread);
> 
> 
> 					# printk kthread finally gets
> 					# scheduled
> 					printk_kthread_func()
> 
> 					set_current_state(TASK_INTERRUPTIBLE);
> 					if (!need_flush_console)
> 					  schedule();
> 
> => printk_kthread is happily sleeping without calling console.

oohh, that tiny race. well, looks quite harmless, it's unlikely that
we had printk()-s up until late_initcall(init_printk_kthread) and not
a single one ever after. but good find!

so the check
	if (printk_kthread)
		need_flush_console = 1

can be replaced with
	if (!printk_sync)
		need_flush_console = 1

or... may be dropped.

> I do not see any code that will modify need_flush_console when
> printk.synchronous is modified at runtime.

printk.synchronous is RO; no runtime modification.

> I know that all this is rather theoretical. My main point is to remove
> unnecessary checks that make the code harder to read and does not bring
> any big advantage.

my point is that those checks are just .loads, which help to avoid
spurious .stores from various CPUs.

CPU1		CPU2		CPU3		...	CPU1024

lock logbuf
need_flush=1
unlock logbuf
		lock logbuf
		need_flush=1
		unlock logbuf
				lock logbuf
				need_flush=1
				unlock logbuf
wakeup kthread					
						...
							lock logbuf
							need_flush=1
							unlock logbuf

isn't it a bit useless need_flush=1 traffic?

	-ss