On Tue, Jan 14, 2025 at 06:45:20PM +0000, Roman Gushchin wrote:
> On Tue, Jan 14, 2025 at 12:25:19PM +0000, Chen Ridong wrote:
> > From: Chen Ridong <chenridong@xxxxxxxxxx>
> >
> > The only difference between 'lruvec_page_state' and
> > 'lruvec_page_state_local' is that they read 'state' and 'state_local',
> > respectively. Factor out an inner functions to make the code more concise.
> > Do the same for reading 'memcg_page_stat' and 'memcg_events'.
> >
> > Signed-off-by: Chen Ridong <chenridong@xxxxxxxxxx>
> > ---
> > mm/memcontrol.c | 72 +++++++++++++++++++++----------------------------
> > 1 file changed, 30 insertions(+), 42 deletions(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index b10e0a8f3375..14541610cad0 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -375,7 +375,8 @@ struct lruvec_stats {
> > long state_pending[NR_MEMCG_NODE_STAT_ITEMS];
> > };
> >
> > -unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx)
> > +static unsigned long __lruvec_page_state(struct lruvec *lruvec,
> > + enum node_stat_item idx, bool local)
> > {
> > struct mem_cgroup_per_node *pn;
> > long x;
> > @@ -389,7 +390,8 @@ unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx)
> > return 0;
> >
> > pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
> > - x = READ_ONCE(pn->lruvec_stats->state[i]);
> > + x = local ? READ_ONCE(pn->lruvec_stats->state_local[i]) :
> > + READ_ONCE(pn->lruvec_stats->state[i]);
> > #ifdef CONFIG_SMP
> > if (x < 0)
> > x = 0;
> > @@ -397,27 +399,16 @@ unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx)
> > return x;
> > }
> >
> > +
> > +unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx)
> > +{
> > + return __lruvec_page_state(lruvec, idx, false);
> > +}
>
> I'd move these wrapper function definitions to memcontrol.h and make them
> static inline.
+1
>
> Other than that, lgtm.
>
> Thank you!
