linux-kernel - Re: [PATCH v5 15/19] mm, swap: add folio to swap cache directly on allocation

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] [day] [month] [year] [list]

Message-ID: <aUi+A1xjbcUh54wi@MiWiFi-R3L-srv>
Date: Mon, 22 Dec 2025 11:41:55 +0800
From: Baoquan He <bhe@...hat.com>
To: Kairui Song <ryncsn@...il.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
	Barry Song <baohua@...nel.org>, Chris Li <chrisl@...nel.org>,
	Nhat Pham <nphamcs@...il.com>, Yosry Ahmed <yosry.ahmed@...ux.dev>,
	David Hildenbrand <david@...nel.org>,
	Johannes Weiner <hannes@...xchg.org>,
	Youngjun Park <youngjun.park@....com>,
	Hugh Dickins <hughd@...gle.com>,
	Baolin Wang <baolin.wang@...ux.alibaba.com>,
	Ying Huang <ying.huang@...ux.alibaba.com>,
	Kemeng Shi <shikemeng@...weicloud.com>,
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
	"Matthew Wilcox (Oracle)" <willy@...radead.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 15/19] mm, swap: add folio to swap cache directly on
 allocation

On 12/22/25 at 10:42am, Kairui Song wrote:
> On Sat, Dec 20, 2025 at 12:12 PM Baoquan He <bhe@...hat.com> wrote:
> >
> > On 12/20/25 at 03:43am, Kairui Song wrote:
> > > From: Kairui Song <kasong@...cent.com>
...snip...
> > > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > > index 327c051d7cd0..29fa8d313a79 100644
> > > --- a/mm/swap_state.c
> > > +++ b/mm/swap_state.c
> > > @@ -122,35 +122,56 @@ void *swap_cache_get_shadow(swp_entry_t entry)
> > >       return NULL;
> > >  }
> > >
> > > +void __swap_cache_add_folio(struct swap_cluster_info *ci,
> > > +                         struct folio *folio, swp_entry_t entry)
> > > +{
> > > +     unsigned long new_tb;
> > > +     unsigned int ci_start, ci_off, ci_end;
> > > +     unsigned long nr_pages = folio_nr_pages(folio);
> > > +
> > > +     VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> > > +     VM_WARN_ON_ONCE_FOLIO(folio_test_swapcache(folio), folio);
> > > +     VM_WARN_ON_ONCE_FOLIO(!folio_test_swapbacked(folio), folio);
> > > +
> > > +     new_tb = folio_to_swp_tb(folio);
> > > +     ci_start = swp_cluster_offset(entry);
> > > +     ci_off = ci_start;
> > > +     ci_end = ci_start + nr_pages;
> > > +     do {
> > > +             VM_WARN_ON_ONCE(swp_tb_is_folio(__swap_table_get(ci, ci_off)));
> > > +             __swap_table_set(ci, ci_off, new_tb);
> > > +     } while (++ci_off < ci_end);
> > > +
> > > +     folio_ref_add(folio, nr_pages);
> > > +     folio_set_swapcache(folio);
> > > +     folio->swap = entry;
> > > +
> > > +     node_stat_mod_folio(folio, NR_FILE_PAGES, nr_pages);
> > > +     lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages);
> > > +}
> > > +
> > >  /**
> > >   * swap_cache_add_folio - Add a folio into the swap cache.
> >
> > With my understanding, __swap_cache_add_folio() is the pure
> > functionality of adding a folio into swap cache. While
> 
> Hi Baoquan, thanks for the review and suggestion!
> 
> > swap_cache_add_folio() is specifically adding a folio into swap cache
> > during swap in path. Not sure if we can rename them to reflect this
> > clearer. At least from the functiona name and below kernel doc we can't
> > feel that. Maybe:
> > __swap_cache_add_folio() -> swap_cache_add_folio()
> 
> The `__` prefix should stay, I think. This function requires the
> caller to lock the swap cluster.
> 
> > swap_cache_add_folio() -> swap_cache_add_swapin_folio()
> 
> Indeed, my plan is that `swap_cache_add_folio` will be gone soon, we
> should always call `swap_cache_alloc_folio` instead to do the swap
> folio allocation in a unified way, and just remove this.
> 
> Currently we can't do that because shmem and anon have different
> routines for swapin folio allocation. Having a unified
> `swap_cache_alloc_folio` will provide better swapin fallout control
> and cleaner cgroup charging to avoid thrashing, etc.
> 
> Also this helper is currently inexplicitly used by zswap writeback
> too, so adding the swapin keyword seems not accurate.

OK, sounds good to me. Thanks for detailed explanation.