linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Yang Shi <shy828301@gmail.com>
To: guro@fb.com, ktkhai@virtuozzo.com, shakeelb@google.com,
	david@fromorbit.com, hannes@cmpxchg.org, mhocko@suse.com,
	akpm@linux-foundation.org
Cc: shy828301@gmail.com, linux-mm@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [v2 PATCH 3/9] mm: vmscan: guarantee shrinker_slab_memcg() sees valid shrinker_maps for online memcg
Date: Mon, 14 Dec 2020 14:37:16 -0800	[thread overview]
Message-ID: <20201214223722.232537-4-shy828301@gmail.com> (raw)
In-Reply-To: <20201214223722.232537-1-shy828301@gmail.com>

The shrink_slab_memcg() races with mem_cgroup_css_online(). A visibility of CSS_ONLINE flag
in shrink_slab_memcg()->mem_cgroup_online() does not guarantee that we will see
memcg->nodeinfo[nid]->shrinker_maps != NULL.  This may occur because of processor reordering
on !x86.

This seems like the below case:

           CPU A          CPU B
store shrinker_map      load CSS_ONLINE
store CSS_ONLINE        load shrinker_map

So the memory ordering could be guaranteed by smp_wmb()/smp_rmb() pair.

The memory barriers pair will guarantee the ordering between shrinker_deferred and CSS_ONLINE
for the following patches as well.

Signed-off-by: Yang Shi <shy828301@gmail.com>
---
 mm/memcontrol.c | 7 +++++++
 mm/vmscan.c     | 8 +++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index ed942734235f..3d4ddbb84a01 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5406,6 +5406,13 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
 		return -ENOMEM;
 	}
 
+	/*
+	 * Barrier for CSS_ONLINE, so that shrink_slab_memcg() sees shirnker_maps
+	 * and shrinker_deferred before CSS_ONLINE. It pairs with the read barrier
+	 * in shrink_slab_memcg().
+	 */
+	smp_wmb();
+
 	/* Online state pins memcg ID, memcg ID pins CSS */
 	refcount_set(&memcg->id.ref, 1);
 	css_get(css);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 912c044301dd..9b31b9c419ec 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -552,13 +552,15 @@ static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid,
 	if (!mem_cgroup_online(memcg))
 		return 0;
 
+	/* Pairs with write barrier in mem_cgroup_css_online() */
+	smp_rmb();
+
 	if (!down_read_trylock(&shrinker_rwsem))
 		return 0;
 
+	/* Once memcg is online it can't be NULL */
 	map = rcu_dereference_protected(memcg->nodeinfo[nid]->shrinker_map,
 					true);
-	if (unlikely(!map))
-		goto unlock;
 
 	for_each_set_bit(i, map->map, shrinker_nr_max) {
 		struct shrink_control sc = {
@@ -612,7 +614,7 @@ static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid,
 			break;
 		}
 	}
-unlock:
+
 	up_read(&shrinker_rwsem);
 	return freed;
 }
-- 
2.26.2



  parent reply	other threads:[~2020-12-14 22:37 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-14 22:37 [RFC v2 PATCH 0/9] Make shrinker's nr_deferred memcg aware Yang Shi
2020-12-14 22:37 ` [v2 PATCH 1/9] mm: vmscan: use nid from shrink_control for tracepoint Yang Shi
2020-12-14 22:37 ` [v2 PATCH 2/9] mm: memcontrol: use shrinker_rwsem to protect shrinker_maps allocation Yang Shi
2020-12-15  2:09   ` Dave Chinner
2020-12-15 13:53     ` Johannes Weiner
2020-12-15 21:59       ` Dave Chinner
2020-12-16 13:17         ` Kirill Tkhai
2020-12-16 19:12         ` Johannes Weiner
2020-12-16 21:56           ` Yang Shi
2020-12-16 19:39         ` Roman Gushchin
2020-12-15 14:07   ` Johannes Weiner
2020-12-15 20:32     ` Yang Shi
2020-12-14 22:37 ` Yang Shi [this message]
2020-12-15  2:04   ` [v2 PATCH 3/9] mm: vmscan: guarantee shrinker_slab_memcg() sees valid shrinker_maps for online memcg Dave Chinner
     [not found]   ` <20201215123802.GA379720@cmpxchg.org>
2020-12-15 12:58     ` Kirill Tkhai
2020-12-15 17:14   ` Johannes Weiner
     [not found]     ` <CAHbLzkrzv48S3ks-x8M=2sHxRS_+c-hLXdt4ScaWD6mC4ZFe8w@mail.gmail.com>
2020-12-28 20:03       ` Yang Shi
2020-12-14 22:37 ` [v2 PATCH 4/9] mm: vmscan: use a new flag to indicate shrinker is registered Yang Shi
2020-12-14 22:37 ` [v2 PATCH 5/9] mm: memcontrol: add per memcg shrinker nr_deferred Yang Shi
2020-12-15  2:22   ` Dave Chinner
2020-12-15 14:45     ` Johannes Weiner
2020-12-15 21:57       ` Yang Shi
2020-12-14 22:37 ` [v2 PATCH 6/9] mm: vmscan: use per memcg nr_deferred of shrinker Yang Shi
2020-12-15  2:46   ` Dave Chinner
2020-12-15 22:27     ` Yang Shi
2020-12-15 23:48       ` Dave Chinner
2020-12-14 22:37 ` [v2 PATCH 7/9] mm: vmscan: don't need allocate shrinker->nr_deferred for memcg aware shrinkers Yang Shi
2020-12-15  3:05   ` Dave Chinner
2020-12-15 23:07     ` Yang Shi
2020-12-18  0:56       ` Yang Shi
2020-12-18  1:09         ` Dave Chinner
2020-12-14 22:37 ` [v2 PATCH 8/9] mm: memcontrol: reparent nr_deferred when memcg offline Yang Shi
2020-12-15  3:07   ` Dave Chinner
2020-12-15 23:10     ` Yang Shi
2020-12-14 22:37 ` [v2 PATCH 9/9] mm: vmscan: shrink deferred objects proportional to priority Yang Shi
2020-12-15  3:23   ` Dave Chinner
2020-12-15 23:59     ` Yang Shi

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=20201214223722.232537-4-shy828301@gmail.com \
    --to=shy828301@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@fromorbit.com \
    --cc=guro@fb.com \
    --cc=hannes@cmpxchg.org \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=shakeelb@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