| Message ID | e809b4e6b0c1626dac6945970de06409a180ee65.1584034301.git.chris@chrisdown.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm, memcg: cgroup v2 tunable load/store tearing fixes | expand |
On Thu 12-03-20 17:33:07, Chris Down wrote: > This can be set concurrently with reads, which may cause the wrong value > to be propagated. > > Signed-off-by: Chris Down <chris@chrisdown.name> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Johannes Weiner <hannes@cmpxchg.org> > Cc: Roman Gushchin <guro@fb.com> > Cc: Tejun Heo <tj@kernel.org> > Cc: linux-mm@kvack.org > Cc: cgroups@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: kernel-team@fb.com Acked-by: Michal Hocko <mhocko@suse.com> > --- > mm/memcontrol.c | 4 ++-- > mm/page_counter.c | 10 ++++++---- > 2 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index c85a304fa4a1..e0ed790a2a8c 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -6261,7 +6261,7 @@ enum mem_cgroup_protection mem_cgroup_protected(struct mem_cgroup *root, > if (!usage) > return MEMCG_PROT_NONE; > > - emin = memcg->memory.min; > + emin = READ_ONCE(memcg->memory.min); > elow = READ_ONCE(memcg->memory.low); > > parent = parent_mem_cgroup(memcg); > @@ -6277,7 +6277,7 @@ enum mem_cgroup_protection mem_cgroup_protected(struct mem_cgroup *root, > if (emin && parent_emin) { > unsigned long min_usage, siblings_min_usage; > > - min_usage = min(usage, memcg->memory.min); > + min_usage = min(usage, READ_ONCE(memcg->memory.min)); > siblings_min_usage = atomic_long_read( > &parent->memory.children_min_usage); > > diff --git a/mm/page_counter.c b/mm/page_counter.c > index 18b7f779f2e2..ae471c7d255f 100644 > --- a/mm/page_counter.c > +++ b/mm/page_counter.c > @@ -17,14 +17,16 @@ static void propagate_protected_usage(struct page_counter *c, > unsigned long usage) > { > unsigned long protected, old_protected; > - unsigned long low; > + unsigned long low, min; > long delta; > > if (!c->parent) > return; > > - if (c->min || atomic_long_read(&c->min_usage)) { > - if (usage <= c->min) > + min = READ_ONCE(c->min); > + > + if (min || atomic_long_read(&c->min_usage)) { > + if (usage <= min) > protected = usage; > else > protected = 0; > @@ -217,7 +219,7 @@ void page_counter_set_min(struct page_counter *counter, unsigned long nr_pages) > { > struct page_counter *c; > > - counter->min = nr_pages; > + WRITE_ONCE(counter->min, nr_pages); > > for (c = counter; c; c = c->parent) > propagate_protected_usage(c, atomic_long_read(&c->usage)); > -- > 2.25.1 >
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index c85a304fa4a1..e0ed790a2a8c 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -6261,7 +6261,7 @@ enum mem_cgroup_protection mem_cgroup_protected(struct mem_cgroup *root, if (!usage) return MEMCG_PROT_NONE; - emin = memcg->memory.min; + emin = READ_ONCE(memcg->memory.min); elow = READ_ONCE(memcg->memory.low); parent = parent_mem_cgroup(memcg); @@ -6277,7 +6277,7 @@ enum mem_cgroup_protection mem_cgroup_protected(struct mem_cgroup *root, if (emin && parent_emin) { unsigned long min_usage, siblings_min_usage; - min_usage = min(usage, memcg->memory.min); + min_usage = min(usage, READ_ONCE(memcg->memory.min)); siblings_min_usage = atomic_long_read( &parent->memory.children_min_usage); diff --git a/mm/page_counter.c b/mm/page_counter.c index 18b7f779f2e2..ae471c7d255f 100644 --- a/mm/page_counter.c +++ b/mm/page_counter.c @@ -17,14 +17,16 @@ static void propagate_protected_usage(struct page_counter *c, unsigned long usage) { unsigned long protected, old_protected; - unsigned long low; + unsigned long low, min; long delta; if (!c->parent) return; - if (c->min || atomic_long_read(&c->min_usage)) { - if (usage <= c->min) + min = READ_ONCE(c->min); + + if (min || atomic_long_read(&c->min_usage)) { + if (usage <= min) protected = usage; else protected = 0; @@ -217,7 +219,7 @@ void page_counter_set_min(struct page_counter *counter, unsigned long nr_pages) { struct page_counter *c; - counter->min = nr_pages; + WRITE_ONCE(counter->min, nr_pages); for (c = counter; c; c = c->parent) propagate_protected_usage(c, atomic_long_read(&c->usage));
This can be set concurrently with reads, which may cause the wrong value to be propagated. Signed-off-by: Chris Down <chris@chrisdown.name> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Roman Gushchin <guro@fb.com> Cc: Tejun Heo <tj@kernel.org> Cc: linux-mm@kvack.org Cc: cgroups@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: kernel-team@fb.com --- mm/memcontrol.c | 4 ++-- mm/page_counter.c | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-)