linux-kernel - Re: [PATCH] mm: don't warn about allocations which stall for too long

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171101133845.GF20040@pathway.suse.cz>
Date:   Wed, 1 Nov 2017 14:38:45 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Vlastimil Babka <vbabka@...e.cz>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
        akpm@...ux-foundation.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Cong Wang <xiyou.wangcong@...il.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Mel Gorman <mgorman@...e.de>, Michal Hocko <mhocko@...nel.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        "yuwang.yuwang" <yuwang.yuwang@...baba-inc.com>
Subject: Re: [PATCH] mm: don't warn about allocations which stall for too long

On Wed 2017-11-01 09:30:05, Vlastimil Babka wrote:
> On 10/31/2017 08:32 PM, Steven Rostedt wrote:
> > 
> > Thank you for the perfect timing. You posted this the day after I
> > proposed a new solution at Kernel Summit in Prague for the printk lock
> > loop that you experienced here.
> > 
> > I attached the pdf that I used for that discussion (ignore the last
> > slide, it was left over and I never went there).
> > 
> > My proposal is to do something like this with printk:
> > 
> > Three types of printk usages:
> > 
> > 1) Active printer (actively writing to the console).
> > 2) Waiter (active printer, first user)
> > 3) Sees active printer and a waiter, and just adds to the log buffer
> >    and leaves.
> > 
> > (new globals)
> > static DEFINE_SPIN_LOCK(console_owner_lock);
> > static struct task_struct console_owner;
> > static bool waiter;
> > 
> > console_unlock() {
> > 
> > [ Assumes this part can not preempt ]
> > 
> > 	spin_lock(console_owner_lock);
> > 	console_owner = current;
> > 	spin_unlock(console_owner_lock);
> > 
> > 	for each message
> > 		write message out to console
> > 
> > 		if (READ_ONCE(waiter))
> > 			break;
> 
> Ah, these two lines clarified for me what I didn't get from your talk,
> so I got the wrong impression that the new scheme is just postponing the
> problem.
> 
> But still, it seems to me that the scheme only works as long as there
> are printk()'s coming with some reasonable frequency. There's still a
> corner case when a storm of printk()'s can come that will fill the ring
> buffers, and while during the storm the printing will be distributed
> between CPUs nicely, the last unfortunate CPU after the storm subsides
> will be left with a large accumulated buffer to print, and there will be
> no waiters to take over if there are no more printk()'s coming. What
> then, should it detect such situation and defer the flushing?

This was my fear as well. Steven argued that this was theoretical.
And I do not have a real-life bullets against this argument at
the moment.

My current main worry with Steven's approach is a risk of deadlocks
that Jan Kara saw when he played with similar solution.

Also I am afraid that it would add yet another twist to the console
locking operations. It is already quite hard to follow the logic,
see the games with:

	+ console_locked
	+ console_suspended
	+ can_use_console()
	+ exclusive_console

And Steven is going to add:

	+ console_owner
	+ waiter

But let's wait for the patch. It might look and work nicely
in the end.

Best Regards,
Petr