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


Groups > linux.kernel > #1503869

Re: [PATCH 1/3] printk: Fix kdb_trap_printk placement

From Peter Zijlstra <peterz@infradead.org>
Newsgroups linux.kernel
Subject Re: [PATCH 1/3] printk: Fix kdb_trap_printk placement
Date 2016-10-19 17:20 +0200
Message-ID <su0OR-308-23@gated-at.bofh.it> (permalink)
References <stGdr-4O9-19@gated-at.bofh.it> <stGdr-4O9-17@gated-at.bofh.it> <su0lQ-2yX-31@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wed, Oct 19, 2016 at 04:41:40PM +0200, Petr Mladek wrote:
> On Tue 2016-10-18 19:08:31, Peter Zijlstra wrote:
> > Some people figured vprintk_emit() makes for a nice API and exported
> > it, bypassing the kdb trap.
> > 
> > This still leaves vprintk_nmi() outside of the kbd reach, should that
> > be fixed too?
> 
> Good question! vkdb_printf() tries to avoid a deadlock but the code is racy:
> 
> int vkdb_printf(enum kdb_msgsrc src, const char *fmt, va_list ap)
> {
> [...]
> 	/* Serialize kdb_printf if multiple cpus try to write at once.
> 	 * But if any cpu goes recursive in kdb, just print the output,
> 	 * even if it is interleaved with any other text.
> 	 */
> 	if (!KDB_STATE(PRINTF_LOCK)) {
> 		KDB_STATE_SET(PRINTF_LOCK);
> 		spin_lock_irqsave(&kdb_printf_lock, flags);
> 		got_printf_lock = 1;
> 		atomic_inc(&kdb_event);
> 	} else {
> 		__acquire(kdb_printf_lock);
> 	}
> 
> 
> Let's have the following situation:
> 
> CPU1					CPU2
> 
> if (!KDB_STATE(PRINTF_LOCK)) {
> 	KDB_STATE_SET(PRINTF_LOCK);
> 
> 					if (!KDB_STATE(PRINTF_LOCK)) {
> 					} else {
> 						__acquire(kdb_printf_lock);
> 					}
> 
> Now, both CPUs are in the critical section and happily writing over each
> other, e.g. in
> 
> 	vsnprintf(next_avail, size_avail, fmt, ap);
> 
> I quess that we want to fix this race. But I am not sure if it will
> be done an NMI-safe way. I am going to send a patch for this.

Something like patch 3 in this series should do I suppose. But the
vkdb_printf() thing using spin_lock_irqsave() seems to suggest it was
never meant to be used from NMI context.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/3] make printk work again Peter Zijlstra <peterz@infradead.org> - 2016-10-18 19:20 +0200
  [PATCH 1/3] printk: Fix kdb_trap_printk placement Peter Zijlstra <peterz@infradead.org> - 2016-10-18 19:20 +0200
    Re: [PATCH 1/3] printk: Fix kdb_trap_printk placement Petr Mladek <pmladek@suse.com> - 2016-10-19 16:50 +0200
      Re: [PATCH 1/3] printk: Fix kdb_trap_printk placement Peter Zijlstra <peterz@infradead.org> - 2016-10-19 17:20 +0200
    Re: [PATCH 1/3] printk: Fix kdb_trap_printk placement Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-10-20 15:10 +0200
  [PATCH 3/3] early_printk: Add simple serialization to early_vprintk() Peter Zijlstra <peterz@infradead.org> - 2016-10-18 19:20 +0200
    Re: [PATCH 3/3] early_printk: Add simple serialization to  early_vprintk() Steven Rostedt <rostedt@goodmis.org> - 2016-10-18 19:30 +0200
      Re: [PATCH 3/3] early_printk: Add simple serialization to  early_vprintk() Peter Zijlstra <peterz@infradead.org> - 2016-10-18 19:40 +0200
        Re: [PATCH 3/3] early_printk: Add simple serialization to  early_vprintk() Steven Rostedt <rostedt@goodmis.org> - 2016-10-18 20:00 +0200
  [PATCH 2/3] early_printk: Add force_early_printk kernel parameter Peter Zijlstra <peterz@infradead.org> - 2016-10-18 19:20 +0200
  Re: [PATCH 0/3] make printk work again Peter Zijlstra <peterz@infradead.org> - 2016-10-19 16:30 +0200
  Re: [PATCH 0/3] make printk work again Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-10-19 16:30 +0200
  Re: [PATCH 0/3] make printk work again Jan Kara <jack@suse.cz> - 2016-10-19 17:00 +0200
    Re: [PATCH 0/3] make printk work again Peter Zijlstra <peterz@infradead.org> - 2016-10-19 18:20 +0200

csiph-web