Re: [PATCH] mm/page_alloc: Wait for oom_lock before retrying.

[Petr Mladek <pmladek@suse.com>]

On Mon 2016-12-26 20:41:06, Sergey Senozhatsky wrote:
> On (12/26/16 19:54), Tetsuo Handa wrote:
> > I tried these 9 patches. Generally OK.
> > 
> > Although there is still "schedule_timeout_killable() lockup with oom_lock held"
> > problem, async-printk patches help avoiding "printk() lockup with oom_lock held"
> > problem. Thank you.
> > 
> > Three comments from me.
> > 
> > (1) Messages from e.g. SysRq-b is not waited for sent to consoles.
> >     "SysRq : Resetting" line is needed as a note that I gave up waiting.
> > 
> > (2) Messages from e.g. SysRq-t should be sent to consoles synchronously?
> >     "echo t > /proc/sysrq-trigger" case can use asynchronous printing.
> >     But since ALT-SysRq-T sequence from keyboard may be used when scheduler
> >     is not responding, it might be better to use synchronous printing.
> >     (Or define a magic key sequence to toggle synchronous/asynchronous?)
> 
> it's really hard to tell if the message comes from sysrq or from
> somewhere else.

Yes, but we have the oposite problem now. We usually do not see any
sysrq message on the console with async printk.

> the current approach -- switch to *always* sync printk
> once we see the first LOGLEVEL_EMERG message. so you can add
> printk(LOGLEVEL_EMERG "sysrq-t\n"); for example, and printk will
> switch to sync mode. sync mode, is might be a bit dangerous though,
> since we printk from IRQ.

Sysrq forces all messages to the console by manipulating the
console_loglevel by purpose, see:

void __handle_sysrq(int key, bool check_mask)
{
	struct sysrq_key_op *op_p;
	int orig_log_level;
	int i;

	rcu_sysrq_start();
	rcu_read_lock();
	/*
	 * Raise the apparent loglevel to maximum so that the sysrq header
	 * is shown to provide the user with positive feedback.  We do not
	 * simply emit this at KERN_EMERG as that would change message
	 * routing in the consumers of /proc/kmsg.
	 */
	orig_log_level = console_loglevel;
	console_loglevel = CONSOLE_LOGLEVEL_DEFAULT;
	pr_info("SysRq : ");

Where the loglevel forcing seems to be already in the initial commit
to git.

The comment explaining why KERN_EMERG is not a good idea was added
by the commit fb144adc517d9ebe8fd ("sysrq: add commentary on why we
use the console loglevel over using KERN_EMERG").

Also it seems that all messages are flushed with disabled interrupts
by purpose. See the commit message for that rcu calls in the commit
722773afd83209d4088d ("sysrq,rcu: suppress RCU stall warnings while
sysrq runs").


Therefore, it would make sense to switch to the synchronous
mode in this section.

The question is if we want to come back to the asynchronous mode
when sysrq is finished. It is not easy to do it race-less. A solution
would be to force synchronous mode via the printk_context per-CPU
variable, similar way like we force printk_safe mode.

Alternatively we could try to flush console before resetting back
the console_loglevel:

	if (console_trylock())
		console_unlock();
	console_loglevel = orig_log_level;


Of course, the best solution would be to store the desired console
level with the message into logbuf. But this is not easy because
we would break ABI for external tools, like crashdump, crash, ...

Best Regards,
Petr

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>