From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751619AbcDWG4j (ORCPT ); Sat, 23 Apr 2016 02:56:39 -0400 Received: from mx2.suse.de ([195.135.220.15]:41332 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751224AbcDWG4i (ORCPT ); Sat, 23 Apr 2016 02:56:38 -0400 Date: Sat, 23 Apr 2016 08:56:33 +0200 From: Jan Kara To: Sergey Senozhatsky Cc: Andrew Morton , Jan Kara , Petr Mladek , Tejun Heo , Tetsuo Handa , linux-kernel@vger.kernel.org, Byungchul Park , Sergey Senozhatsky Subject: Re: [PATCH v12 3/3] printk: make printk.synchronous param rw Message-ID: <20160423065633.GA6771@quack2.suse.cz> References: <1461333180-2897-1-git-send-email-sergey.senozhatsky@gmail.com> <1461333180-2897-4-git-send-email-sergey.senozhatsky@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1461333180-2897-4-git-send-email-sergey.senozhatsky@gmail.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 22-04-16 22:53:00, Sergey Senozhatsky wrote: > Change `synchronous' printk param to be RW, so user space > can change printk mode back and forth to/from sync mode > (which is considered to be more reliable). > > Signed-off-by: Sergey Senozhatsky The patch looks good to me. One suggestion below: > @@ -1785,7 +1782,7 @@ asmlinkage int vprintk_emit(int facility, int level, > * operate in sync mode once panic() occurred. > */ > if (console_loglevel != CONSOLE_LOGLEVEL_MOTORMOUTH && > - printk_kthread) { > + !printk_sync && printk_kthread) { > /* Offload printing to a schedulable context. */ > printk_kthread_need_flush_console = true; > wake_up_process(printk_kthread); It would seem more future-proof to hide '!printk_sync && printk_kthread' into a wrapper function as it is somewhat subtle detail that printk_kthread needn't exist while !printk_sync and I can imagine someone forgetting to check that in the future. Something like 'can_print_async()'? But I don't feel too strongly about that so feel free to add: Reviewed-by: Jan Kara regardless whether you change this or not. Honza -- Jan Kara SUSE Labs, CR