Re: [PATCH] mm/memcg: bail out early when !memcg in mem_cgroup_lruvec

[Andrew Morton]

On Fri, 27 Nov 2020 11:08:35 +0800 Alex Shi <alex....@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;
        }