From: Glauber Costa <glommer@parallels.com>
To: Michal Hocko <mhocko@suse.cz>
Cc: cgroups@vger.kernel.org, linux-mm@kvack.org,
kamezawa.hiroyu@jp.fujitsu.com,
Johannes Weiner <hannes@cmpxchg.org>, Tejun Heo <tj@kernel.org>
Subject: Re: [PATCH v2 4/7] memcg: fast hierarchy-aware child test.
Date: Mon, 21 Jan 2013 11:58:49 +0400 [thread overview]
Message-ID: <50FCF539.6070000@parallels.com> (raw)
In-Reply-To: <20130118160610.GI10701@dhcp22.suse.cz>
On 01/18/2013 08:06 PM, Michal Hocko wrote:
>> + /* bounce at first found */
>> > + for_each_mem_cgroup_tree(iter, memcg) {
> This will not work. Consider you will see a !online memcg. What happens?
> mem_cgroup_iter will css_get group that it returns and css_put it when
> it visits another one or finishes the loop. So your poor iter will be
> released before it gets born. Not good.
>
Reading this again, I don't really follow. The iterator is not supposed
to put() anything it hasn't get()'d before, so we will never release the
group. Note that if it ever appears in here, the css refcnt is expected
to be at least 1 already.
The online test relies on the memcg refcnt, not on the css refcnt.
Actually, now that the value setting is all done in css_online, the css
refcnt should be enough to denote if the cgroup already has children,
without a memcg-specific test. The css refcnt is bumped somewhere
between alloc and online. Unless Tejun objects it, I think I will just
get rid of the online test, and rely on the fact that if the iterator
sees any children, we should already online.
--
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:[~2013-01-21 7:58 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-11 9:45 [PATCH v2 0/7] replace cgroup_lock with local memcg lock Glauber Costa
2013-01-11 9:45 ` [PATCH v2 1/7] memcg: prevent changes to move_charge_at_immigrate during task attach Glauber Costa
2013-01-18 14:16 ` Michal Hocko
2013-01-11 9:45 ` [PATCH v2 2/7] memcg: split part of memcg creation to css_online Glauber Costa
2013-01-18 15:25 ` Michal Hocko
2013-01-18 19:28 ` Glauber Costa
2013-01-21 7:33 ` Glauber Costa
2013-01-21 8:38 ` Michal Hocko
2013-01-21 8:42 ` Glauber Costa
2013-01-11 9:45 ` [PATCH v2 3/7] memcg: provide online test for memcg Glauber Costa
2013-01-18 15:37 ` Michal Hocko
2013-01-18 15:56 ` Michal Hocko
2013-01-18 19:42 ` Glauber Costa
2013-01-18 19:43 ` Glauber Costa
2013-01-18 19:41 ` Glauber Costa
2013-01-11 9:45 ` [PATCH v2 4/7] memcg: fast hierarchy-aware child test Glauber Costa
2013-01-18 16:06 ` Michal Hocko
2013-01-21 7:58 ` Glauber Costa [this message]
2013-01-21 8:34 ` Michal Hocko
2013-01-21 8:41 ` Glauber Costa
2013-01-21 9:15 ` Michal Hocko
2013-01-21 9:19 ` Glauber Costa
2013-01-11 9:45 ` [PATCH v2 5/7] May god have mercy on my soul Glauber Costa
2013-01-18 16:07 ` Michal Hocko
2013-01-11 9:45 ` [PATCH v2 6/7] memcg: replace cgroup_lock with memcg specific memcg_lock Glauber Costa
2013-01-18 16:21 ` Michal Hocko
2013-01-11 9:45 ` [PATCH v2 7/7] memcg: increment static branch right after limit set Glauber Costa
2013-01-18 16:23 ` 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=50FCF539.6070000@parallels.com \
--to=glommer@parallels.com \
--cc=cgroups@vger.kernel.org \
--cc=hannes@cmpxchg.org \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.cz \
--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