On Tue, Oct 27, 2020 at 9:36 PM Michal Hocko <mhocko@xxxxxxxx> wrote: > > On Tue 27-10-20 16:02:56, Muchun Song wrote: > > We can reuse the code of mem_cgroup_lruvec() to simplify the code > > of the mem_cgroup_page_lruvec(). > > yes, removing the code duplication is reasonable. But ... > > > > > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx> > > --- > > include/linux/memcontrol.h | 44 +++++++++++++++++++++++++++----------- > > mm/memcontrol.c | 40 ---------------------------------- > > 2 files changed, 32 insertions(+), 52 deletions(-) > > > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > > index 95807bf6be64..5e8480e54cd8 100644 > > --- a/include/linux/memcontrol.h > > +++ b/include/linux/memcontrol.h > > @@ -451,16 +451,9 @@ mem_cgroup_nodeinfo(struct mem_cgroup *memcg, int nid) > > return memcg->nodeinfo[nid]; > > } > > > > -/** > > - * mem_cgroup_lruvec - get the lru list vector for a memcg & node > > - * @memcg: memcg of the wanted lruvec > > - * > > - * Returns the lru list vector holding pages for a given @memcg & > > - * @node combination. This can be the node lruvec, if the memory > > - * controller is disabled. > > - */ > > -static inline struct lruvec *mem_cgroup_lruvec(struct mem_cgroup *memcg, > > - struct pglist_data *pgdat) > > +static inline struct lruvec *mem_cgroup_node_lruvec(struct mem_cgroup *memcg, > > + struct pglist_data *pgdat, > > + int nid) > > This is just wrong interface. Either take nid or pgdat. You do not want > both because that just begs for wrong usage. If we want to avoid abuse of mem_cgroup_node_lruvec. We can move those functions to the memcontrol.c. And add the "static" attribute to the mem_cgroup_node_lruvec. Just export mem_cgroup_lruvec and mem_cgroup_page_lruvec. Is this OK? Thanks. > -- > Michal Hocko > SUSE Labs -- Yours, Muchun