From: Hugh Dickins <hughd@google.com> To: Michal Hocko <mhocko@suse.cz>, Tejun Heo <tj@kernel.org> Cc: Andrew Morton <akpm@linux-foundation.org>, Johannes Weiner <hannes@cmpxchg.org>, Greg Thelen <gthelen@google.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: [PATCH 2/2] memcg: barriers to see memcgs as fully initialized Date: Wed, 12 Feb 2014 17:29:09 -0800 (PST) [thread overview] Message-ID: <alpine.LSU.2.11.1402121727050.5917@eggly.anvils> (raw) In-Reply-To: <alpine.LSU.2.11.1402121717420.5917@eggly.anvils> Commit d8ad30559715 ("mm/memcg: iteration skip memcgs not yet fully initialized") is not bad, but Greg Thelen asks "Are barriers needed?" Yes, I'm afraid so: this makes it a little heavier than the original, but there's no point in guaranteeing that mem_cgroup_iter() returns only fully initialized memcgs, if we don't guarantee that the initialization is visible. If we move online_css()'s setting CSS_ONLINE after rcu_assign_pointer() (I don't see why not), we can reasonably rely on the smp_wmb() in that. But I can't find a pre-existing barrier at the mem_cgroup_iter() end, so add an smp_rmb() where __mem_cgroup_iter_next() returns non-NULL. Fixes: d8ad30559715 ("mm/memcg: iteration skip memcgs not yet fully initialized") Signed-off-by: Hugh Dickins <hughd@google.com> Cc: stable@vger.kernel.org # 3.12+ --- I'd have been happier not to have to add this patch: maybe you can see a better placement, or a way we can avoid this altogether. kernel/cgroup.c | 8 +++++++- mm/memcontrol.c | 11 +++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) --- 3.14-rc2+/kernel/cgroup.c 2014-02-02 18:49:07.737302111 -0800 +++ linux/kernel/cgroup.c 2014-02-12 11:59:52.804041895 -0800 @@ -4063,9 +4063,15 @@ static int online_css(struct cgroup_subs if (ss->css_online) ret = ss->css_online(css); if (!ret) { - css->flags |= CSS_ONLINE; css->cgroup->nr_css++; rcu_assign_pointer(css->cgroup->subsys[ss->subsys_id], css); + /* + * Set CSS_ONLINE after rcu_assign_pointer(), so that its + * smp_wmb() will guarantee that those seeing CSS_ONLINE + * can see the initialization done in ss->css_online() - if + * they provide an smp_rmb(), as in __mem_cgroup_iter_next(). + */ + css->flags |= CSS_ONLINE; } return ret; } --- 3.14-rc2+/mm/memcontrol.c 2014-02-12 11:55:02.836035004 -0800 +++ linux/mm/memcontrol.c 2014-02-12 11:59:52.804041895 -0800 @@ -1128,9 +1128,16 @@ skip_node: */ if (next_css) { if ((next_css == &root->css) || - ((next_css->flags & CSS_ONLINE) && css_tryget(next_css))) + ((next_css->flags & CSS_ONLINE) && css_tryget(next_css))) { + /* + * Ensure that all memcg initialization, done before + * CSS_ONLINE was set, will be visible to our caller. + * This matches the smp_wmb() in online_css()'s + * rcu_assign_pointer(), before it set CSS_ONLINE. + */ + smp_rmb(); return mem_cgroup_from_css(next_css); - + } prev_css = next_css; goto skip_node; }
WARNING: multiple messages have this Message-ID (diff)
From: Hugh Dickins <hughd@google.com> To: Michal Hocko <mhocko@suse.cz>, Tejun Heo <tj@kernel.org> Cc: Andrew Morton <akpm@linux-foundation.org>, Johannes Weiner <hannes@cmpxchg.org>, Greg Thelen <gthelen@google.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: [PATCH 2/2] memcg: barriers to see memcgs as fully initialized Date: Wed, 12 Feb 2014 17:29:09 -0800 (PST) [thread overview] Message-ID: <alpine.LSU.2.11.1402121727050.5917@eggly.anvils> (raw) In-Reply-To: <alpine.LSU.2.11.1402121717420.5917@eggly.anvils> Commit d8ad30559715 ("mm/memcg: iteration skip memcgs not yet fully initialized") is not bad, but Greg Thelen asks "Are barriers needed?" Yes, I'm afraid so: this makes it a little heavier than the original, but there's no point in guaranteeing that mem_cgroup_iter() returns only fully initialized memcgs, if we don't guarantee that the initialization is visible. If we move online_css()'s setting CSS_ONLINE after rcu_assign_pointer() (I don't see why not), we can reasonably rely on the smp_wmb() in that. But I can't find a pre-existing barrier at the mem_cgroup_iter() end, so add an smp_rmb() where __mem_cgroup_iter_next() returns non-NULL. Fixes: d8ad30559715 ("mm/memcg: iteration skip memcgs not yet fully initialized") Signed-off-by: Hugh Dickins <hughd@google.com> Cc: stable@vger.kernel.org # 3.12+ --- I'd have been happier not to have to add this patch: maybe you can see a better placement, or a way we can avoid this altogether. kernel/cgroup.c | 8 +++++++- mm/memcontrol.c | 11 +++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) --- 3.14-rc2+/kernel/cgroup.c 2014-02-02 18:49:07.737302111 -0800 +++ linux/kernel/cgroup.c 2014-02-12 11:59:52.804041895 -0800 @@ -4063,9 +4063,15 @@ static int online_css(struct cgroup_subs if (ss->css_online) ret = ss->css_online(css); if (!ret) { - css->flags |= CSS_ONLINE; css->cgroup->nr_css++; rcu_assign_pointer(css->cgroup->subsys[ss->subsys_id], css); + /* + * Set CSS_ONLINE after rcu_assign_pointer(), so that its + * smp_wmb() will guarantee that those seeing CSS_ONLINE + * can see the initialization done in ss->css_online() - if + * they provide an smp_rmb(), as in __mem_cgroup_iter_next(). + */ + css->flags |= CSS_ONLINE; } return ret; } --- 3.14-rc2+/mm/memcontrol.c 2014-02-12 11:55:02.836035004 -0800 +++ linux/mm/memcontrol.c 2014-02-12 11:59:52.804041895 -0800 @@ -1128,9 +1128,16 @@ skip_node: */ if (next_css) { if ((next_css == &root->css) || - ((next_css->flags & CSS_ONLINE) && css_tryget(next_css))) + ((next_css->flags & CSS_ONLINE) && css_tryget(next_css))) { + /* + * Ensure that all memcg initialization, done before + * CSS_ONLINE was set, will be visible to our caller. + * This matches the smp_wmb() in online_css()'s + * rcu_assign_pointer(), before it set CSS_ONLINE. + */ + smp_rmb(); return mem_cgroup_from_css(next_css); - + } prev_css = next_css; goto skip_node; } -- 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-13 1:29 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-02-13 1:26 [PATCH 1/2] memcg: fix endless loop in __mem_cgroup_iter_next Hugh Dickins 2014-02-13 1:26 ` Hugh Dickins 2014-02-13 1:29 ` Hugh Dickins [this message] 2014-02-13 1:29 ` [PATCH 2/2] memcg: barriers to see memcgs as fully initialized Hugh Dickins 2014-02-13 14:53 ` Michal Hocko 2014-02-13 14:53 ` Michal Hocko 2014-02-16 2:52 ` Hugh Dickins 2014-02-16 2:52 ` Hugh Dickins 2014-02-13 21:07 ` Tejun Heo 2014-02-13 21:07 ` Tejun Heo 2014-02-13 14:23 ` [PATCH 1/2] memcg: fix endless loop in __mem_cgroup_iter_next Michal Hocko 2014-02-13 14: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=alpine.LSU.2.11.1402121727050.5917@eggly.anvils \ --to=hughd@google.com \ --cc=akpm@linux-foundation.org \ --cc=gthelen@google.com \ --cc=hannes@cmpxchg.org \ --cc=linux-kernel@vger.kernel.org \ --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: linkBe 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.