linux-kernel - Re: [PATCH 1/2] mm: vmscan: remove folio_test

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]

Message-ID: <giqlfqh3itraeh5w5aofkrhnae7kglc53f3uzbe357chk22blh@xx2tqnp6dmch>
Date: Mon, 15 Sep 2025 13:00:18 -0700
From: Shakeel Butt <shakeel.butt@...ux.dev>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>
Cc: akpm@...ux-foundation.org, hannes@...xchg.org, david@...hat.com, 
	mhocko@...nel.org, zhengqi.arch@...edance.com, lorenzo.stoakes@...cle.com, 
	hughd@...gle.com, willy@...radead.org, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] mm: vmscan: remove folio_test_private() check in
 pageout()

On Sat, Sep 13, 2025 at 11:24:35AM +0800, Baolin Wang wrote:
> 
> 
> On 2025/9/13 00:13, Shakeel Butt wrote:
> > On Fri, Sep 12, 2025 at 11:45:07AM +0800, Baolin Wang wrote:
> > > Currently, we no longer attempt to write back filesystem folios in pageout(),
> > > and only tmpfs/shmem folios and anonymous swapcache folios can be written back.
> > > Moreover, tmpfs/shmem and swapcache folios do not use the PG_private flag,
> > > which means no fs-private private data is used. Therefore, we can remove the
> > > redundant folio_test_private() checks and related buffer_head release logic.
> > > 
> > > Signed-off-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
> > > ---
> > >   mm/vmscan.c | 16 +---------------
> > >   1 file changed, 1 insertion(+), 15 deletions(-)
> > > 
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index f1fc36729ddd..8056fccb9cc4 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > > @@ -697,22 +697,8 @@ static pageout_t pageout(struct folio *folio, struct address_space *mapping,
> > >   	 * swap_backing_dev_info is bust: it doesn't reflect the
> > >   	 * congestion state of the swapdevs.  Easy to fix, if needed.
> > >   	 */
> > > -	if (!is_page_cache_freeable(folio))
> > > +	if (!is_page_cache_freeable(folio) || !mapping)
> > >   		return PAGE_KEEP;
> > > -	if (!mapping) {
> > > -		/*
> > > -		 * Some data journaling orphaned folios can have
> > > -		 * folio->mapping == NULL while being dirty with clean buffers.
> > > -		 */
> > 
> > Can this case not happen anymore and try_to_free_buffers is not needed?
> 
> For dirty file folios, pageout() will return PAGE_KEEP and put them back on
> the LRU list. So even if mapping = NULL, background workers for writeback
> will continue to handle them, rather than in shrink_folio_list().
> 
> For clean file folios, the !mapping case will be be handled later in
> shrink_folio_list(), please see the following comments:
> 
> /*
>  * If the folio has buffers, try to free the buffer
>  * mappings associated with this folio. If we succeed
>  * we try to free the folio as well.
>  *
>  * We do this even if the folio is dirty.
>  * filemap_release_folio() does not perform I/O, but it
>  * is possible for a folio to have the dirty flag set,
>  * but it is actually clean (all its buffers are clean).
>  * This happens if the buffers were written out directly,
>  * with submit_bh(). ext3 will do this, as well as
>  * the blockdev mapping.  filemap_release_folio() will
>  * discover that cleanness and will drop the buffers
>  * and mark the folio clean - it can be freed.
>  *
>  * Rarely, folios can have buffers and no ->mapping.
>  * These are the folios which were not successfully
>  * invalidated in truncate_cleanup_folio().  We try to
>  * drop those buffers here and if that worked, and the
>  * folio is no longer mapped into process address space
>  * (refcount == 1) it can be freed.  Otherwise, leave
>  * the folio on the LRU so it is swappable.
>  */
> 

Thanks a lot for the explanation, yes this makes sense.

With that, please add:

Acked-by: Shakeel Butt <shakeel.butt@...ux.dev>