Re: [RFC][PATCH 6/9] mm/vmscan: add page demotion counter

[Yang Shi]

On Thu, Aug 20, 2020 at 3:26 PM Yang Shi <shy828301@xxxxxxxxx> wrote:
>
> On Tue, Aug 18, 2020 at 11:53 AM Dave Hansen
> <dave.hansen@xxxxxxxxxxxxxxx> wrote:
> >
> >
> > From: Yang Shi <yang.shi@xxxxxxxxxxxxxxxxx>
> >
> > Account the number of demoted pages into reclaim_state->nr_demoted.
> >
> > Add pgdemote_kswapd and pgdemote_direct VM counters showed in
> > /proc/vmstat.
> >
> > [ daveh:
> >    - __count_vm_events() a bit, and made them look at the THP
> >      size directly rather than getting data from migrate_pages()
> > ]
>
> This reminds me that we'd better to make migrate_pages() return the
> number of base pages. Now both nr_failed and nr_succeeded is inc'ed
> regardless of whether it is THP.
>
> A quick look finds the callers of migrate_pages() seem not care about
> the exact returned number, they just care if it is positive or not.
>
> And reclaim code depends on correct nr_succeeded to avoid over reclaim.
>
> I will come up with a patch to post to the mailing list.

I looks this problem has been fixed by commit
1a5bae25e3cf95c4e83a97f87a6b5280d9acbb22 ("mm/vmstat: add events for
THP migration without split") in v5.9-rc1. With that commit,
nr_succeeded will be bumped by 512 if a THP is migrated successfully
(without split). So, we don't need worry about this anymore.

>
> >
> > Signed-off-by: Yang Shi <yang.shi@xxxxxxxxxxxxxxxxx>
> > Signed-off-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> > Cc: David Rientjes <rientjes@xxxxxxxxxx>
> > Cc: Huang Ying <ying.huang@xxxxxxxxx>
> > Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> > ---
> >
> >  b/include/linux/vm_event_item.h |    2 ++
> >  b/mm/vmscan.c                   |    6 ++++++
> >  b/mm/vmstat.c                   |    2 ++
> >  3 files changed, 10 insertions(+)
> >
> > diff -puN include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter include/linux/vm_event_item.h
> > --- a/include/linux/vm_event_item.h~mm-vmscan-add-page-demotion-counter 2020-08-18 11:36:54.062583176 -0700
> > +++ b/include/linux/vm_event_item.h     2020-08-18 11:36:54.070583176 -0700
> > @@ -32,6 +32,8 @@ enum vm_event_item { PGPGIN, PGPGOUT, PS
> >                 PGREFILL,
> >                 PGSTEAL_KSWAPD,
> >                 PGSTEAL_DIRECT,
> > +               PGDEMOTE_KSWAPD,
> > +               PGDEMOTE_DIRECT,
> >                 PGSCAN_KSWAPD,
> >                 PGSCAN_DIRECT,
> >                 PGSCAN_DIRECT_THROTTLE,
> > diff -puN mm/vmscan.c~mm-vmscan-add-page-demotion-counter mm/vmscan.c
> > --- a/mm/vmscan.c~mm-vmscan-add-page-demotion-counter   2020-08-18 11:36:54.064583176 -0700
> > +++ b/mm/vmscan.c       2020-08-18 11:36:54.072583176 -0700
> > @@ -147,6 +147,7 @@ struct scan_control {
> >                 unsigned int immediate;
> >                 unsigned int file_taken;
> >                 unsigned int taken;
> > +               unsigned int demoted;
> >         } nr;
> >
> >         /* for recording the reclaimed slab by now */
> > @@ -1146,6 +1147,11 @@ static unsigned int demote_page_list(str
> >                 list_splice(ret_list, demote_pages);
> >         }
> >
> > +       if (current_is_kswapd())
> > +               __count_vm_events(PGDEMOTE_KSWAPD, nr_succeeded);
> > +       else
> > +               __count_vm_events(PGDEMOTE_DIRECT, nr_succeeded);
> > +
> >         return nr_succeeded;
> >  }
> >
> > diff -puN mm/vmstat.c~mm-vmscan-add-page-demotion-counter mm/vmstat.c
> > --- a/mm/vmstat.c~mm-vmscan-add-page-demotion-counter   2020-08-18 11:36:54.067583176 -0700
> > +++ b/mm/vmstat.c       2020-08-18 11:36:54.072583176 -0700
> > @@ -1200,6 +1200,8 @@ const char * const vmstat_text[] = {
> >         "pgrefill",
> >         "pgsteal_kswapd",
> >         "pgsteal_direct",
> > +       "pgdemote_kswapd",
> > +       "pgdemote_direct",
> >         "pgscan_kswapd",
> >         "pgscan_direct",
> >         "pgscan_direct_throttle",
> > _