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


Groups > linux.kernel > #1611720 > unrolled thread

[RFC][PATCHv2 0/8] printk: introduce printing kernel thread

Started bySergey Senozhatsky <sergey.senozhatsky@gmail.com>
First post2017-03-29 11:30 +0200
Last post2017-04-04 10:30 +0200
Articles 19 on this page of 59 — 10 participants

Back to article view | Back to linux.kernel


Contents

  [RFC][PATCHv2 0/8] printk: introduce printing kernel thread Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-03-29 11:30 +0200
    [RFC][PATCHv2 2/8] printk: introduce printing kernel thread Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-03-29 11:30 +0200
      Re: [RFC][PATCHv2 2/8] printk: introduce printing kernel thread Petr Mladek <pmladek@suse.com> - 2017-04-04 11:10 +0200
        Re: [RFC][PATCHv2 2/8] printk: introduce printing kernel thread Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-04 11:40 +0200
      Re: [RFC][PATCHv2 2/8] printk: introduce printing kernel thread Pavel Machek <pavel@ucw.cz> - 2017-04-06 19:20 +0200
        Re: [RFC][PATCHv2 2/8] printk: introduce printing kernel thread Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-07 07:20 +0200
          Re: [RFC][PATCHv2 2/8] printk: introduce printing kernel thread Pavel Machek <pavel@ucw.cz> - 2017-04-07 09:30 +0200
            Re: [RFC][PATCHv2 2/8] printk: introduce printing kernel thread Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-07 10:20 +0200
              Re: [RFC][PATCHv2 2/8] printk: introduce printing kernel thread Pavel Machek <pavel@ucw.cz> - 2017-04-07 14:10 +0200
    [RFC][PATCHv2 5/8] sysrq: switch to printk.emergency mode in unsafe places Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-03-29 11:30 +0200
      Re: [RFC][PATCHv2 5/8] sysrq: switch to printk.emergency mode in  unsafe places Petr Mladek <pmladek@suse.com> - 2017-03-31 17:40 +0200
        Re: [RFC][PATCHv2 5/8] sysrq: switch to printk.emergency mode in  unsafe places Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-04-01 02:10 +0200
    [RFC][PATCHv2 1/8] printk: move printk_pending out of per-cpu Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-03-29 11:40 +0200
      Re: [RFC][PATCHv2 1/8] printk: move printk_pending out of per-cpu Petr Mladek <pmladek@suse.com> - 2017-03-31 15:20 +0200
        Re: [RFC][PATCHv2 1/8] printk: move printk_pending out of per-cpu Peter Zijlstra <peterz@infradead.org> - 2017-03-31 15:40 +0200
          Re: [RFC][PATCHv2 1/8] printk: move printk_pending out of per-cpu Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-03 13:30 +0200
            Re: [RFC][PATCHv2 1/8] printk: move printk_pending out of per-cpu Petr Mladek <pmladek@suse.com> - 2017-04-03 14:50 +0200
    [RFC][PATCHv2 6/8] kexec: switch to printk.emergency mode in unsafe places Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-03-29 11:40 +0200
      Re: [RFC][PATCHv2 6/8] kexec: switch to printk.emergency mode in  unsafe places Petr Mladek <pmladek@suse.com> - 2017-03-31 17:40 +0200
    [RFC][PATCHv2 8/8] printk: enable printk offloading Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-03-29 11:40 +0200
      Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-03-31 04:40 +0200
        Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-03-31 06:10 +0200
          Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Ye Xiaolong <xiaolong.ye@intel.com> - 2017-03-31 08:50 +0200
            Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-03-31 16:50 +0200
              Re: [printk]  fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage ebiederm@xmission.com (Eric W. Biederman) - 2017-03-31 17:40 +0200
                Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Jan Kara <jack@suse.cz> - 2017-04-03 11:40 +0200
                  Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Petr Mladek <pmladek@suse.com> - 2017-04-03 12:10 +0200
                  Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Pavel Machek <pavel@ucw.cz> - 2017-04-06 19:40 +0200
                    Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-07 06:50 +0200
                      Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Pavel Machek <pavel@ucw.cz> - 2017-04-07 09:20 +0200
                        Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-07 09:50 +0200
                          Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Pavel Machek <pavel@ucw.cz> - 2017-04-07 10:20 +0200
                            Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-07 14:20 +0200
                              Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Pavel Machek <pavel@ucw.cz> - 2017-04-07 14:50 +0200
                                Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 16:50 +0200
                                Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-04-07 17:20 +0200
                                  Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Peter Zijlstra <peterz@infradead.org> - 2017-04-07 17:30 +0200
                                    Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-04-07 17:50 +0200
                                      Re: [printk]  fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage ebiederm@xmission.com (Eric W. Biederman) - 2017-04-09 20:30 +0200
                                        Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-10 06:50 +0200
                                  Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Pavel Machek <pavel@ucw.cz> - 2017-04-09 12:20 +0200
                                    Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-10 07:00 +0200
                                      Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Petr Mladek <pmladek@suse.com> - 2017-04-10 14:00 +0200
                            Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 16:40 +0200
                              Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Pavel Machek <pavel@ucw.cz> - 2017-04-09 12:00 +0200
                Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-03 13:00 +0200
            Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Ye Xiaolong <xiaolong.ye@intel.com> - 2017-04-05 09:40 +0200
              Re: [printk]  fbc14616f4:  BUG:kernel_reboot-without-warning_in_test_stage Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-05 10:50 +0200
      Re: [RFC][PATCHv2 8/8] printk: enable printk offloading Petr Mladek <pmladek@suse.com> - 2017-04-03 17:50 +0200
        Re: [RFC][PATCHv2 8/8] printk: enable printk offloading Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-04-04 14:30 +0200
    [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-03-29 11:40 +0200
      Re: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe  places Petr Mladek <pmladek@suse.com> - 2017-03-31 17:10 +0200
      Re: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe  places Pavel Machek <pavel@ucw.cz> - 2017-04-06 19:30 +0200
        Re: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe  places Andreas Mohr <andi@lisas.de> - 2017-04-09 13:00 +0200
          Re: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe  places Petr Mladek <pmladek@suse.com> - 2017-04-10 14:30 +0200
            Re: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe  places Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-04-10 16:40 +0200
    [RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-03-29 11:40 +0200
      Re: [RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter Petr Mladek <pmladek@suse.com> - 2017-04-03 17:30 +0200
        Re: [RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-04-04 10:30 +0200

Page 3 of 3 — ← Prev page 1 2 [3]


#1619469 — Re: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage

FromPavel Machek <pavel@ucw.cz>
Date2017-04-09 12:20 +0200
SubjectRe: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage
Message-ID<tui6T-1tb-21@gated-at.bofh.it>
In reply to#1618901

[Multipart message — attachments visible in raw view] — view raw

On Sat 2017-04-08 00:13:06, Sergey Senozhatsky wrote:
> On (04/07/17 14:44), Pavel Machek wrote:
> [..]
> > > [..]
> > > > I believe "spend at most 2 seconds in printk(), then print a warning
> > > > and offload" is a solution closer to what we had before.
> > > 
> > > a warning here can be very noisy.
> > 
> > Well, on normally-configured it should be ok. We don't commonly see
> > printk problems... If it is too noisy, perhaps we should increase from
> > 2 seconds, but I don't think it will be problem.
> 
> we are looking at different typical setups :) serial console being 45
> seconds behind logbuf does not surprise me anymore.
> 
> [..]
> > > what we have been thinking about is something like printk-stall detection.
> > > we probably (there are some if-s) can detect in printk() that offloading
> > > does not work and we must automatically switch to printk_emergency mode.
> > > that, in theory, can relax our dependency on printk_emergency_begin/end
> > > being in the right place at the right time. need to think more about it.
> > 
> > So... I don't really like the begin/end interface. I would rather have
> > printk_emergency(KERN_ ...).
> 
> you mean a single printk_emergency() switches printk to emergency mode
> or printk_emergency(KERN_ ... ) is a single message that must be printed
> in emergency mode?

The latter. Having state is ugly.

> printk() depends on console_trylock(). we can't expect printk_emergency(KERN_ ...)
> to always do more than just log_store().
> 
> the idea behind begin/end interface is that you can do
> 
> 	emergency_begin
> 	printk
> 	pr_cont
> 	pr_cont
> 	pr_cont
> 	printk
> 	dump_stack
> 	emergency_end
> 
> with out the need of rewriting dump_stack() or anything else to use
> printk_emergency(). we, for example, do this in sysrq patch from this
> series.

Well.. I guess it is less work to include emergency_begin/end() but I
also believe result will state-less solution will be cleaner.

> > Second... I don't think "stuck detector" is that helpful. What I
> > usually seen was some rather innocent kernel message followed by
> > hard-lock. That's where "message delayed" is useful..
> 
> a side note,
> that's rather unclear to me how would "message delayed" really help.
> if your system hard-lockup so badly and there are no printk messages
> even from NMI watchdog, then we won't be able to print that message.

We are talking about

   printk("unusual condition");
   do_something_clever(); /* Which unfortunately hard-crashes the machine */

that works with my proposal, but not with yours. Seen it happen many
times before.

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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


#1619632 — Re: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2017-04-10 07:00 +0200
SubjectRe: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage
Message-ID<tuzAJ-4sI-3@gated-at.bofh.it>
In reply to#1619469
On (04/09/17 12:12), Pavel Machek wrote:
[..]
> > a side note,
> > that's rather unclear to me how would "message delayed" really help.
> > if your system hard-lockup so badly and there are no printk messages
> > even from NMI watchdog, then we won't be able to print that message.
> 
> We are talking about
> 
>    printk("unusual condition");
>    do_something_clever(); /* Which unfortunately hard-crashes the machine */
> 
> that works with my proposal, but not with yours. Seen it happen many
> times before.

I see your point, sure.
I can't completely agree on "that works with my proposal, but not with yours."

on SMP system this would be true only if no other CPU holds the console_sem
at the time we call printk(). (skipping irrelevant cases when we have suspended
console or !online CPU and !CON_ANYTIME console). and there is nothing that
makes "no other CPU holds the console_sem" always true on SMP system at any
given point in time. so no, "A always works, B never works" is not accurate.

but, once again, I see your point.

	-ss

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


#1619853 — Re: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage

FromPetr Mladek <pmladek@suse.com>
Date2017-04-10 14:00 +0200
SubjectRe: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage
Message-ID<tuG9b-av-5@gated-at.bofh.it>
In reply to#1619632
On Mon 2017-04-10 13:53:39, Sergey Senozhatsky wrote:
> On (04/09/17 12:12), Pavel Machek wrote:
> [..]
> > > a side note,
> > > that's rather unclear to me how would "message delayed" really help.
> > > if your system hard-lockup so badly and there are no printk messages
> > > even from NMI watchdog, then we won't be able to print that message.
> > 
> > We are talking about
> > 
> >    printk("unusual condition");
> >    do_something_clever(); /* Which unfortunately hard-crashes the machine */
> > 
> > that works with my proposal, but not with yours. Seen it happen many
> > times before.
> 
> I see your point, sure.
> I can't completely agree on "that works with my proposal, but not with yours."
> 
> on SMP system this would be true only if no other CPU holds the console_sem
> at the time we call printk(). (skipping irrelevant cases when we have suspended
> console or !online CPU and !CON_ANYTIME console). and there is nothing that
> makes "no other CPU holds the console_sem" always true on SMP system at any
> given point in time. so no, "A always works, B never works" is not accurate.
> 
> but, once again, I see your point.

A compromise might be to move the offloading from vprintk_emit() to
console_unlock(). By other words, the printk could always try to
flush some messages to the console. The console might trigger
the offload (wakeup kthread) after few lines or when the printing
takes too long.

We could go even furter. We could replace the cond_resched() in
console_unlock() with a need_resched() check. Then we could avoid
sleeping with console_sem taken.

It will avoid the softlockups caused by printk(). It should
work pretty well in most critical situations.

Of course, it will not guarantee that we will see all messages when
there is a flood of messages from many CPUs. But it was never
guaranteed.

Best Regards,
Petr

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


#1618849 — Re: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage

FromSteven Rostedt <rostedt@goodmis.org>
Date2017-04-07 16:40 +0200
SubjectRe: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage
Message-ID<ttDdn-8mD-5@gated-at.bofh.it>
In reply to#1618561
On Fri, 7 Apr 2017 10:14:49 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> > serial console can be quite slow. and port->lock, that is acquired by
> > console_unlock()->call_console_drivers()->write(), is also accessible
> > by serial driver's IRQ handler, and this lock may be busy long
> > enough -- as long as that IRQ handler transmits/receives chars. but
> > that's not the point.  
> 
> Well. This is what we had for 20 years.

But for the last 20 years we were not booting on machines with over 200
CPUs. Well, we were, but those had custom kernels (which probably
dismantled printk).

-- Steve

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


#1619468 — Re: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage

FromPavel Machek <pavel@ucw.cz>
Date2017-04-09 12:00 +0200
SubjectRe: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage
Message-ID<tuhNv-161-3@gated-at.bofh.it>
In reply to#1618849

[Multipart message — attachments visible in raw view] — view raw

On Fri 2017-04-07 10:29:17, Steven Rostedt wrote:
> On Fri, 7 Apr 2017 10:14:49 +0200
> Pavel Machek <pavel@ucw.cz> wrote:
> 
> > > serial console can be quite slow. and port->lock, that is acquired by
> > > console_unlock()->call_console_drivers()->write(), is also accessible
> > > by serial driver's IRQ handler, and this lock may be busy long
> > > enough -- as long as that IRQ handler transmits/receives chars. but
> > > that's not the point.  
> > 
> > Well. This is what we had for 20 years.
> 
> But for the last 20 years we were not booting on machines with over 200
> CPUs. Well, we were, but those had custom kernels (which probably
> dismantled printk).

Well, not a problem. Just find a solution that works for dual core
machines as well as it did for 20 years.

2 seconds timeout as proposed earlier should work well for big
machines, and with no regressions on small machines.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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


#1615090 — Re: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2017-04-03 13:00 +0200
SubjectRe: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage
Message-ID<ts7Sh-5KY-9@gated-at.bofh.it>
In reply to#1614118
On (03/31/17 10:28), Eric W. Biederman wrote:
[..]
> > ... I'd also probably add pr_emerg() print-out to emergency_restart(),
> > the same way kernel_restart()/kernel_halt()/kernel_power_off() do.
> >
> > for those cases when emergency_restart() is called with printk in
> > kthreaded mode, not in emergency mode.
> 
> No. No. No.
> 
> emergency_restart should be the equivalent of a watchdog going off.
> AKA it is long past the point where you want to be coordinating
> with other parts of the kernel.  Rebooting is the priority.
> A print statement absolutely does not belong in emergency_restart.
> 
> The fact that nothing managed to get printed out without magic flushing
> code is highly disturbing.

Eric, have you checked what is usually going on right before the
emergency_restart() call?

a quick grep.

kernel/panic.c

	pr_emerg("Kernel panic - not syncing: %s\n", buf);
...
	console_flush_on_panic();
...
	emergency_restart();


kernel/debug/kdb/kdb_main.c

	kdb_printf("forcing reboot\n");
	kdb_reboot(0, NULL);
		emergency_restart();


kernel/debug/gdbstub.c

	gdb_cmd_reboot()
		printk(KERN_CRIT "Executing emergency reboot\n");
		machine_emergency_restart();


drivers/tty/sysrq.c

	__handle_sysrq()
		pr_info("SysRq : ");
		pr_cont("%s\n", op_p->action_msg);
		op_p->handler(key);
			sysrq_handle_reboot()
				emergency_restart()

and so on...


all those printk()-s, that are happening right before emergency_restart(),
in fact flush (!) all the pending logbuf messages to the serial console.
and seems it doesn't cause any troubles on you side. but having printk()
not one line _before_the emergency_restart(), but _in_ emergency_restart()
is all of a sudden very disturbing. how come?


> Looking from the outside this patchset appears to be broken by design.
> 
> If you don't want kernel functions suffering from the overhead of
> printing to a slow output device, don't do that then.

sorry, this is not productive. "don't use printk()" is not a solution.


> The point of printk is to give debugging output.  You have fundamentally
> incapacitated printk from serving it's primary purpose.

the point of the patch set is that printk has a fundamental issue -- it
can easily soft/hard lockup the system; it can stall RCU; it can cause
OOM; and so on and on and on.

	-ss

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


#1616653 — Re: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage

FromYe Xiaolong <xiaolong.ye@intel.com>
Date2017-04-05 09:40 +0200
SubjectRe: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage
Message-ID<tsNHP-85H-7@gated-at.bofh.it>
In reply to#1613665
On 03/31, Ye Xiaolong wrote:
>On 03/31, Sergey Senozhatsky wrote:
>>On (03/31/17 11:35), Sergey Senozhatsky wrote:
>>[..]
>>> > [   21.009531] VFS: Warning: trinity-c2 using old stat() call. Recompile your binary.
>>> > [   21.148898] VFS: Warning: trinity-c0 using old stat() call. Recompile your binary.
>>> > [   22.298208] warning: process `trinity-c2' used the deprecated sysctl system call with 
>>> > 
>>> > Elapsed time: 310
>>> > BUG: kernel reboot-without-warning in test stage
>>> 
>>> so as far as I understand, this is the "missing kernel messages"
>>> type of bug report. a worst case scenario.
>>
>>panic() should have called console_flush_on_panic(), which sould have
>>flushed the messages regardless the printk_kthread state. so it probably
>>was not panic() that rebooted the kernel. (probably).
>>
>>kernel_restart() and kernel_halt() have pr_emerg() messages, printk switches
>>to printk_emergency mode the first time it sees EMERG level message. (may be
>>we switch to late).
>>
>>on the other hand, there is a emergency_restart(), where we don't switch
>>to printk_emergency mode and don't flush the existing kernel messages.
>>there is a bunch of places that call emergency_restart(), including sysrq.
>>
>>may I ask you, how do you usually restart the vm after the test?
>>`echo X > /proc/sysrq-trigger'?
>
>Yes.
>
>>
>>does this patch make it any better?
>
>I am trying it and will post the result once I get it.

Sorry for the late. I applied the patch of on top of the fbc14616f4 ("printk: enable printk offloading") 
and the "reboot-without-waring" issue is gone for 6 times of tests.

testcase/path_params/tbox_group/run: trinity/300s/vm-kbuild-yocto-ia32

fbc14616f483788a  a75384abd8885080d6923d7036  
----------------  --------------------------  
          4:4         -100%            0:6     dmesg.BUG:kernel_reboot-without-warning_in_test_stage

Thanks,
Xiaolong

>
>Thanks,
>Xiaolong
>>
>>---
>> drivers/tty/sysrq.c | 8 ++------
>> 1 file changed, 2 insertions(+), 6 deletions(-)
>>
>>diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
>>index 817dfb69914d..069f5540be36 100644
>>--- a/drivers/tty/sysrq.c
>>+++ b/drivers/tty/sysrq.c
>>@@ -240,7 +240,6 @@ static DECLARE_WORK(sysrq_showallcpus, sysrq_showregs_othercpus);
>> 
>> static void sysrq_handle_showallcpus(int key)
>> {
>>-	printk_emergency_begin();
>> 	/*
>> 	 * Fall back to the workqueue based printing if the
>> 	 * backtrace printing did not succeed or the
>>@@ -255,7 +254,6 @@ static void sysrq_handle_showallcpus(int key)
>> 		}
>> 		schedule_work(&sysrq_showallcpus);
>> 	}
>>-	printk_emergency_end();
>> }
>> 
>> static struct sysrq_key_op sysrq_showallcpus_op = {
>>@@ -282,10 +280,8 @@ static struct sysrq_key_op sysrq_showregs_op = {
>> 
>> static void sysrq_handle_showstate(int key)
>> {
>>-	printk_emergency_begin();
>> 	show_state();
>> 	show_workqueue_state();
>>-	printk_emergency_end();
>> }
>> static struct sysrq_key_op sysrq_showstate_op = {
>> 	.handler	= sysrq_handle_showstate,
>>@@ -296,9 +292,7 @@ static struct sysrq_key_op sysrq_showstate_op = {
>> 
>> static void sysrq_handle_showstate_blocked(int key)
>> {
>>-	printk_emergency_begin();
>> 	show_state_filter(TASK_UNINTERRUPTIBLE);
>>-	printk_emergency_end();
>> }
>> static struct sysrq_key_op sysrq_showstate_blocked_op = {
>> 	.handler	= sysrq_handle_showstate_blocked,
>>@@ -537,6 +531,7 @@ void __handle_sysrq(int key, bool check_mask)
>> 	int orig_log_level;
>> 	int i;
>> 
>>+	printk_emergency_begin();
>> 	rcu_sysrq_start();
>> 	rcu_read_lock();
>> 	/*
>>@@ -582,6 +577,7 @@ void __handle_sysrq(int key, bool check_mask)
>> 	}
>> 	rcu_read_unlock();
>> 	rcu_sysrq_end();
>>+	printk_emergency_end();
>> }
>> 
>> void handle_sysrq(int key)
>>-- 
>>2.12.2
>>

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


#1616710 — Re: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2017-04-05 10:50 +0200
SubjectRe: [printk] fbc14616f4: BUG:kernel_reboot-without-warning_in_test_stage
Message-ID<tsONA-kg-19@gated-at.bofh.it>
In reply to#1616653
On (04/05/17 15:29), Ye Xiaolong wrote:
[..]
> >>does this patch make it any better?
> >
> >I am trying it and will post the result once I get it.
> 
> Sorry for the late. I applied the patch of on top of the fbc14616f4 ("printk: enable printk offloading") 
> and the "reboot-without-waring" issue is gone for 6 times of tests.
> 
> testcase/path_params/tbox_group/run: trinity/300s/vm-kbuild-yocto-ia32
> 
> fbc14616f483788a  a75384abd8885080d6923d7036  
> ----------------  --------------------------  
>           4:4         -100%            0:6     dmesg.BUG:kernel_reboot-without-warning_in_test_stage

thanks!

	-ss

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


#1615351 — Re: [RFC][PATCHv2 8/8] printk: enable printk offloading

FromPetr Mladek <pmladek@suse.com>
Date2017-04-03 17:50 +0200
SubjectRe: [RFC][PATCHv2 8/8] printk: enable printk offloading
Message-ID<tscoW-jE-19@gated-at.bofh.it>
In reply to#1611739
On Wed 2017-03-29 18:25:11, Sergey Senozhatsky wrote:
> Initialize the kernel printing thread and enable printk()
> offloading.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  kernel/printk/printk.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 0d96839bb450..acfdc50580db 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2796,6 +2796,25 @@ static int printk_kthread_func(void *data)
>  	return 0;
>  }
>  
> +/*
> + * Init printk kthread at late_initcall stage, after core/arch/device/etc.
> + * initialization.
> + */
> +static int __init init_printk_kthread(void)
> +{
> +	struct task_struct *thread;
> +
> +	thread = kthread_run(printk_kthread_func, NULL, "printk");
> +	if (IS_ERR(thread)) {
> +		pr_err("printk: unable to create printing thread\n");
> +		return PTR_ERR(thread);
> +	}
> +
> +	printk_kthread = thread;
> +	return 0;
> +}
> +late_initcall(init_printk_kthread);

I like the simplicity. I just wonder if people on tiny devices might
want to disable it. In each case, it does not make sense on non-SMP
machines or when people force the emergency mode all the time.

I am not sure what is the practice here. I wonder if we should be
proactive or keep it as is and wait until anyone complains. IMHO,
it is not that big deal but...

Best Regards,
Petr

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


#1615974 — Re: [RFC][PATCHv2 8/8] printk: enable printk offloading

FromSergey Senozhatsky <sergey.senozhatsky@gmail.com>
Date2017-04-04 14:30 +0200
SubjectRe: [RFC][PATCHv2 8/8] printk: enable printk offloading
Message-ID<tsvKW-4O6-31@gated-at.bofh.it>
In reply to#1615351
On (04/03/17 17:42), Petr Mladek wrote:
> > +/*
> > + * Init printk kthread at late_initcall stage, after core/arch/device/etc.
> > + * initialization.
> > + */
> > +static int __init init_printk_kthread(void)
> > +{
> > +	struct task_struct *thread;
> > +
> > +	thread = kthread_run(printk_kthread_func, NULL, "printk");
> > +	if (IS_ERR(thread)) {
> > +		pr_err("printk: unable to create printing thread\n");
> > +		return PTR_ERR(thread);
> > +	}
> > +
> > +	printk_kthread = thread;
> > +	return 0;
> > +}
> > +late_initcall(init_printk_kthread);
> 
> I like the simplicity. I just wonder if people on tiny devices might
> want to disable it. In each case, it does not make sense on non-SMP
> machines or when people force the emergency mode all the time.
> 
> I am not sure what is the practice here. I wonder if we should be
> proactive or keep it as is and wait until anyone complains. IMHO,
> it is not that big deal but...

I tend to agree that this is not a big deal, as of now.
I've bigger concerns.

	-ss

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


#1611740 — [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places

FromSergey Senozhatsky <sergey.senozhatsky@gmail.com>
Date2017-03-29 11:40 +0200
Subject[RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places
Message-ID<tqif8-4VF-35@gated-at.bofh.it>
In reply to#1611720
It's not always possible/safe to wake_up() printk kernel
thread. For example, late suspend/early resume may printk()
while timekeeping is not initialized yet, so calling into the
scheduler may result in recursive warnings.

Another thing to notice is the fact PM at some point
freezes user space and kernel threads: freeze_processes()
and freeze_kernel_threads(), correspondingly. Thus we need
printk() to operate in old mode there and attempt to
immediately flush pending kernel message to the console.

This patch adds printk_emergency_begin/on sections.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 kernel/power/hibernate.c | 8 ++++++++
 kernel/power/suspend.c   | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index a8b978c35a6a..a8c6bb2dbcef 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -502,6 +502,7 @@ int hibernation_restore(int platform_mode)
 {
 	int error;
 
+	printk_emergency_begin();
 	pm_prepare_console();
 	suspend_console();
 	pm_restrict_gfp_mask();
@@ -519,6 +520,7 @@ int hibernation_restore(int platform_mode)
 	pm_restore_gfp_mask();
 	resume_console();
 	pm_restore_console();
+	printk_emergency_end();
 	return error;
 }
 
@@ -542,6 +544,7 @@ int hibernation_platform_enter(void)
 		goto Close;
 
 	entering_platform_hibernation = true;
+	printk_emergency_begin();
 	suspend_console();
 	error = dpm_suspend_start(PMSG_HIBERNATE);
 	if (error) {
@@ -589,6 +592,7 @@ int hibernation_platform_enter(void)
 	entering_platform_hibernation = false;
 	dpm_resume_end(PMSG_RESTORE);
 	resume_console();
+	printk_emergency_end();
 
  Close:
 	hibernation_ops->end();
@@ -692,6 +696,7 @@ int hibernate(void)
 		goto Unlock;
 	}
 
+	printk_emergency_begin();
 	pm_prepare_console();
 	error = __pm_notifier_call_chain(PM_HIBERNATION_PREPARE, -1, &nr_calls);
 	if (error) {
@@ -759,6 +764,7 @@ int hibernate(void)
  Exit:
 	__pm_notifier_call_chain(PM_POST_HIBERNATION, nr_calls, NULL);
 	pm_restore_console();
+	printk_emergency_end();
 	atomic_inc(&snapshot_device_available);
  Unlock:
 	unlock_system_sleep();
@@ -868,6 +874,7 @@ static int software_resume(void)
 		goto Unlock;
 	}
 
+	printk_emergency_begin();
 	pm_prepare_console();
 	error = __pm_notifier_call_chain(PM_RESTORE_PREPARE, -1, &nr_calls);
 	if (error) {
@@ -884,6 +891,7 @@ static int software_resume(void)
  Finish:
 	__pm_notifier_call_chain(PM_POST_RESTORE, nr_calls, NULL);
 	pm_restore_console();
+	printk_emergency_end();
 	atomic_inc(&snapshot_device_available);
 	/* For success case, the suspend path will release the lock */
  Unlock:
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 15e6baef5c73..1f897b149fc0 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -433,6 +433,7 @@ int suspend_devices_and_enter(suspend_state_t state)
 	if (!sleep_state_supported(state))
 		return -ENOSYS;
 
+	printk_emergency_begin();
 	error = platform_suspend_begin(state);
 	if (error)
 		goto Close;
@@ -462,6 +463,7 @@ int suspend_devices_and_enter(suspend_state_t state)
 
  Close:
 	platform_resume_end(state);
+	printk_emergency_end();
 	return error;
 
  Recover_platform:
@@ -520,6 +522,7 @@ static int enter_state(suspend_state_t state)
 #endif
 
 	pr_debug("PM: Preparing system for sleep (%s)\n", pm_states[state]);
+	printk_emergency_begin();
 	pm_suspend_clear_flags();
 	error = suspend_prepare(state);
 	if (error)
@@ -537,6 +540,7 @@ static int enter_state(suspend_state_t state)
  Finish:
 	pr_debug("PM: Finishing wakeup.\n");
 	suspend_finish();
+	printk_emergency_end();
  Unlock:
 	mutex_unlock(&pm_mutex);
 	return error;
-- 
2.12.2

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


#1614094 — Re: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places

FromPetr Mladek <pmladek@suse.com>
Date2017-03-31 17:10 +0200
SubjectRe: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places
Message-ID<tr6lA-6eB-21@gated-at.bofh.it>
In reply to#1611740
On Wed 2017-03-29 18:25:07, Sergey Senozhatsky wrote:
> It's not always possible/safe to wake_up() printk kernel
> thread. For example, late suspend/early resume may printk()
> while timekeeping is not initialized yet, so calling into the
> scheduler may result in recursive warnings.
> 
> Another thing to notice is the fact PM at some point
> freezes user space and kernel threads: freeze_processes()
> and freeze_kernel_threads(), correspondingly. Thus we need
> printk() to operate in old mode there and attempt to
> immediately flush pending kernel message to the console.
> 
> This patch adds printk_emergency_begin/on sections.
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

It looks reasonable to me. Feel free to use:

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

Well, it still would be great if people more familiar
with this code look at it.

Best Regards,
Petr

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


#1618216 — Re: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places

FromPavel Machek <pavel@ucw.cz>
Date2017-04-06 19:30 +0200
SubjectRe: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places
Message-ID<ttjom-3R8-25@gated-at.bofh.it>
In reply to#1611740

[Multipart message — attachments visible in raw view] — view raw

On Wed 2017-03-29 18:25:07, Sergey Senozhatsky wrote:
> It's not always possible/safe to wake_up() printk kernel
> thread. For example, late suspend/early resume may printk()
> while timekeeping is not initialized yet, so calling into the
> scheduler may result in recursive warnings.
> 
> Another thing to notice is the fact PM at some point
> freezes user space and kernel threads: freeze_processes()
> and freeze_kernel_threads(), correspondingly. Thus we need
> printk() to operate in old mode there and attempt to
> immediately flush pending kernel message to the console.
> 
> This patch adds printk_emergency_begin/on sections.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

I don't like this. It is symptom of printk getting much more fragile
now.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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


#1619479 — Re: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places

FromAndreas Mohr <andi@lisas.de>
Date2017-04-09 13:00 +0200
SubjectRe: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places
Message-ID<tuiJA-1J6-5@gated-at.bofh.it>
In reply to#1618216
On Thu, Apr 06, 2017 at 07:20:52PM +0200, Pavel Machek wrote:
> On Wed 2017-03-29 18:25:07, Sergey Senozhatsky wrote:
> > It's not always possible/safe to wake_up() printk kernel
> > thread. For example, late suspend/early resume may printk()
> > while timekeeping is not initialized yet, so calling into the
> > scheduler may result in recursive warnings.
> > 
> > Another thing to notice is the fact PM at some point
> > freezes user space and kernel threads: freeze_processes()
> > and freeze_kernel_threads(), correspondingly. Thus we need
> > printk() to operate in old mode there and attempt to
> > immediately flush pending kernel message to the console.
> > 
> > This patch adds printk_emergency_begin/on sections.
> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> 
> I don't like this. It is symptom of printk getting much more fragile
> now.


Sergey has mentioned it already:
"at some point freezes user space and kernel threads".
Well, this is the action which is *itself* causing thoroughly disrupting consequences,
which I'd think thus ought to be responsible to
ensure *itself* that all resulting consequences actually can be dealt with properly,
rather than having
weird *completely-unrelated-dependency* crap
("there happens to be some functionality called printk, and we need to bend it,
since we need to bend it, since otherwise it would not be bent" - ahem...)
leak into ("layer violation" keyword)
pm handling implementation specifics.
IOW, I would think that for any relevant kthread use in API user code,
such code ought to be able to
register kthread-API-provided callbacks (observer pattern, or whatever)
where the (back to current case:) printk kthread would then be able to
*implicitly*/*invisibly* switch the entire printk operation interface
(e.g. via a global interface struct) to
the "dumb"/"safe" fallback variant.
Potential interface: kthread_notify(callback_func, kthread_notification_type);

That way it could (hopefully) be ensured that
people could use a consistent "printk" *interface* universally regardless of which
"special" conditions happen to be in place at the moment.
(IOW, keep interface behaviour which is required/expected at user code
definitely isolated from
awkward "implementation aspects" necessity which is currently poisoning user code implementation).


Put differently,
handling preferrably ought to get consistently adapted (i.e., switched) *centrally*,
rather than
requiring weird helpers (printk_emergency_X()) at all user code sites.

...or so goes the theory.
(quite possibly such thoughts may hit roadblocks e.g. due to locking/atomicity issues)

HTH,

Andreas Mohr

-- 
GNU/Linux. It's not the software that's free, it's you.

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


#1619872 — Re: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places

FromPetr Mladek <pmladek@suse.com>
Date2017-04-10 14:30 +0200
SubjectRe: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places
Message-ID<tuGCd-Cp-1@gated-at.bofh.it>
In reply to#1619479
On Sun 2017-04-09 12:59:18, Andreas Mohr wrote:
> On Thu, Apr 06, 2017 at 07:20:52PM +0200, Pavel Machek wrote:
> > On Wed 2017-03-29 18:25:07, Sergey Senozhatsky wrote:
> > > It's not always possible/safe to wake_up() printk kernel
> > > thread. For example, late suspend/early resume may printk()
> > > while timekeeping is not initialized yet, so calling into the
> > > scheduler may result in recursive warnings.
> > > 
> > > Another thing to notice is the fact PM at some point
> > > freezes user space and kernel threads: freeze_processes()
> > > and freeze_kernel_threads(), correspondingly. Thus we need
> > > printk() to operate in old mode there and attempt to
> > > immediately flush pending kernel message to the console.
> > > 
> Sergey has mentioned it already:
> "at some point freezes user space and kernel threads".
> Well, this is the action which is *itself* causing thoroughly disrupting consequences,
> which I'd think thus ought to be responsible to
> ensure *itself* that all resulting consequences actually can be dealt with properly,
> rather than having
> weird *completely-unrelated-dependency* crap
> ("there happens to be some functionality called printk, and we need to bend it,
> since we need to bend it, since otherwise it would not be bent" - ahem...)
> leak into ("layer violation" keyword)
> pm handling implementation specifics.
> IOW, I would think that for any relevant kthread use in API user code,
> such code ought to be able to
> register kthread-API-provided callbacks (observer pattern, or whatever)
> where the (back to current case:) printk kthread would then be able to
> *implicitly*/*invisibly* switch the entire printk operation interface
> (e.g. via a global interface struct) to
> the "dumb"/"safe" fallback variant.
> Potential interface: kthread_notify(callback_func, kthread_notification_type);

Interesting idea. The power management area probably can be solved
by the existing notifiers framework, see register_pm_notifier().

I haven't checked it but if the notifiers are called on right
locations, it would be cleaner than adding the calls into
the pm code.


> That way it could (hopefully) be ensured that
> people could use a consistent "printk" *interface* universally regardless of which
> "special" conditions happen to be in place at the moment.
> (IOW, keep interface behaviour which is required/expected at user code
> definitely isolated from
> awkward "implementation aspects" necessity which is currently poisoning user code implementation).

> Put differently,
> handling preferrably ought to get consistently adapted (i.e., switched) *centrally*,
> rather than
> requiring weird helpers (printk_emergency_X()) at all user code sites.

Note that there already all many printk/console related "hacks"
in sensitive code paths. For example, see the use of
pm_prepare_console(), suspend_console(), console_level.

Best Regards,
Petr

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


#1619982 — Re: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places

FromSergey Senozhatsky <sergey.senozhatsky@gmail.com>
Date2017-04-10 16:40 +0200
SubjectRe: [RFC][PATCHv2 4/8] pm: switch to printk.emergency mode in unsafe places
Message-ID<tuIE3-1UB-29@gated-at.bofh.it>
In reply to#1619872
On (04/10/17 14:20), Petr Mladek wrote:
[..]
> > Sergey has mentioned it already:
> > "at some point freezes user space and kernel threads".
> > Well, this is the action which is *itself* causing thoroughly disrupting consequences,
> > which I'd think thus ought to be responsible to
> > ensure *itself* that all resulting consequences actually can be dealt with properly,
> > rather than having
> > weird *completely-unrelated-dependency* crap
> > ("there happens to be some functionality called printk, and we need to bend it,
> > since we need to bend it, since otherwise it would not be bent" - ahem...)
> > leak into ("layer violation" keyword)
> > pm handling implementation specifics.
> > IOW, I would think that for any relevant kthread use in API user code,
> > such code ought to be able to
> > register kthread-API-provided callbacks (observer pattern, or whatever)
> > where the (back to current case:) printk kthread would then be able to
> > *implicitly*/*invisibly* switch the entire printk operation interface
> > (e.g. via a global interface struct) to
> > the "dumb"/"safe" fallback variant.
> > Potential interface: kthread_notify(callback_func, kthread_notification_type);
> 
> Interesting idea. The power management area probably can be solved
> by the existing notifiers framework.

good idea indeed.

wish we also had kexec and sysrq notifiers :) there is a
`panic_notifier_list', but that's not exactly what we need.

[..]
> > Put differently,
> > handling preferrably ought to get consistently adapted (i.e., switched) *centrally*,
> > rather than
> > requiring weird helpers (printk_emergency_X()) at all user code sites.
> 
> Note that there already all many printk/console related "hacks"
> in sensitive code paths. For example, see the use of
> pm_prepare_console(), suspend_console(), console_level.

yep. I wonder if some of those can be moved to printk pm notifiers.
but that's out of the scope of this patch set.

	-ss

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


#1611741 — [RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter

FromSergey Senozhatsky <sergey.senozhatsky@gmail.com>
Date2017-03-29 11:40 +0200
Subject[RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter
Message-ID<tqif8-4VF-39@gated-at.bofh.it>
In reply to#1611720
This param permits user-space to forcibly on/off printk emergency
mode via /sys/module/printk/parameters/emergency_mode node.

We have annotated sections in the kernel that switch printk to
emergency, but there might be places/cases when user space would
want to have printk operate in emergency mode all the time.

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

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 1927b5cb5cbe..0d96839bb450 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -455,7 +455,7 @@ static struct task_struct *printk_kthread __read_mostly;
 static atomic_t printk_emergency __read_mostly;
 /*
  * Disable printk_kthread permanently. Unlike `oops_in_progress'
- * it doesn't go back to 0.
+ * it doesn't go back to 0 (unless enforced by user-space).
  */
 static bool printk_kthread_disabled __read_mostly;
 
@@ -483,6 +483,24 @@ void printk_emergency_end(void)
 	atomic_dec(&printk_emergency);
 }
 
+static int printk_kthread_disabled_set(const char *val,
+					const struct kernel_param *kp)
+{
+	return param_set_bool(val, kp);
+}
+
+static const struct kernel_param_ops printk_kthread_disabled_ops = {
+	.set = printk_kthread_disabled_set,
+	.get = param_get_bool,
+};
+
+module_param_cb(emergency_mode,
+		&printk_kthread_disabled_ops,
+		&printk_kthread_disabled,
+		0644);
+MODULE_PARM_DESC(emergency_mode,
+		"don't offload message printing to printk kthread");
+
 /* Return log buffer address */
 char *log_buf_addr_get(void)
 {
-- 
2.12.2

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


#1615320 — Re: [RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter

FromPetr Mladek <pmladek@suse.com>
Date2017-04-03 17:30 +0200
SubjectRe: [RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter
Message-ID<tsc5z-cE-1@gated-at.bofh.it>
In reply to#1611741
On Wed 2017-03-29 18:25:10, Sergey Senozhatsky wrote:
> This param permits user-space to forcibly on/off printk emergency
> mode via /sys/module/printk/parameters/emergency_mode node.
> 
> We have annotated sections in the kernel that switch printk to
> emergency, but there might be places/cases when user space would
> want to have printk operate in emergency mode all the time.

Thanks a lot for the parameter.
 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  kernel/printk/printk.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 1927b5cb5cbe..0d96839bb450 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -455,7 +455,7 @@ static struct task_struct *printk_kthread __read_mostly;
>  static atomic_t printk_emergency __read_mostly;
>  /*
>   * Disable printk_kthread permanently. Unlike `oops_in_progress'
> - * it doesn't go back to 0.
> + * it doesn't go back to 0 (unless enforced by user-space).
>   */
>  static bool printk_kthread_disabled __read_mostly;
>  
> @@ -483,6 +483,24 @@ void printk_emergency_end(void)
>  	atomic_dec(&printk_emergency);
>  }
>  
> +static int printk_kthread_disabled_set(const char *val,
> +					const struct kernel_param *kp)
> +{
> +	return param_set_bool(val, kp);
> +}
> +
> +static const struct kernel_param_ops printk_kthread_disabled_ops = {
> +	.set = printk_kthread_disabled_set,
> +	.get = param_get_bool,
> +};
> +
> +module_param_cb(emergency_mode,
> +		&printk_kthread_disabled_ops,
> +		&printk_kthread_disabled,
> +		0644);
> +MODULE_PARM_DESC(emergency_mode,
> +		"don't offload message printing to printk kthread");

I wonder if we could make this easier. Something like:

static bool printk_force_emergency;
module_param_named(force_emergency, printk_force_emergency,
		   bool, S_IRUGO | S_IWUSR);

and use it instead of printk_kthread_disabled variable. It was
confusing anyway. You already mentioned that it did not
stop the kthread.

Also the relation between the sysfs entry and printk code
will be cleaner. People might thing that emergency_mode
shows the immediate value of printk_emergency variable.

Best Regards,
Petr

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


#1615778 — Re: [RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2017-04-04 10:30 +0200
SubjectRe: [RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter
Message-ID<tss0G-2lg-9@gated-at.bofh.it>
In reply to#1615320
On (04/03/17 17:29), Petr Mladek wrote:
[..]
> > +module_param_cb(emergency_mode,
> > +		&printk_kthread_disabled_ops,
> > +		&printk_kthread_disabled,
> > +		0644);
> > +MODULE_PARM_DESC(emergency_mode,
> > +		"don't offload message printing to printk kthread");
> 
> I wonder if we could make this easier. Something like:
> 
> static bool printk_force_emergency;
> module_param_named(force_emergency, printk_force_emergency,
> 		   bool, S_IRUGO | S_IWUSR);

yes, can do. thanks.

> and use it instead of printk_kthread_disabled variable. It was
> confusing anyway. You already mentioned that it did not
> stop the kthread.

yeah, I didn't like the `printk_kthread_disabled' naming, but
at the same time didn't feel like having `printk_emergency' and
`printk_forced_emergency'. will take a look.

	-ss

[toc] | [prev] | [standalone]


Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel


csiph-web