| 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
| ||
|
Message-ID: <CAMgjq7A405kAAY80xzLxFHuKOcGt7iSYeYYv9G0iZaMHcwrWyA@mail.gmail.com>
Date: Fri, 31 Oct 2025 15:02:04 +0800
From: Kairui Song <ryncsn@...il.com>
To: YoungJun Park <youngjun.park@....com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
Baoquan He <bhe@...hat.com>, Barry Song <baohua@...nel.org>, Chris Li <chrisl@...nel.org>,
Nhat Pham <nphamcs@...il.com>, Johannes Weiner <hannes@...xchg.org>,
Yosry Ahmed <yosry.ahmed@...ux.dev>, David Hildenbrand <david@...hat.com>,
Hugh Dickins <hughd@...gle.com>, Baolin Wang <baolin.wang@...ux.alibaba.com>,
"Huang, Ying" <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 15/19] mm, swap: add folio to swap cache directly on allocation
On Fri, Oct 31, 2025 at 1:56 PM YoungJun Park <youngjun.park@....com> wrote:
>
> On Wed, Oct 29, 2025 at 11:58:41PM +0800, Kairui Song wrote:
> > From: Kairui Song <kasong@...cent.com>
>
> Hello Kairui
>
> > The allocator uses SWAP_HAS_CACHE to pin a swap slot upon allocation.
> > SWAP_HAS_CACHE is being deprecated as it caused a lot of confusion.
> > This pinning usage here can be dropped by adding the folio to swap
> > cache directly on allocation.
> >
> > All swap allocations are folio-based now (except for hibernation), so
> > the swap allocator can always take the folio as the parameter. And now
> > both swap cache (swap table) and swap map are protected by the cluster
> > lock, scanning the map and inserting the folio can be done in the same
> > critical section. This eliminates the time window that a slot is pinned
> > by SWAP_HAS_CACHE, but it has no cache, and avoids touching the lock
> > multiple times.
> >
> > This is both a cleanup and an optimization.
> >
> > Signed-off-by: Kairui Song <kasong@...cent.com>
> > ---
> > include/linux/swap.h | 5 --
> > mm/swap.h | 8 +--
> > mm/swap_state.c | 56 +++++++++++-------
> > mm/swapfile.c | 161 +++++++++++++++++++++------------------------------
> > 4 files changed, 105 insertions(+), 125 deletions(-)
> >
> > diff --git a/include/linux/swap.h b/include/linux/swap.h
> > index ac3caa4c6999..4b4b81fbc6a3 100644
> > --- a/include/linux/swap.h
> > +++ b/include/linux/swap.h
> > @@ -452,7 +452,6 @@ static inline long get_nr_swap_pages(void)
> > }
> >
> > extern void si_swapinfo(struct sysinfo *);
> > -void put_swap_folio(struct folio *folio, swp_entry_t entry);
> > extern int add_swap_count_continuation(swp_entry_t, gfp_t);
> > int swap_type_of(dev_t device, sector_t offset);
> > int find_first_swap(dev_t *device);
> > @@ -534,10 +533,6 @@ static inline void swap_put_entries_direct(swp_entry_t ent, int nr)
> > {
> > }
> >
> > -static inline void put_swap_folio(struct folio *folio, swp_entry_t swp)
> > -{
> > -}
> > -
> > static inline int __swap_count(swp_entry_t entry)
> > {
> > return 0;
> > diff --git a/mm/swap.h b/mm/swap.h
> > index 74c61129d7b7..03694ffa662f 100644
> > --- a/mm/swap.h
> > +++ b/mm/swap.h
> > @@ -277,13 +277,13 @@ void __swapcache_clear_cached(struct swap_info_struct *si,
> > */
> > struct folio *swap_cache_get_folio(swp_entry_t entry);
> > void *swap_cache_get_shadow(swp_entry_t entry);
> > -int swap_cache_add_folio(struct folio *folio, swp_entry_t entry,
> > - void **shadow, bool alloc);
> > void swap_cache_del_folio(struct folio *folio);
> > struct folio *swap_cache_alloc_folio(swp_entry_t entry, gfp_t gfp_flags,
> > struct mempolicy *mpol, pgoff_t ilx,
> > bool *alloced);
> > /* Below helpers require the caller to lock and pass in the swap cluster. */
> > +void __swap_cache_add_folio(struct swap_cluster_info *ci,
> > + struct folio *folio, swp_entry_t entry);
> > void __swap_cache_del_folio(struct swap_cluster_info *ci,
> > struct folio *folio, swp_entry_t entry, void *shadow);
> > void __swap_cache_replace_folio(struct swap_cluster_info *ci,
> > @@ -459,8 +459,8 @@ static inline void *swap_cache_get_shadow(swp_entry_t entry)
> > return NULL;
> > }
> >
> > -static inline int swap_cache_add_folio(struct folio *folio, swp_entry_t entry,
> > - void **shadow, bool alloc)
> > +static inline void *__swap_cache_add_folio(struct swap_cluster_info *ci,
> > + struct folio *folio, swp_entry_t entry)
> > {
> > }
>
> Just a nit,
> void* return nothing.
>
> changed to void (original function prototype is return void)
> or how about just remove If this is not used on !CONFIG_SWAP
Thanks! Yeah it can be just removed, no one is using it when
!CONFIG_SWAP after this commit. Will clean it up.
>
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index d2bcca92b6e0..85d9f99c384f 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -122,6 +122,34 @@ 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.
> > * @folio: The folio to be added.
> > @@ -136,23 +164,18 @@ void *swap_cache_get_shadow(swp_entry_t entry)
> > * The caller also needs to update the corresponding swap_map slots with
> > * SWAP_HAS_CACHE bit to avoid race or conflict.
> > */
> > -int swap_cache_add_folio(struct folio *folio, swp_entry_t entry,
> > - void **shadowp, bool alloc)
> > +static int swap_cache_add_folio(struct folio *folio, swp_entry_t entry,
> > + void **shadowp)
>
> It is also a small thing.
> "alloc" parameter removed then the comment might be updated.
Nice suggestion, will cleanup the comment too.
>
> Thanks,
> Youngjun Park
>
Powered by blists - more mailing lists