Re: [PATCH] vmscan: add a vmscan event for reclaim_pages

[Vlastimil Babka]

On 10/9/24 11:31 AM, Jaewon Kim wrote:
> The reclaim_folio_list uses a dummy reclaim_stat and is not being
> used. To know the memory stat, add a new trace event. This is useful how
> how many pages are not reclaimed or why.
> 
> This is an example.
> mm_vmscan_reclaim_pages: nr_scanned=17 nr_reclaimed=17 nr_dirty=0 nr_writeback=0 nr_congested=0 nr_immediate=0 nr_activate_anon=0 nr_activate_file=0 nr_ref_keep=0 nr_unmap_fail=0
> 
> Currenlty reclaim_folio_list is only called by reclaim_pages, and
> reclaim_pages is used by damon and madvise. In the latest Android,
> reclaim_pages is also used by shmem to reclaim all pages in a
> address_space.
> 
> Signed-off-by: Jaewon Kim <jaewon31.kim@xxxxxxxxxxx>
> ---
>  include/trace/events/vmscan.h | 41 +++++++++++++++++++++++++++++++++++
>  mm/vmscan.c                   | 40 +++++++++++++++++++++++++++++-----
>  2 files changed, 76 insertions(+), 5 deletions(-)
> 
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index 1a488c30afa5..509110a12fa5 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -346,6 +346,47 @@ TRACE_EVENT(mm_vmscan_write_folio,
>  		show_reclaim_flags(__entry->reclaim_flags))
>  );
>  
> +TRACE_EVENT(mm_vmscan_reclaim_pages,
> +
> +	TP_PROTO(unsigned long nr_scanned, unsigned long nr_reclaimed,
> +		struct reclaim_stat *stat),
> +
> +	TP_ARGS(nr_scanned, nr_reclaimed, stat),
> +
> +	TP_STRUCT__entry(
> +		__field(unsigned long, nr_scanned)
> +		__field(unsigned long, nr_reclaimed)
> +		__field(unsigned long, nr_dirty)
> +		__field(unsigned long, nr_writeback)
> +		__field(unsigned long, nr_congested)
> +		__field(unsigned long, nr_immediate)
> +		__field(unsigned int, nr_activate0)
> +		__field(unsigned int, nr_activate1)
> +		__field(unsigned long, nr_ref_keep)
> +		__field(unsigned long, nr_unmap_fail)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->nr_scanned = nr_scanned;
> +		__entry->nr_reclaimed = nr_reclaimed;
> +		__entry->nr_dirty = stat->nr_dirty;
> +		__entry->nr_writeback = stat->nr_writeback;
> +		__entry->nr_congested = stat->nr_congested;
> +		__entry->nr_immediate = stat->nr_immediate;
> +		__entry->nr_activate0 = stat->nr_activate[0];
> +		__entry->nr_activate1 = stat->nr_activate[1];
> +		__entry->nr_ref_keep = stat->nr_ref_keep;
> +		__entry->nr_unmap_fail = stat->nr_unmap_fail;
> +	),
> +
> +	TP_printk("nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d nr_ref_keep=%ld nr_unmap_fail=%ld",
> +		__entry->nr_scanned, __entry->nr_reclaimed,
> +		__entry->nr_dirty, __entry->nr_writeback,
> +		__entry->nr_congested, __entry->nr_immediate,
> +		__entry->nr_activate0, __entry->nr_activate1,
> +		__entry->nr_ref_keep, __entry->nr_unmap_fail)
> +);
> +
>  TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
>  
>  	TP_PROTO(int nid,
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 749cdc110c74..4776c42dfd2a 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2126,9 +2126,9 @@ static void shrink_active_list(unsigned long nr_to_scan,
>  }
>  
>  static unsigned int reclaim_folio_list(struct list_head *folio_list,
> -				      struct pglist_data *pgdat)
> +				      struct pglist_data *pgdat,
> +				      struct reclaim_stat *stat)
>  {
> -	struct reclaim_stat dummy_stat;
>  	unsigned int nr_reclaimed;
>  	struct folio *folio;
>  	struct scan_control sc = {
> @@ -2139,7 +2139,7 @@ static unsigned int reclaim_folio_list(struct list_head *folio_list,
>  		.no_demotion = 1,
>  	};
>  
> -	nr_reclaimed = shrink_folio_list(folio_list, pgdat, &sc, &dummy_stat, true);
> +	nr_reclaimed = shrink_folio_list(folio_list, pgdat, &sc, stat, true);
>  	while (!list_empty(folio_list)) {
>  		folio = lru_to_folio(folio_list);
>  		list_del(&folio->lru);
> @@ -2149,16 +2149,40 @@ static unsigned int reclaim_folio_list(struct list_head *folio_list,
>  	return nr_reclaimed;
>  }
>  
> +static void reclaim_stat_add(struct reclaim_stat *stat_from,
> +			     struct reclaim_stat *stat_to)
> +{
> +	int type;
> +
> +	if (!trace_mm_vmscan_reclaim_pages_enabled())
> +		return;
> +
> +	stat_to->nr_dirty += stat_from->nr_dirty;
> +	stat_to->nr_unqueued_dirty += stat_from->nr_unqueued_dirty;
> +	stat_to->nr_congested += stat_from->nr_congested;
> +	stat_to->nr_writeback += stat_from->nr_writeback;
> +	stat_to->nr_immediate += stat_from->nr_immediate;
> +	stat_to->nr_pageout += stat_from->nr_pageout;
> +	for (type = 0; type < ANON_AND_FILE; type++)
> +		stat_to->nr_activate[type] += stat_from->nr_activate[type];
> +	stat_to->nr_ref_keep += stat_from->nr_ref_keep;
> +	stat_to->nr_unmap_fail += stat_from->nr_unmap_fail;
> +	stat_to->nr_lazyfree_fail += stat_from->nr_lazyfree_fail;
> +}

Could we avoid this by using a single stat that just accumulates over
multiple calls to reclaim_folio_list()?

That means shrink_folio_list() would not do the initial memset(0) and it
would be caller responsibility.

AFAICS shrink_folio_list() only cares about these fields:

pgactivate = stat->nr_activate[0] + stat->nr_activate[1];

in order to do

count_vm_events(PGACTIVATE, pgactivate);

Which could be adjusted to deal with accumulating stat - i.e. take an
initial sum of the fields in stat and subtract from the final sum to get
the delta.

>  unsigned long reclaim_pages(struct list_head *folio_list)
>  {
>  	int nid;
> +	unsigned int nr_scanned = 0;
>  	unsigned int nr_reclaimed = 0;
>  	LIST_HEAD(node_folio_list);
>  	unsigned int noreclaim_flag;
> +	struct reclaim_stat stat_total, stat_one;
>  
>  	if (list_empty(folio_list))
>  		return nr_reclaimed;
>  
> +	memset(&stat_total, 0, sizeof(stat_total));
>  	noreclaim_flag = memalloc_noreclaim_save();
>  
>  	nid = folio_nid(lru_to_folio(folio_list));
> @@ -2168,14 +2192,20 @@ unsigned long reclaim_pages(struct list_head *folio_list)
>  		if (nid == folio_nid(folio)) {
>  			folio_clear_active(folio);
>  			list_move(&folio->lru, &node_folio_list);
> +			nr_scanned += folio_nr_pages(folio);
>  			continue;
>  		}
>  
> -		nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid));
> +		nr_reclaimed += reclaim_folio_list(&node_folio_list,
> +						   NODE_DATA(nid), &stat_one);
> +		reclaim_stat_add(&stat_one, &stat_total);
>  		nid = folio_nid(lru_to_folio(folio_list));
>  	} while (!list_empty(folio_list));
>  
> -	nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid));
> +	nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid),
> +					   &stat_one);
> +	reclaim_stat_add(&stat_one, &stat_total);
> +	trace_mm_vmscan_reclaim_pages(nr_scanned, nr_reclaimed, &stat_total);
>  
>  	memalloc_noreclaim_restore(noreclaim_flag);
>