Re: [PATCH] mm/vmscan: remove pglist_data->inactive_ratio

[Michal Hocko]

On Tue 19-07-16 10:07:29, Ganesh Mahendran wrote:
> In patch [1], the inactive_ratio is now automatically calculated

It is better to give the direct reference to the patch 59dc76b0d4df
("mm: vmscan: reduce size of inactive file list")

> in inactive_list_is_low(). So there is no need to keep inactive_ratio
> in pglist_data,

OK

> and shown in zoneinfo.

I am not so sure about this. To be honest I have never really used this
value but maybe there is somebody outher who relies on it. It would be
safer if the ratio calculation in inactive_list_is_low would be
extracted and used to display the information rather than dropping that
on the floor.

The patch should also state that the above patch has broken the zoneinfo
information.

> [1] mm: vmscan: reduce size of inactive file list
> 
> Signed-off-by: Ganesh Mahendran <opensource.ganesh@xxxxxxxxx>
> ---
>  include/linux/mmzone.h | 6 ------
>  mm/vmscan.c            | 2 +-
>  mm/vmstat.c            | 6 ++----
>  3 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index a3b7f45..b3ade54 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -700,12 +700,6 @@ typedef struct pglist_data {
>  	/* Fields commonly accessed by the page reclaim scanner */
>  	struct lruvec		lruvec;
>  
> -	/*
> -	 * The target ratio of ACTIVE_ANON to INACTIVE_ANON pages on
> -	 * this node's LRU.  Maintained by the pageout code.
> -	 */
> -	unsigned int inactive_ratio;
> -
>  	unsigned long		flags;
>  
>  	ZONE_PADDING(_pad2_)
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 429bf3a..3c1de58 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1915,7 +1915,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
>   * page has a chance to be referenced again before it is reclaimed.
>   *
>   * The inactive_ratio is the target ratio of ACTIVE to INACTIVE pages
> - * on this LRU, maintained by the pageout code. A zone->inactive_ratio
> + * on this LRU, maintained by the pageout code. A inactive_ratio
>   * of 3 means 3:1 or 25% of the pages are kept on the inactive list.
>   *
>   * total     target    max
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 91ecca9..74a0eca 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1491,11 +1491,9 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
>  	}
>  	seq_printf(m,
>  		   "\n  node_unreclaimable:  %u"
> -		   "\n  start_pfn:           %lu"
> -		   "\n  node_inactive_ratio: %u",
> +		   "\n  start_pfn:           %lu",
>  		   !pgdat_reclaimable(zone->zone_pgdat),
> -		   zone->zone_start_pfn,
> -		   zone->zone_pgdat->inactive_ratio);
> +		   zone->zone_start_pfn);
>  	seq_putc(m, '\n');
>  }
>  
> -- 
> 1.9.1
> 

-- 
Michal Hocko
SUSE Labs