Re: [PATCH v3] mm,page_alloc: wait for oom_lock than back off

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

Michal Hocko wrote:
> On Wed 21-03-18 20:35:47, Tetsuo Handa wrote:
> > Michal Hocko wrote:
> > > On Wed 21-03-18 19:39:32, Tetsuo Handa wrote:
> > > > Tetsuo Handa wrote:
> > > > > Michal Hocko wrote:
> > > > > > > But since Michal is still worrying that adding a single synchronization
> > > > > > > point into the OOM path is risky (without showing a real life example
> > > > > > > where lock_killable() in the coldest OOM path hurts), changes made by
> > > > > > > this patch will be enabled only when oom_compat_mode=0 kernel command line
> > > > > > > parameter is specified so that users can test whether their workloads get
> > > > > > > hurt by this patch.
> > > > > > > 
> > > > > > Nacked with passion. This is absolutely hideous. First of all there is
> > > > > > absolutely no need for the kernel command line. That is just trying to
> > > > > > dance around the fact that you are not able to argue for the change
> > > > > > and bring reasonable arguments on the table. We definitely do not want
> > > > > > two subtly different modes for the oom handling. Secondly, and repeatedly,
> > > > > > you are squashing multiple changes into a single patch. And finally this
> > > > > > is too big of a hammer for something that even doesn't solve the problem
> > > > > > for PREEMPTIVE kernels which are free to schedule regardless of the
> > > > > > sleep or the reclaim retry you are so passion about.
> > > > > 
> > > > > So, where is your version? Offload to a kernel thread like the OOM reaper?
> > > > > Get rid of oom_lock? Just rejecting my proposal makes no progress.
> > > > > 
> > > > Did you come up with some idea?
> > > > Even CONFIG_PREEMPT=y, as far as I tested, v2 patch significantly reduces stalls than now.
> > > > I believe there is no valid reason not to test my v2 patch at linux-next.
> > > 
> > > There are and I've mentioned them in my review feedback.
> > > 
> > Where? When I tried to disable preemption while oom_lock is held,
> > you suggested not to disable preemption. Thus, I followed your feedback.
> > Now, you again complain about preemption.
> > 
> > When I tried to replace only mutex_trylock() with mutex_lock_killable() in v1,
> > you said we need followup changes. Thus, I added followup changes in v2.
> > 
> > What are still missing? I can't understand what you are saying.
> 
> http://lkml.kernel.org/r/20180302141000.GB12772@dhcp22.suse.cz
> 
> There are several points I really disliked. Ignoring them is not going
> to move this work forward.

"And finally this is too big of a hammer for something that even doesn't solve
the problem for PREEMPTIVE kernels which are free to schedule regardless of the
sleep or the reclaim retry you are so passion about." is not a problem, for
preemption is there in the hope that preemption allows processes to do something
useful. But current code allows processes to simply waste CPU resources by
pointless direct reclaim and prevents the owner of oom_lock from making progress
(i.e. AB-BA deadlock).

"Secondly, and repeatedly, you are squashing multiple changes into a single
patch." is a result of your feedback "This is not a solution without further
steps." While v2 patch will need to be split into multiple patches when merging,
you should give feedback based on what changes are missing. Doing multiple changes
into a single patch can be a reason not to merge but can not be a reason not to
test these changes.

"We definitely do not want two subtly different modes for the oom handling." is
not there in v2 patch.

If you say "you are not able to argue for the change and bring reasonable arguments
on the table.", please show us your arguments which is better than mine. Nothing can
justify current code (i.e. AB-BA deadlock). I'm asking your arguments by
"So, where is your version?"