From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752617AbcBKOlJ (ORCPT ); Thu, 11 Feb 2016 09:41:09 -0500 Received: from mx2.suse.de ([195.135.220.15]:56124 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752531AbcBKOlI (ORCPT ); Thu, 11 Feb 2016 09:41:08 -0500 Date: Thu, 11 Feb 2016 15:41:05 +0100 From: Petr Mladek To: Sergey Senozhatsky Cc: Andrew Morton , Jan Kara , Tejun Heo , Kyle McMartin , Dave Jones , Calvin Owens , linux-kernel@vger.kernel.org, Sergey Senozhatsky , "Paul E. McKenney" Subject: Re: [RFC][PATCH v3 4/4] printk: set may_schedule for some of console_trylock callers Message-ID: <20160211144105.GM3305@pathway.suse.cz> References: <1453536913-9545-1-git-send-email-sergey.senozhatsky@gmail.com> <1453536913-9545-5-git-send-email-sergey.senozhatsky@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1453536913-9545-5-git-send-email-sergey.senozhatsky@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat 2016-01-23 17:15:13, Sergey Senozhatsky wrote: > console_unlock() allows to cond_resched() if its caller has > set `console_may_schedule' to 1, since > 'commit 8d91f8b15361 ("printk: do cond_resched() between lines while > outputting to consoles")'. > > The rules are: > -- console_lock() always sets `console_may_schedule' to 1 > -- console_trylock() always sets `console_may_schedule' to 0 > > However, console_trylock() callers (among them is printk()) do > not always call printk() from atomic contexts, and some of them > can cond_resched() in console_unlock(), so console_trylock() > can set `console_may_schedule' to 1 for such processes. > > For !CONFIG_PREEMPT_COUNT kernels, however, console_trylock() > always sets `console_may_schedule' to 0. > > It's possible to drop explicit preempt_disable()/preempt_enable() > in vprintk_emit(), because console_unlock() and console_trylock() > are now smart enough: > a) console_unlock() does not cond_resched() when it's unsafe > (console_trylock() takes care of that) > b) console_unlock() does can_use_console() check. > > Signed-off-by: Sergey Senozhatsky > --- > kernel/printk/printk.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 99925ce..097ca8b 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1769,20 +1769,12 @@ asmlinkage int vprintk_emit(int facility, int level, > if (!in_sched) { > lockdep_off(); > /* > - * Disable preemption to avoid being preempted while holding > - * console_sem which would prevent anyone from printing to > - * console > - */ > - preempt_disable(); > - > - /* > * Try to acquire and then immediately release the console > * semaphore. The release will print out buffers and wake up > * /dev/kmsg and syslog() users. > */ > if (console_trylock()) > console_unlock(); > - preempt_enable(); > lockdep_on(); > } > > @@ -2115,7 +2107,16 @@ int console_trylock(void) > return 0; > } > console_locked = 1; > - console_may_schedule = 0; > + /* > + * On !PREEMPT_COUNT kernels we can't reliably detect if it's safe > + * to schedule -- e.g. calling printk while holding a spin_lock, > + * because preempt_disable()/preempt_enable() are just barriers and > + * don't modify preempt_count() there. console_may_schedule is > + * always 0 on !PREEMPT_COUNT kernels. > + */ > + console_may_schedule = !oops_in_progress && > + preemptible() && > + !rcu_preempt_depth(); > return 1; We discussed this a lot but I am still a bit nervous ;-) Avoid scheduling when oops_in_progress makes sense. preemptible() takes care of preemption and IRQ contexts. The comment above explains that it is safe to use here. The check for rcu_preempt_depth() makes sense. But is it safe, please? rcu_preempt_depth() returns 0 if CONFIG_PREEMPT_RCU is not enabled. It means that you are not able to detect RCU read section and it might cause problems. I rather add Paul into CC. Best Regards, Petr