[PATCH v2 2/2] mm/readahead: don't decrease mmap_miss when folio has workingset flags

Liu Shixin posted 2 patches 1 year, 9 months ago
[PATCH v2 2/2] mm/readahead: don't decrease mmap_miss when folio has workingset flags
Posted by Liu Shixin 1 year, 9 months ago
If there are too many folios that are recently evicted in a file, then
they will probably continue to be evicted. In such situation, there is
no positive effect to read-ahead this file since it is only a waste of IO.

The mmap_miss is increased in do_sync_mmap_readahead() and decreased in
both do_async_mmap_readahead() and filemap_map_pages(). In order to skip
read-ahead in above scenario, the mmap_miss have to increased exceed
MMAP_LOTSAMISS. This can be done by stop decreased mmap_miss when folio
has workingset flags. The async path is not to care because in above
scenario, it's hard to run into the async path.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 mm/filemap.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 8df4797c5287..753771310127 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3439,7 +3439,8 @@ static vm_fault_t filemap_map_folio_range(struct vm_fault *vmf,
 		if (PageHWPoison(page + count))
 			goto skip;
 
-		(*mmap_miss)++;
+		if (!folio_test_workingset(folio))
+			(*mmap_miss)++;
 
 		/*
 		 * NOTE: If there're PTE markers, we'll leave them to be
@@ -3488,7 +3489,8 @@ static vm_fault_t filemap_map_order0_folio(struct vm_fault *vmf,
 	if (PageHWPoison(page))
 		return ret;
 
-	(*mmap_miss)++;
+	if (!folio_test_workingset(folio))
+		(*mmap_miss)++;
 
 	/*
 	 * NOTE: If there're PTE markers, we'll leave them to be
-- 
2.25.1
Re: [PATCH v2 2/2] mm/readahead: don't decrease mmap_miss when folio has workingset flags
Posted by Jan Kara 1 year, 8 months ago
On Fri 22-03-24 17:35:55, Liu Shixin wrote:
> If there are too many folios that are recently evicted in a file, then
> they will probably continue to be evicted. In such situation, there is
> no positive effect to read-ahead this file since it is only a waste of IO.
> 
> The mmap_miss is increased in do_sync_mmap_readahead() and decreased in
> both do_async_mmap_readahead() and filemap_map_pages(). In order to skip
> read-ahead in above scenario, the mmap_miss have to increased exceed
> MMAP_LOTSAMISS. This can be done by stop decreased mmap_miss when folio
> has workingset flags. The async path is not to care because in above
> scenario, it's hard to run into the async path.
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
...
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 8df4797c5287..753771310127 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3439,7 +3439,8 @@ static vm_fault_t filemap_map_folio_range(struct vm_fault *vmf,
>  		if (PageHWPoison(page + count))
>  			goto skip;
>  
> -		(*mmap_miss)++;
> +		if (!folio_test_workingset(folio))
> +			(*mmap_miss)++;

Hum, so this means we consider this a 'hit' if the page is completely new
in the page cache or evicted long time ago. OK, makes sense. It would be
nice to add a comment in this direction to explain the condition. Frankly
the whole mmap_miss accounting is broken as I've outlined in my patch
series. But I guess this works as a fixup for your immediate problem and
we can make mmap_miss accounting sensible later. So for now feel free to
add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
[PATCH v3] mm/filemap: don't decrease mmap_miss when folio has workingset flag
Posted by Liu Shixin 1 year, 8 months ago
If there are too many folios that are recently evicted in a file, then
they will probably continue to be evicted. In such situation, there is
no positive effect to read-ahead this file since it is only a waste of IO.

The mmap_miss is increased in do_sync_mmap_readahead() and decreased in
both do_async_mmap_readahead() and filemap_map_pages(). In order to skip
read-ahead in above scenario, the mmap_miss have to increased exceed
MMAP_LOTSAMISS. This can be done by stop decreased mmap_miss when folio
has workingset flag. The async path is not to care because in above
scenario, it's hard to run into the async path.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
v2->v3: Update the title and comment. And add reviewed-by from Jan.

Andrew, please update patch[2] with this new patch, thanks.

 mm/filemap.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 8df4797c5287..780aad026b26 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3439,7 +3439,15 @@ static vm_fault_t filemap_map_folio_range(struct vm_fault *vmf,
 		if (PageHWPoison(page + count))
 			goto skip;
 
-		(*mmap_miss)++;
+		/*
+		 * If there are too many folios that are recently evicted
+		 * in a file, they will probably continue to be evicted.
+		 * In such situation, read-ahead is only a waste of IO.
+		 * Don't decrease mmap_miss in this scenario to make sure
+		 * we can stop read-ahead.
+		 */
+		if (!folio_test_workingset(folio))
+			(*mmap_miss)++;
 
 		/*
 		 * NOTE: If there're PTE markers, we'll leave them to be
@@ -3488,7 +3496,9 @@ static vm_fault_t filemap_map_order0_folio(struct vm_fault *vmf,
 	if (PageHWPoison(page))
 		return ret;
 
-	(*mmap_miss)++;
+	/* See comment of filemap_map_folio_range() */
+	if (!folio_test_workingset(folio))
+		(*mmap_miss)++;
 
 	/*
 	 * NOTE: If there're PTE markers, we'll leave them to be
-- 
2.25.1