Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1333016 > unrolled thread

[PATCH v4 0/2] let printk()/console_trylock() callers to cond_resched()

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/2] let printk()/console_trylock() callers to cond_resched() Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-02-12 19:40 +0100
    [PATCH v4 3/3] printk: check CON_ENABLED in have_callable_console() Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-02-12 19:40 +0100
      Re: [PATCH v4 3/3] printk: check CON_ENABLED in  have_callable_console() Petr Mladek <pmladek@suse.com> - 2016-02-16 15:50 +0100
        Re: [PATCH v4 3/3] printk: check CON_ENABLED in  have_callable_console() Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-02-16 16:50 +0100
    [PATCH v4 2/3] printk: set may_schedule for some of console_trylock callers Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-02-12 19:40 +0100
      Re: [PATCH v4 2/3] printk: set may_schedule for some of  console_trylock callers Petr Mladek <pmladek@suse.com> - 2016-02-16 15:50 +0100
        Re: [PATCH v4 2/3] printk: set may_schedule for some of  console_trylock callers Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-02-16 16:50 +0100

#1333016 — [PATCH v4 0/2] let printk()/console_trylock() callers to cond_resched()

Hello,

console_unlock() allows to cond_resched() if its caller has
set `console_may_schedule' to 1 (this functionality present
since commit '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

printk() calls console_unlock() with preemption desabled, which
basically can lead to RCU stalls, watchdog soft lockups, etc. if
something is simultaneously calling printk() frequent enough (IOW,
console_sem owner always has new data to send to console divers
and can't leave console_unlock() for a long time).

printk()->console_trylock() callers do not necessarily execute in
atomic contexts, and some of them can cond_resched() in
console_unlock(). console_trylock() can set `console_may_schedule'
to 1 (allow cond_resched() later in consoe_unlock()) when it's safe.

v4:
-- added "printk: check CON_ENABLED in have_callable_console()" patch
-- drop a "optional optimization" patch from the series (Petr Mladek)
-- merge 0001 and "drop console_trylock_for_printk" patches (Petr Mladek)
-- tweak console_trylock() comment (Petr Mladek)

v2-v3 (thanks to Petr Mladek for reviews):
-- do not call can_use_console() on every iteration in console_unlock() (Petr Mladek)
-- move "This stops the holder of console_sem.." comment (noted by Petr Mladek)
-- take extra care of !PREEMPT_COUNT kernels (Petr Mladek)
-- call_console_drivers() still must check cpu_online && CON_ANYTIME (Petr Mladek)
-- removed console_trylock_for_printk() (noted by Petr Mladek)

v1-v2:
-- make have_callable_console() available for !PRINTK configs (lkp@intel.com)
-- take care of RCU preempt kernels in console_trylock()


Sergey Senozhatsky (3):
  printk: move can_use_console out of console_trylock_for_printk
  printk: set may_schedule for some of console_trylock callers
  printk: check CON_ENABLED in have_callable_console()

 kernel/printk/printk.c | 121 +++++++++++++++++++++++--------------------------
 1 file changed, 57 insertions(+), 64 deletions(-)

-- 
2.7.1

[toc] | [next] | [standalone]

#1333017 — [PATCH v4 3/3] printk: check CON_ENABLED in have_callable_console()

have_callable_console() must also test CON_ENABLED bit, not just
CON_ANYTIME. We may have disabled CON_ANYTIME console so printk
can wrongly assume that it's safe to call_console_drivers().

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 kernel/printk/printk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 9f0c3e1..9917f69 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2139,7 +2139,8 @@ static int have_callable_console(void)
 	struct console *con;
 
 	for_each_console(con)
-		if (con->flags & CON_ANYTIME)
+		if ((con->flags & CON_ENABLED) &&
+				(con->flags & CON_ANYTIME))
 			return 1;
 
 	return 0;
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1335475 — Re: [PATCH v4 3/3] printk: check CON_ENABLED in have_callable_console()

On Sat 2016-02-13 03:37:12, Sergey Senozhatsky wrote:
> have_callable_console() must also test CON_ENABLED bit, not just
> CON_ANYTIME. We may have disabled CON_ANYTIME console so printk
> can wrongly assume that it's safe to call_console_drivers().
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

Makes perfect sense.

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

[toc] | [prev] | [next] | [standalone]

#1335571 — Re: [PATCH v4 3/3] printk: check CON_ENABLED in have_callable_console()

On (02/16/16 15:44), Petr Mladek wrote:
> On Sat 2016-02-13 03:37:12, Sergey Senozhatsky wrote:
> > have_callable_console() must also test CON_ENABLED bit, not just
> > CON_ANYTIME. We may have disabled CON_ANYTIME console so printk
> > can wrongly assume that it's safe to call_console_drivers().
> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> 
> Makes perfect sense.
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

thanks.

	-ss

[toc] | [prev] | [next] | [standalone]

#1333023 — [PATCH v4 2/3] printk: set may_schedule for some of console_trylock callers

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 <sergey.senozhatsky@gmail.com>
---
 kernel/printk/printk.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0d65a81..9f0c3e1 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,20 @@ int console_trylock(void)
 		return 0;
 	}
 	console_locked = 1;
-	console_may_schedule = 0;
+	/*
+	 * When PREEMPT_COUNT disabled 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 there
+	 * and preempt_count() is always 0.
+	 *
+	 * RCU read sections have a separate preemption counter when
+	 * PREEMPT_RCU enabled thus we must take extra care and check
+	 * rcu_preempt_depth(), otherwise RCU read sections modify
+	 * preempt_count().
+	 */
+	console_may_schedule = !oops_in_progress &&
+			preemptible() &&
+			!rcu_preempt_depth();
 	return 1;
 }
 EXPORT_SYMBOL(console_trylock);
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1335476 — Re: [PATCH v4 2/3] printk: set may_schedule for some of console_trylock callers

On Sat 2016-02-13 03:37:11, 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 <sergey.senozhatsky@gmail.com>

It looks safe after all.

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

[toc] | [prev] | [next] | [standalone]

#1335553 — Re: [PATCH v4 2/3] printk: set may_schedule for some of console_trylock callers

On (02/16/16 15:43), Petr Mladek wrote:
> On Sat 2016-02-13 03:37:11, 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 <sergey.senozhatsky@gmail.com>
> 
> It looks safe after all.
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> 

thanks.

	-ss

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web