linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: mhocko@suse.com
Cc: hannes@cmpxchg.org, yuwang668899@gmail.com, linux-mm@kvack.org,
	chenggang.qcg@alibaba-inc.com, yuwang.yuwang@alibaba-inc.com,
	akpm@linux-foundation.org
Subject: Re: [PATCH] mm,page_alloc: softlockup on warn_alloc on
Date: Mon, 18 Sep 2017 15:31:31 +0900	[thread overview]
Message-ID: <201709181531.HGI09326.OFQMFOtVHFJSLO@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <20170918060524.sut26yl65j2cf3jk@dhcp22.suse.cz>

Michal Hocko wrote:
> > > The synchronization has worked this way for a long time (trylock
> > > failure assuming progress, but the order/NOFS/zone bailouts from
> > > actually OOM-killing inside the locked section). We should really fix
> > > *that* rather than serializing warn_alloc().
> > > 
> > > For GFP_NOFS, it seems to go back to 9879de7373fc ("mm: page_alloc:
> > > embed OOM killing naturally into allocation slowpath"). Before that we
> > > didn't use to call __alloc_pages_may_oom() for NOFS allocations. So I
> > > still wonder why this only now appears to be causing problems.
> > > 
> > > In any case, converting that trylock to a sleeping lock in this case
> > > makes sense to me. Nobody is blocking under this lock (except that one
> > > schedule_timeout_killable(1) after dispatching a victim) and it's not
> > > obvious to me why we'd need that level of concurrency under OOM.
> > 
> > You can try http://lkml.kernel.org/r/1500202791-5427-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
> > and http://lkml.kernel.org/r/1503577106-9196-2-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp together.
> > Then, we can remove mutex_lock(&oom_lock) serialization from __oom_reap_task_mm()
> > which still exists because Andrea's patch was accepted instead of Michal's patch.
> 
> We can safely drop the oom_lock from __oom_reap_task_mm now. Andrea
> didn't want to do it in his patch because that is a separate thing
> logically. But nothing should prefent the removal now that AFAICS.

No! The oom_lock in __oom_reap_task_mm() is still required due to lack of
really last second allocation attempt. If we do really last second
allocation attempt, we can remove the oom_lock from __oom_reap_task_mm().



Enter __alloc_pages_may_oom()              Enter __oom_reap_task_mm()

  Take oom_lock

  Try last get_page_from_freelist()

                                             No "take oom_lock" here

                                             Reap memory

                                             Set MMF_OOM_SKIP

                                             No "release oom_lock" here

                                           Leave __oom_reap_task_mm()

  Enter out_of_memory()

    Enter select_bad_process()

      Enter oom_evaluate_task()

        Check if MMF_OOM_SKIP is already set

      Leave oom_evaluate_task()

    Leave select_bad_process()

    No "really last get_page_from_freelist()" here

    Kill the next victim needlessly

  Leave out_of_memory()

  Release oom_lock

Leave __alloc_pages_may_oom()

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

  reply	other threads:[~2017-09-18  6:31 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-15  9:58 wang Yu
2017-09-15 10:39 ` Michal Hocko
2017-09-15 11:38   ` Tetsuo Handa
2017-09-15 12:00     ` Michal Hocko
2017-09-15 12:09       ` Tetsuo Handa
2017-09-15 12:14         ` Michal Hocko
2017-09-15 14:12           ` Tetsuo Handa
2017-09-15 14:23             ` Michal Hocko
2017-09-24  1:56             ` Tetsuo Handa
2017-09-15 14:37 ` Johannes Weiner
2017-09-15 15:23   ` Tetsuo Handa
2017-09-15 18:44     ` Johannes Weiner
2017-09-16  0:25       ` Tetsuo Handa
2017-09-18  6:05         ` Michal Hocko
2017-09-18  6:31           ` Tetsuo Handa [this message]
2017-09-18  6:43             ` Michal Hocko
2017-09-16  4:12   ` Tetsuo Handa
2017-10-11 11:14     ` Tetsuo Handa
2017-10-18 10:54       ` Tetsuo Handa
2017-09-18  6:03   ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201709181531.HGI09326.OFQMFOtVHFJSLO@I-love.SAKURA.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=akpm@linux-foundation.org \
    --cc=chenggang.qcg@alibaba-inc.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=yuwang.yuwang@alibaba-inc.com \
    --cc=yuwang668899@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox