From: Michal Hocko <mhocko@kernel.org>
To: Tejun Heo <tj@kernel.org>
Cc: Petr Mladek <pmladek@suse.com>,
cgroups@vger.kernel.org, Cyril Hrubis <chrubis@suse.cz>,
linux-kernel@vger.kernel.org,
Johannes Weiner <hannes@cmpxchg.org>
Subject: Re: [BUG] cgroup/workques/fork: deadlock when moving cgroups
Date: Wed, 13 Apr 2016 21:23:14 +0200 [thread overview]
Message-ID: <20160413192313.GA30260@dhcp22.suse.cz> (raw)
In-Reply-To: <20160413183309.GG3676@htj.duckdns.org>
On Wed 13-04-16 14:33:09, Tejun Heo wrote:
> Hello, Petr.
>
> (cc'ing Johannes)
>
> On Wed, Apr 13, 2016 at 11:42:16AM +0200, Petr Mladek wrote:
> ...
> > By other words, "memcg_move_char/2860" flushes a work. But it cannot
> > get flushed because one worker is blocked and another one could not
> > get created. All these operations are blocked by the very same
> > "memcg_move_char/2860".
> >
> > Note that also "systemd/1" is waiting for "cgroup_mutex" in
> > proc_cgroup_show(). But it seems that it is not in the main
> > cycle causing the deadlock.
> >
> > I am able to reproduce this problem quite easily (within few minutes).
> > There are often even more tasks waiting for the cgroups-related locks
> > but they are not causing the deadlock.
> >
> >
> > The question is how to solve this problem. I see several possibilities:
> >
> > + avoid using workqueues in lru_add_drain_all()
> >
> > + make lru_add_drain_all() killable and restartable
> >
> > + do not block fork() when lru_add_drain_all() is running,
> > e.g. using some lazy techniques like RCU, workqueues
> >
> > + at least do not block fork of workers; AFAIK, they have a limited
> > cgroups usage anyway because they are marked with PF_NO_SETAFFINITY
> >
> >
> > I am willing to test any potential fix or even work on the fix.
> > But I do not have that big insight into the problem, so I would
> > need some pointers.
>
> An easy solution would be to make lru_add_drain_all() use a
> WQ_MEM_RECLAIM workqueue.
I think we can live without lru_add_drain_all() in the migration path.
We are talking about 4 pagevecs so 56 pages. The charge migration is
racy anyway. What concerns me more is how all this is fragile. It sounds
just too easy to add a dependency on per-cpu sync work later and
reintroduce this issue which is quite hard to detect.
Cannot we come up with something more robust? Or at least warn when we
try to use per-cpu workers with problematic locks held?
Thanks!
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2016-04-13 19:23 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-13 9:42 Petr Mladek
2016-04-13 18:33 ` Tejun Heo
2016-04-13 18:57 ` Tejun Heo
2016-04-13 19:23 ` Michal Hocko [this message]
2016-04-13 19:28 ` Michal Hocko
2016-04-13 19:37 ` Tejun Heo
2016-04-13 19:48 ` Michal Hocko
2016-04-14 7:06 ` Michal Hocko
2016-04-14 15:32 ` Tejun Heo
2016-04-14 17:50 ` Johannes Weiner
2016-04-15 7:06 ` Michal Hocko
2016-04-15 14:38 ` Tejun Heo
2016-04-15 15:08 ` Michal Hocko
2016-04-15 15:25 ` Tejun Heo
2016-04-17 12:00 ` Michal Hocko
2016-04-18 14:40 ` Petr Mladek
2016-04-19 14:01 ` Michal Hocko
2016-04-19 15:39 ` Petr Mladek
2016-04-15 19:17 ` [PATCH for-4.6-fixes] memcg: remove lru_add_drain_all() invocation from mem_cgroup_move_charge() Tejun Heo
2016-04-17 12:07 ` Michal Hocko
2016-04-20 21:29 ` Tejun Heo
2016-04-21 3:27 ` Michal Hocko
2016-04-21 15:00 ` Petr Mladek
2016-04-21 15:51 ` Tejun Heo
2016-04-21 23:06 ` [PATCH 1/2] cgroup, cpuset: replace cpuset_post_attach_flush() with cgroup_subsys->post_attach callback Tejun Heo
2016-04-21 23:09 ` [PATCH 2/2] memcg: relocate charge moving from ->attach to ->post_attach Tejun Heo
2016-04-22 13:57 ` Petr Mladek
2016-04-25 8:25 ` Michal Hocko
2016-04-25 19:42 ` Tejun Heo
2016-04-25 19:44 ` Tejun Heo
2016-04-21 23:11 ` [PATCH 1/2] cgroup, cpuset: replace cpuset_post_attach_flush() with cgroup_subsys->post_attach callback Tejun Heo
2016-04-21 15:56 ` [PATCH for-4.6-fixes] memcg: remove lru_add_drain_all() invocation from mem_cgroup_move_charge() 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=20160413192313.GA30260@dhcp22.suse.cz \
--to=mhocko@kernel.org \
--cc=cgroups@vger.kernel.org \
--cc=chrubis@suse.cz \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pmladek@suse.com \
--cc=tj@kernel.org \
/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
all inboxes | Powered by JetHome®