Messages in this thread |  | | Date | Fri, 27 Nov 2020 20:02:15 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] mm/memcg: bail out early when !memcg in mem_cgroup_lruvec |
| |
On Fri, 27 Nov 2020 11:08:35 +0800 Alex Shi <alex.shi@linux.alibaba.com> wrote:
> Sometime, we use NULL memcg in mem_cgroup_lruvec(memcg, pgdat) > so we could get out early in the situation to avoid useless checking. > > Also warning if both parameter are NULL.
Why do you think a warning is needed here?
> --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -613,14 +613,13 @@ static inline struct lruvec *mem_cgroup_lruvec(struct mem_cgroup *memcg, > struct mem_cgroup_per_node *mz; > struct lruvec *lruvec; > > - if (mem_cgroup_disabled()) { > + VM_WARN_ON_ONCE(!memcg && !pgdat); > + > + if (mem_cgroup_disabled() || !memcg) { > lruvec = &pgdat->__lruvec; > goto out; > } > > - if (!memcg) > - memcg = root_mem_cgroup; > -
This change isn't obviously equivalent, is it?
> mz = mem_cgroup_nodeinfo(memcg, pgdat->node_id); > lruvec = &mz->lruvec; > out:
And the resulting code is awkward:
if (mem_cgroup_disabled() || !memcg) { lruvec = &pgdat->__lruvec; goto out; }
mz = mem_cgroup_nodeinfo(memcg, pgdat->node_id); lruvec = &mz->lruvec; out:
could be
if (mem_cgroup_disabled() || !memcg) { lruvec = &pgdat->__lruvec; } else { mem_cgroup_per_node mz;
mz = mem_cgroup_nodeinfo(memcg, pgdat->node_id); lruvec = &mz->lruvec; }
|  |