From: Vladimir Davydov <vdavydov@virtuozzo.com>
To: David Rientjes <rientjes@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Michal Hocko <mhocko@kernel.org>,
Johannes Weiner <hannes@cmpxchg.org>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
<linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] mm: oom: deduplicate victim selection code for memcg and global oom
Date: Tue, 28 Jun 2016 19:16:42 +0300 [thread overview]
Message-ID: <20160628161642.GA30658@esperanza> (raw)
In-Reply-To: <alpine.DEB.2.10.1606271713320.81440@chino.kir.corp.google.com>
On Mon, Jun 27, 2016 at 05:14:31PM -0700, David Rientjes wrote:
> On Mon, 27 Jun 2016, Vladimir Davydov wrote:
>
> > When selecting an oom victim, we use the same heuristic for both memory
> > cgroup and global oom. The only difference is the scope of tasks to
> > select the victim from. So we could just export an iterator over all
> > memcg tasks and keep all oom related logic in oom_kill.c, but instead we
> > duplicate pieces of it in memcontrol.c reusing some initially private
> > functions of oom_kill.c in order to not duplicate all of it. That looks
> > ugly and error prone, because any modification of select_bad_process
> > should also be propagated to mem_cgroup_out_of_memory.
> >
> > Let's rework this as follows: keep all oom heuristic related code
> > private to oom_kill.c and make oom_kill.c use exported memcg functions
> > when it's really necessary (like in case of iterating over memcg tasks).
> >
>
> I don't know how others feel, but this actually turns out harder to read
> for me with all the extra redirection with minimal savings (a few dozen
> lines of code).
Well, if you guys find the code difficult to read after this patch,
let's leave it as is. Sorry for the noise.
next prev parent reply other threads:[~2016-06-28 16:33 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-27 16:39 [PATCH v2] mm: oom: deduplicate victim selection code for memcg and global oom Vladimir Davydov
2016-06-28 0:14 ` David Rientjes
2016-06-28 16:16 ` Vladimir Davydov [this message]
2016-07-01 11:18 ` Michal Hocko
2016-07-21 12:41 ` Johannes Weiner
2016-07-23 14:49 ` Vladimir Davydov
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=20160628161642.GA30658@esperanza \
--to=vdavydov@virtuozzo.com \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=rientjes@google.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