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

[Michal Hocko <mhocko@kernel.org>]

On Wed 21-03-18 21:20:12, Tetsuo Handa wrote:
> 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?"

This is just a waste of time. I am off from this thread.

My nack still holds and you should seriously reconsider the way you take
the review feedback.

-- 
Michal Hocko
SUSE Labs