On Tue, Oct 27, 2020 at 9:36 PM Michal Hocko [off-list ref] wrote:

On Tue 27-10-20 16:02:56, Muchun Song wrote:

quoted

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 ...

quoted

Signed-off-by: Muchun Song <redacted>
---
 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