Re: [RFC] mm/vmscan.c: avoid possible long latency caused by too_many_isolated()

[Xing Zhengjun <zhengjun.xing@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4255 bytes --]

Hi,

    In the system with very few file pages (nr_active_file + 
nr_inactive_file < 100), it is easy to reproduce "nr_isolated_file > 
nr_inactive_file",  then too_many_isolated return true, 
shrink_inactive_list enter "msleep(100)", the long latency will happen.

The test case to reproduce it is very simple: allocate many huge 
pages(near the DRAM size), then do free, repeat the same operation many 
times.
In the test case, the system with very few file pages (nr_active_file + 
nr_inactive_file < 100), I have dumpped the numbers of 
active/inactive/isolated file pages during the whole test(see in the 
attachments) , in shrink_inactive_list "too_many_isolated" is very easy 
to return true, then enter "msleep(100)",in "too_many_isolated" 
sc->gfp_mask is 0x342cca ("_GFP_IO" and "__GFP_FS" is masked) , it is 
also very easy to enter “inactive >>=3”, then “isolated > inactive” will 
be true.

So I  have a proposal to set a threshold number for the total file pages 
to ignore the system with very few file pages, and then bypass the 100ms 
sleep.
It is hard to set a perfect number for the threshold, so I just give an 
example of "256" for it.

I appreciate it if you can give me your suggestion/comments. Thanks.


On 4/16/2021 10:35 AM, zhengjun.xing@linux.intel.com wrote:
> From: Zhengjun Xing <zhengjun.xing@linux.intel.com>
> 
> In the system with very few file pages, it is easy to reproduce
> "nr_isolated_file > nr_inactive_file",  then too_many_isolated return true,
> shrink_inactive_list enter "msleep(100)", the long latency will happen.
> The test case to reproduce it is very simple, allocate a lot of huge pages
> (near the DRAM size), then do free, repeat the same operation many times.
> There is a 3/10 rate to reproduce the issue. In the test, sc-> gfp_mask
> is 0x342cca ("_GFP_IO" and "__GFP_FS" is masked),it is more easy to enter
> “inactive >>=3”, then “isolated > inactive” will easy to be true.
> 
> So I  have a proposal to set a threshold number for the total file pages
> to ignore the system with very few file pages, and then bypass the 100ms
> sleep. It is hard to set a perfect number for the threshold, so I
> just give an example of "256" for it, need more inputs for it.
> 
> Signed-off-by: Zhengjun Xing <zhengjun.xing@linux.intel.com>
> ---
>   mm/vmscan.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 562e87cbd7a1..a1926463455c 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -168,6 +168,7 @@ struct scan_control {
>    * From 0 .. 200.  Higher means more swappy.
>    */
>   int vm_swappiness = 60;
> +int lru_list_threshold = SWAP_CLUSTER_MAX << 3;
>   
>   static void set_task_reclaim_state(struct task_struct *task,
>   				   struct reclaim_state *rs)
> @@ -1785,7 +1786,7 @@ int isolate_lru_page(struct page *page)
>   static int too_many_isolated(struct pglist_data *pgdat, int file,
>   		struct scan_control *sc)
>   {
> -	unsigned long inactive, isolated;
> +	unsigned long inactive, isolated, active, nr_lru_pages;
>   
>   	if (current_is_kswapd())
>   		return 0;
> @@ -1796,11 +1797,13 @@ static int too_many_isolated(struct pglist_data *pgdat, int file,
>   	if (file) {
>   		inactive = node_page_state(pgdat, NR_INACTIVE_FILE);
>   		isolated = node_page_state(pgdat, NR_ISOLATED_FILE);
> +		active = node_page_state(pgdat, NR_ACTIVE_FILE);
>   	} else {
>   		inactive = node_page_state(pgdat, NR_INACTIVE_ANON);
>   		isolated = node_page_state(pgdat, NR_ISOLATED_ANON);
> +		active = node_page_state(pgdat, NR_ACTIVE_ANON);
>   	}
> -
> +	nr_lru_pages = inactive + active;
>   	/*
>   	 * GFP_NOIO/GFP_NOFS callers are allowed to isolate more pages, so they
>   	 * won't get blocked by normal direct-reclaimers, forming a circular
> @@ -1809,6 +1812,10 @@ static int too_many_isolated(struct pglist_data *pgdat, int file,
>   	if ((sc->gfp_mask & (__GFP_IO | __GFP_FS)) == (__GFP_IO | __GFP_FS))
>   		inactive >>= 3;
>   
> +	if (isolated > inactive)
> +		if (nr_lru_pages < lru_list_threshold)
> +			return 0;
> +
>   	return isolated > inactive;
>   }
>   
> 

-- 
Zhengjun Xing

[-- Attachment #2: proc-vmstat-anon.png --]
[-- Type: image/png, Size: 4897 bytes --]

[-- Attachment #3: proc-vmstat-file.png --]
[-- Type: image/png, Size: 6729 bytes --]

[-- Attachment #4: proc-vmstat-file-all.png --]
[-- Type: image/png, Size: 6738 bytes --]