| Message ID | 20220403020833.26164-1-richard.weiyang@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/memcg: non-hierarchical mode is deprecated | expand |
On Sun 03-04-22 02:08:33, Wei Yang wrote: > After commit bef8620cd8e0 ("mm: memcg: deprecate the non-hierarchical > mode"), we won't have a NULL parent except root_mem_cgroup. And this > case is handled when (memcg == root). > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > CC: Roman Gushchin <roman.gushchin@linux.dev> > CC: Johannes Weiner <hannes@cmpxchg.org> Acked-by: Michal Hocko <mhocko@suse.com> Thanks! > --- > mm/memcontrol.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 2cd8bfdec379..3ceb9b8592b1 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -6587,9 +6587,6 @@ void mem_cgroup_calculate_protection(struct mem_cgroup *root, > return; > > parent = parent_mem_cgroup(memcg); > - /* No parent means a non-hierarchical mode on v1 memcg */ > - if (!parent) > - return; > > if (parent == root) { > memcg->memory.emin = READ_ONCE(memcg->memory.min); > -- > 2.33.1
On Sun, Apr 03, 2022 at 02:08:33AM +0000, Wei Yang wrote: > After commit bef8620cd8e0 ("mm: memcg: deprecate the non-hierarchical > mode"), we won't have a NULL parent except root_mem_cgroup. And this > case is handled when (memcg == root). > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > CC: Roman Gushchin <roman.gushchin@linux.dev> > CC: Johannes Weiner <hannes@cmpxchg.org> Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev> Thanks! > --- > mm/memcontrol.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 2cd8bfdec379..3ceb9b8592b1 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -6587,9 +6587,6 @@ void mem_cgroup_calculate_protection(struct mem_cgroup *root, > return; > > parent = parent_mem_cgroup(memcg); > - /* No parent means a non-hierarchical mode on v1 memcg */ > - if (!parent) > - return; > > if (parent == root) { > memcg->memory.emin = READ_ONCE(memcg->memory.min); > -- > 2.33.1 >
On Mon, Apr 04, 2022 at 11:27:53AM +0200, Michal Hocko wrote: >On Sun 03-04-22 02:08:33, Wei Yang wrote: >> After commit bef8620cd8e0 ("mm: memcg: deprecate the non-hierarchical >> mode"), we won't have a NULL parent except root_mem_cgroup. And this >> case is handled when (memcg == root). >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> CC: Roman Gushchin <roman.gushchin@linux.dev> >> CC: Johannes Weiner <hannes@cmpxchg.org> > >Acked-by: Michal Hocko <mhocko@suse.com> >Thanks! > Thanks for the ack. When reading the code, I found one redundant check in shrink_node_memcgs(). shrink_node_memcgs mem_cgroup_below_min mem_cgroup_supports_protection mem_cgroup_below_low mem_cgroup_supports_protection I am not sure it worthwhile to take it out. shrink_node_memcgs mem_cgroup_supports_protection mem_cgroup_below_min mem_cgroup_below_low Look forward your opinion.
On Tue 05-04-22 02:22:18, Wei Yang wrote: > On Mon, Apr 04, 2022 at 11:27:53AM +0200, Michal Hocko wrote: > >On Sun 03-04-22 02:08:33, Wei Yang wrote: > >> After commit bef8620cd8e0 ("mm: memcg: deprecate the non-hierarchical > >> mode"), we won't have a NULL parent except root_mem_cgroup. And this > >> case is handled when (memcg == root). > >> > >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > >> CC: Roman Gushchin <roman.gushchin@linux.dev> > >> CC: Johannes Weiner <hannes@cmpxchg.org> > > > >Acked-by: Michal Hocko <mhocko@suse.com> > >Thanks! > > > > Thanks for the ack. When reading the code, I found one redundant check in > shrink_node_memcgs(). > > shrink_node_memcgs > mem_cgroup_below_min > mem_cgroup_supports_protection > mem_cgroup_below_low > mem_cgroup_supports_protection > > I am not sure it worthwhile to take it out. > > shrink_node_memcgs > mem_cgroup_supports_protection > mem_cgroup_below_min > mem_cgroup_below_low > > Look forward your opinion. I guess you refer to mem_cgroup_is_root check in mem_cgroup_supports_protection, right? You are right that the check is not really required because e{min,low} should always stay at 0 for the root memcg AFAICS. On the other hand the check is not in any hot path and it really adds clarity here because protection is not really supported on the root memcg. So I am not this is an overall win.
On Tue, Apr 05, 2022 at 08:26:59AM +0200, Michal Hocko wrote: >On Tue 05-04-22 02:22:18, Wei Yang wrote: >> On Mon, Apr 04, 2022 at 11:27:53AM +0200, Michal Hocko wrote: >> >On Sun 03-04-22 02:08:33, Wei Yang wrote: >> >> After commit bef8620cd8e0 ("mm: memcg: deprecate the non-hierarchical >> >> mode"), we won't have a NULL parent except root_mem_cgroup. And this >> >> case is handled when (memcg == root). >> >> >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> >> CC: Roman Gushchin <roman.gushchin@linux.dev> >> >> CC: Johannes Weiner <hannes@cmpxchg.org> >> > >> >Acked-by: Michal Hocko <mhocko@suse.com> >> >Thanks! >> > >> >> Thanks for the ack. When reading the code, I found one redundant check in >> shrink_node_memcgs(). >> >> shrink_node_memcgs >> mem_cgroup_below_min >> mem_cgroup_supports_protection >> mem_cgroup_below_low >> mem_cgroup_supports_protection >> >> I am not sure it worthwhile to take it out. >> >> shrink_node_memcgs >> mem_cgroup_supports_protection >> mem_cgroup_below_min >> mem_cgroup_below_low >> >> Look forward your opinion. > >I guess you refer to mem_cgroup_is_root check in mem_cgroup_supports_protection, >right? > >You are right that the check is not really required because e{min,low} >should always stay at 0 for the root memcg AFAICS. On the other hand the >check is not in any hot path and it really adds clarity here because >protection is not really supported on the root memcg. So I am not this >is an overall win. Agree. >-- >Michal Hocko >SUSE Labs
On Sun, Apr 03, 2022 at 02:08:33AM +0000, Wei Yang wrote: > After commit bef8620cd8e0 ("mm: memcg: deprecate the non-hierarchical > mode"), we won't have a NULL parent except root_mem_cgroup. And this > case is handled when (memcg == root). > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > CC: Roman Gushchin <roman.gushchin@linux.dev> > CC: Johannes Weiner <hannes@cmpxchg.org> Reviewed-by: Shakeel Butt <shakeelb@google.com>
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 2cd8bfdec379..3ceb9b8592b1 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -6587,9 +6587,6 @@ void mem_cgroup_calculate_protection(struct mem_cgroup *root, return; parent = parent_mem_cgroup(memcg); - /* No parent means a non-hierarchical mode on v1 memcg */ - if (!parent) - return; if (parent == root) { memcg->memory.emin = READ_ONCE(memcg->memory.min);
After commit bef8620cd8e0 ("mm: memcg: deprecate the non-hierarchical mode"), we won't have a NULL parent except root_mem_cgroup. And this case is handled when (memcg == root). Signed-off-by: Wei Yang <richard.weiyang@gmail.com> CC: Roman Gushchin <roman.gushchin@linux.dev> CC: Johannes Weiner <hannes@cmpxchg.org> --- mm/memcontrol.c | 3 --- 1 file changed, 3 deletions(-)