Received-SPF: pass (google.com: domain of linux-kernel+bounces-117370-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Mon, 25 Mar 2024 16:30:46 +0100
From: Jan Kara <jack@suse.cz>
To: Liu Shixin <liushixin2@huawei.com>
Cc: Jan Kara <jack@suse.cz>, Matthew Wilcox <willy@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH v2 2/2] mm/readahead: don't decrease mmap_miss when folio
 has workingset flags
Message-ID: <20240325153046.5m5xls7emjfa45da@quack3>
References: <20240322093555.226789-1-liushixin2@huawei.com>
 <20240322093555.226789-3-liushixin2@huawei.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240322093555.226789-3-liushixin2@huawei.com>

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