Re: [PATCH] mm,page_alloc: Serialize warn_alloc() if schedulable.

[Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>]

Michal Hocko wrote:
> On Sat 08-07-17 13:59:54, Tetsuo Handa wrote:
> [...]
> > Quoting from http://lkml.kernel.org/r/20170705081956.GA14538@dhcp22.suse.cz :
> > Michal Hocko wrote:
> > > On Sat 01-07-17 20:43:56, Tetsuo Handa wrote:
> > > > You are rejecting serialization under OOM without giving a chance to test
> > > > side effects of serialization under OOM at linux-next.git. I call such attitude
> > > > "speculation" which you never accept.
> > > 
> > > No I am rejecting abusing the lock for purpose it is not aimed for.
> > 
> > Then, why adding a new lock (not oom_lock but warn_alloc_lock) is not acceptable?
> > Since warn_alloc_lock is aimed for avoiding messages by warn_alloc() getting
> > jumbled, there should be no reason you reject this lock.
> > 
> > If you don't like locks, can you instead accept below one?
> 
> No, seriously! Just think about what you are proposing. You are stalling
> and now you will stall _random_ tasks even more. Some of them for
> unbound amount of time because of inherent unfairness of cmpxchg.

The cause of stall when oom_lock is already held is that threads which failed to
hold oom_lock continue almost busy looping; schedule_timeout_uninterruptible(1) is
not sufficient when there are multiple threads doing the same thing, for direct
reclaim/compaction consumes a lot of CPU time.

What makes this situation worse is, since warn_alloc() periodically appends to
printk() buffer, the thread inside the OOM killer with oom_lock held can stall
forever due to cond_resched() from console_unlock() from printk().

Below change significantly reduces possibility of falling into printk() v.s. oom_lock
lockup problem, for the thread inside the OOM killer with oom_lock held no longer
blocks inside printk(). Though there still remains possibility of sleeping for
unexpectedly long at schedule_timeout_killable(1) with the oom_lock held.

--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -1051,8 +1051,10 @@ bool out_of_memory(struct oom_control *oc)
 		panic("Out of memory and no killable processes...\n");
 	}
 	if (oc->chosen && oc->chosen != (void *)-1UL) {
+		preempt_disable();
 		oom_kill_process(oc, !is_memcg_oom(oc) ? "Out of memory" :
 				 "Memory cgroup out of memory");
+		preempt_enable_no_resched();
 		/*
 		 * Give the killed process a good chance to exit before trying
 		 * to allocate memory again.

I wish we could agree with applying this patch until printk-kthread can
work reliably...

> 
> If there is a _real_ problem it should be debugged and fixed. If this
> is a limitation of what printk can handle then we should think how to
> throttle it even more (e.g. does it make much sense to dump_stack when
> it hasn't changed since the last time?). If this is about dump_stack
> taking too long then we should look into it but we definitely should add
> a more on top.

The real problem is lack of CPU time for reclaiming memory when allocating
threads failed to hold oom_lock. And you are refusing to allow allocating
threads give CPU time to the thread holding oom_lock.

--
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>