On Thu, Aug 20, 2020 at 3:26 PM Yang Shi <[email protected]> wrote:
>
> On Tue, Aug 18, 2020 at 11:53 AM Dave Hansen
> <[email protected]> wrote:
> >
> >
> > From: Yang Shi <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Dave Hansen <[email protected]>
> > Cc: David Rientjes <[email protected]>
> > Cc: Huang Ying <[email protected]>
> > Cc: Dan Williams <[email protected]>
> > ---
> >
> > 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",
> > _
