From: Hugh Dickins <hughd@google.com>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Hugh Dickins <hughd@google.com>, Tejun Heo <tj@kernel.org>,
Filipe Brandenburger <filbranden@google.com>,
Li Zefan <lizefan@huawei.com>,
Andrew Morton <akpm@linux-foundation.org>,
Michal Hocko <mhocko@suse.cz>, Greg Thelen <gthelen@google.com>,
Michel Lespinasse <walken@google.com>,
Markus Blank-Burian <burian@muenster.de>,
Shawn Bohrer <shawn.bohrer@gmail.com>,
cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
Subject: Re: [PATCH] cgroup: use an ordered workqueue for cgroup destruction
Date: Wed, 12 Feb 2014 14:59:44 -0800 (PST) [thread overview]
Message-ID: <alpine.LSU.2.11.1402121417230.5029@eggly.anvils> (raw)
In-Reply-To: <20140207164321.GE6963@cmpxchg.org>
On Fri, 7 Feb 2014, Johannes Weiner wrote:
> On Thu, Feb 06, 2014 at 03:56:01PM -0800, Hugh Dickins wrote:
> > Sometimes the cleanup after memcg hierarchy testing gets stuck in
> > mem_cgroup_reparent_charges(), unable to bring non-kmem usage down to 0.
> >
> > There may turn out to be several causes, but a major cause is this: the
> > workitem to offline parent can get run before workitem to offline child;
> > parent's mem_cgroup_reparent_charges() circles around waiting for the
> > child's pages to be reparented to its lrus, but it's holding cgroup_mutex
> > which prevents the child from reaching its mem_cgroup_reparent_charges().
> >
> > Just use an ordered workqueue for cgroup_destroy_wq.
> >
> > Fixes: e5fca243abae ("cgroup: use a dedicated workqueue for cgroup destruction")
> > Suggested-by: Filipe Brandenburger <filbranden@google.com>
> > Signed-off-by: Hugh Dickins <hughd@google.com>
> > Cc: stable@vger.kernel.org # 3.10+
>
> I think this is a good idea for now and -stable:
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
You might be wondering why this patch didn't reach Linus yet.
It's because more thorough testing, by others here, found that it
wasn't always solving the problem: so I asked Tejun privately to
hold off from sending it in, until we'd worked out why not.
Most of our testing being on a v3,11-based kernel, it was perfectly
possible that the problem was merely our own e.g. missing Tejun's
8a2b75384444 ("workqueue: fix ordered workqueues in NUMA setups").
But that turned out not to be enough to fix it either. Then Filipe
pointed out how percpu_ref_kill_and_confirm() uses call_rcu_sched()
before we ever get to put the offline on to the workqueue: by the
time we get to the workqueue, the ordering has already been lost.
So, thanks for the Acks, but I'm afraid that this ordered workqueue
solution is just not good enough: we should simply forget that patch
and provide a different answer.
So I'm now posting a couple of alternative solutions: 1/2 from Filipe
at the memcg end, and 2/2 from me at the cgroup end. Each of these
has stood up to better testing, so you can choose between them,
or work out a better answer.
(By the way, I have another little pair of memcg/cgroup fixes to post
shortly, nothing to do with these two: it would be less confusing if
I had some third fix to add in there, but sadly not.)
Hugh
--
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>
next prev parent reply other threads:[~2014-02-12 23:00 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-06 23:56 Hugh Dickins
2014-02-07 13:45 ` Michal Hocko
2014-02-07 14:04 ` Tejun Heo
2014-02-07 14:37 ` Michal Hocko
2014-02-07 15:13 ` Tejun Heo
2014-02-07 15:28 ` Michal Hocko
2014-02-07 20:20 ` Hugh Dickins
2014-02-07 20:35 ` Tejun Heo
2014-02-07 21:06 ` Hugh Dickins
2014-02-07 15:21 ` Tejun Heo
2014-02-07 16:43 ` Johannes Weiner
2014-02-10 15:46 ` Michal Hocko
2014-02-12 22:59 ` Hugh Dickins [this message]
2014-02-12 23:06 ` [PATCH 2/2] cgroup: bring back kill_cnt to order css destruction Hugh Dickins
2014-02-13 0:28 ` Tejun Heo
2014-02-13 0:38 ` Hugh Dickins
2014-02-13 0:09 ` [PATCH] Revert "cgroup: use an ordered workqueue for cgroup destruction" Tejun Heo
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=alpine.LSU.2.11.1402121417230.5029@eggly.anvils \
--to=hughd@google.com \
--cc=akpm@linux-foundation.org \
--cc=burian@muenster.de \
--cc=cgroups@vger.kernel.org \
--cc=filbranden@google.com \
--cc=gthelen@google.com \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lizefan@huawei.com \
--cc=mhocko@suse.cz \
--cc=shawn.bohrer@gmail.com \
--cc=tj@kernel.org \
--cc=walken@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