All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Thelen <gthelen@google.com>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@kernel.org>,
	Shakeel Butt <shakeelb@google.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux MM <linux-mm@kvack.org>,
	linux-fsdevel@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] fs, mm: account filp and names caches to kmemcg
Date: Thu, 26 Oct 2017 12:56:48 -0700	[thread overview]
Message-ID: <xr93y3nxbs3j.fsf@gthelen.svl.corp.google.com> (raw)
In-Reply-To: <20171026143140.GB21147@cmpxchg.org>

Michal Hocko wrote:
> Greg Thelen wrote:
> > So a force charge fallback might be a needed even with oom killer successful
> > invocations.  Or we'll need to teach out_of_memory() to return three values
> > (e.g. NO_VICTIM, NEW_VICTIM, PENDING_VICTIM) and try_charge() can loop on
> > NEW_VICTIM.
> 
> No we, really want to wait for the oom victim to do its job. The only thing we
> should be worried about is when out_of_memory doesn't invoke the reaper. There
> is only one case like that AFAIK - GFP_NOFS request. I have to think about
> this case some more. We currently fail in that case the request.

Nod, but I think only wait a short time (more below).  The
schedule_timeout_killable(1) in out_of_memory() seems ok to me.  I don't
think there's a problem overcharging a little bit to expedite oom
killing.

Johannes Weiner wrote:
> True. I was assuming we'd retry MEM_CGROUP_RECLAIM_RETRIES times at a maximum,
> even if the OOM killer indicates a kill has been issued. What you propose
> makes sense too.

Sounds good.

It looks like the oom reaper will wait 1 second
(MAX_OOM_REAP_RETRIES*HZ/10) before giving up and setting MMF_OOM_SKIP,
which enables the oom killer to select another victim.  Repeated
try_charge() => out_of_memory() calls will return true while there's a
pending victim.  After the first call, out_of_memory() doesn't appear to
sleep.  So I assume try_charge() would quickly burn through
MEM_CGROUP_RECLAIM_RETRIES (5) attempts before resorting to
overcharging.  IMO, this is fine because:
1) it's possible the victim wants locks held by try_charge caller.  So
   waiting for the oom reaper to timeout and out_of_memory to select
   additional victims would kill more than required.
2) waiting 1 sec to detect a livelock between try_charge() and pending
   oom victim seems unfortunate.

WARNING: multiple messages have this Message-ID (diff)
From: Greg Thelen <gthelen@google.com>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@kernel.org>,
	Shakeel Butt <shakeelb@google.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux MM <linux-mm@kvack.org>,
	linux-fsdevel@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] fs, mm: account filp and names caches to kmemcg
Date: Thu, 26 Oct 2017 12:56:48 -0700	[thread overview]
Message-ID: <xr93y3nxbs3j.fsf@gthelen.svl.corp.google.com> (raw)
In-Reply-To: <20171026143140.GB21147@cmpxchg.org>

Michal Hocko wrote:
> Greg Thelen wrote:
> > So a force charge fallback might be a needed even with oom killer successful
> > invocations.  Or we'll need to teach out_of_memory() to return three values
> > (e.g. NO_VICTIM, NEW_VICTIM, PENDING_VICTIM) and try_charge() can loop on
> > NEW_VICTIM.
> 
> No we, really want to wait for the oom victim to do its job. The only thing we
> should be worried about is when out_of_memory doesn't invoke the reaper. There
> is only one case like that AFAIK - GFP_NOFS request. I have to think about
> this case some more. We currently fail in that case the request.

Nod, but I think only wait a short time (more below).  The
schedule_timeout_killable(1) in out_of_memory() seems ok to me.  I don't
think there's a problem overcharging a little bit to expedite oom
killing.

Johannes Weiner wrote:
> True. I was assuming we'd retry MEM_CGROUP_RECLAIM_RETRIES times at a maximum,
> even if the OOM killer indicates a kill has been issued. What you propose
> makes sense too.

Sounds good.

It looks like the oom reaper will wait 1 second
(MAX_OOM_REAP_RETRIES*HZ/10) before giving up and setting MMF_OOM_SKIP,
which enables the oom killer to select another victim.  Repeated
try_charge() => out_of_memory() calls will return true while there's a
pending victim.  After the first call, out_of_memory() doesn't appear to
sleep.  So I assume try_charge() would quickly burn through
MEM_CGROUP_RECLAIM_RETRIES (5) attempts before resorting to
overcharging.  IMO, this is fine because:
1) it's possible the victim wants locks held by try_charge caller.  So
   waiting for the oom reaper to timeout and out_of_memory to select
   additional victims would kill more than required.
2) waiting 1 sec to detect a livelock between try_charge() and pending
   oom victim seems unfortunate.

--
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-10-26 19:56 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-05 22:21 [PATCH] fs, mm: account filp and names caches to kmemcg Shakeel Butt
2017-10-05 22:21 ` Shakeel Butt
2017-10-06  7:59 ` Michal Hocko
2017-10-06  7:59   ` Michal Hocko
2017-10-06 19:33   ` Shakeel Butt
2017-10-06 19:33     ` Shakeel Butt
2017-10-09  6:24     ` Michal Hocko
2017-10-09  6:24       ` Michal Hocko
2017-10-09 17:52       ` Greg Thelen
2017-10-09 17:52         ` Greg Thelen
2017-10-09 18:04         ` Michal Hocko
2017-10-09 18:04           ` Michal Hocko
2017-10-09 18:17           ` Michal Hocko
2017-10-09 18:17             ` Michal Hocko
2017-10-10  9:10             ` Michal Hocko
2017-10-10  9:10               ` Michal Hocko
2017-10-10 22:21               ` Shakeel Butt
2017-10-10 22:21                 ` Shakeel Butt
2017-10-11  9:09                 ` Michal Hocko
2017-10-11  9:09                   ` Michal Hocko
2017-10-09 20:26         ` Johannes Weiner
2017-10-09 20:26           ` Johannes Weiner
2017-10-10  9:14           ` Michal Hocko
2017-10-10  9:14             ` Michal Hocko
2017-10-10 14:17             ` Johannes Weiner
2017-10-10 14:17               ` Johannes Weiner
2017-10-10 14:24               ` Michal Hocko
2017-10-10 14:24                 ` Michal Hocko
2017-10-12 19:03                 ` Johannes Weiner
2017-10-12 19:03                   ` Johannes Weiner
2017-10-12 23:57                   ` Greg Thelen
2017-10-12 23:57                     ` Greg Thelen
2017-10-13  6:51                     ` Michal Hocko
2017-10-13  6:51                       ` Michal Hocko
2017-10-13  6:35                   ` Michal Hocko
2017-10-13  6:35                     ` Michal Hocko
2017-10-13  7:00                     ` Michal Hocko
2017-10-13  7:00                       ` Michal Hocko
2017-10-13 15:24                       ` Michal Hocko
2017-10-13 15:24                         ` Michal Hocko
2017-10-24 12:18                         ` Michal Hocko
2017-10-24 12:18                           ` Michal Hocko
2017-10-24 17:54                           ` Johannes Weiner
2017-10-24 17:54                             ` Johannes Weiner
2017-10-24 16:06                         ` Johannes Weiner
2017-10-24 16:06                           ` Johannes Weiner
2017-10-24 16:22                           ` Michal Hocko
2017-10-24 16:22                             ` Michal Hocko
2017-10-24 17:23                             ` Johannes Weiner
2017-10-24 17:23                               ` Johannes Weiner
2017-10-24 17:55                               ` Michal Hocko
2017-10-24 17:55                                 ` Michal Hocko
2017-10-24 18:58                                 ` Johannes Weiner
2017-10-24 18:58                                   ` Johannes Weiner
2017-10-24 20:15                                   ` Michal Hocko
2017-10-24 20:15                                     ` Michal Hocko
2017-10-25  6:51                                     ` Greg Thelen
2017-10-25  6:51                                       ` Greg Thelen
2017-10-25  7:15                                       ` Michal Hocko
2017-10-25  7:15                                         ` Michal Hocko
2017-10-25 13:11                                         ` Johannes Weiner
2017-10-25 13:11                                           ` Johannes Weiner
2017-10-25 14:12                                           ` Michal Hocko
2017-10-25 14:12                                             ` Michal Hocko
2017-10-25 16:44                                             ` Johannes Weiner
2017-10-25 16:44                                               ` Johannes Weiner
2017-10-25 17:29                                               ` Michal Hocko
2017-10-25 17:29                                                 ` Michal Hocko
2017-10-25 18:11                                                 ` Johannes Weiner
2017-10-25 18:11                                                   ` Johannes Weiner
2017-10-25 19:00                                                   ` Michal Hocko
2017-10-25 19:00                                                     ` Michal Hocko
2017-10-25 21:13                                                     ` Johannes Weiner
2017-10-25 21:13                                                       ` Johannes Weiner
2017-10-25 22:49                                                       ` Greg Thelen
2017-10-25 22:49                                                         ` Greg Thelen
2017-10-26  7:49                                                         ` Michal Hocko
2017-10-26  7:49                                                           ` Michal Hocko
2017-10-26 12:45                                                           ` Tetsuo Handa
2017-10-26 12:45                                                             ` Tetsuo Handa
2017-10-26 14:31                                                         ` Johannes Weiner
2017-10-26 14:31                                                           ` Johannes Weiner
2017-10-26 19:56                                                           ` Greg Thelen [this message]
2017-10-26 19:56                                                             ` Greg Thelen
2017-10-27  8:20                                                             ` Michal Hocko
2017-10-27  8:20                                                               ` Michal Hocko
2017-10-27 20:50                                               ` Shakeel Butt
2017-10-27 20:50                                                 ` Shakeel Butt
2017-10-30  8:29                                                 ` Michal Hocko
2017-10-30  8:29                                                   ` Michal Hocko
2017-10-30 19:28                                                   ` Shakeel Butt
2017-10-30 19:28                                                     ` Shakeel Butt
2017-10-31  8:00                                                     ` Michal Hocko
2017-10-31  8:00                                                       ` Michal Hocko
2017-10-31 16:49                                                       ` Johannes Weiner
2017-10-31 16:49                                                         ` Johannes Weiner
2017-10-31 18:50                                                         ` Michal Hocko
2017-10-31 18:50                                                           ` Michal Hocko
2017-10-24 15:45                     ` Johannes Weiner
2017-10-24 15:45                       ` Johannes Weiner
2017-10-24 16:30                       ` Michal Hocko
2017-10-24 16:30                         ` Michal Hocko
2017-10-10 23:32 ` Al Viro
2017-10-10 23:32   ` Al Viro

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=xr93y3nxbs3j.fsf@gthelen.svl.corp.google.com \
    --to=gthelen@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=shakeelb@google.com \
    --cc=vdavydov.dev@gmail.com \
    --cc=viro@zeniv.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.