linux-kernel - Re: [PATCH] mm/vmscan: remove pglist_data->inactive

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 19 Jul 2016 09:34:29 +0200
From:	Michal Hocko <mhocko@...nel.org>
To:	Ganesh Mahendran <opensource.ganesh@...il.com>
Cc:	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	akpm@...ux-foundation.org, mgorman@...hsingularity.net,
	minchan@...nel.org, hannes@...xchg.org, riel@...hat.com,
	dan.j.williams@...el.com, vdavydov@...tuozzo.com,
	kirill.shutemov@...ux.intel.com, cl@...ux.com, hughd@...gle.com
Subject: Re: [PATCH] mm/vmscan: remove pglist_data->inactive_ratio

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@...il.com>
> ---
>  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