| 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: <CAMgjq7DcpMgLjX1m=+4SM=zMe5+H4qDLqdOUGnYGNBQ_HsKw-w@mail.gmail.com>
Date: Wed, 3 Dec 2025 13:33:02 +0800
From: Kairui Song <ryncsn@...il.com>
To: Baolin Wang <baolin.wang@...ux.alibaba.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>, 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>,
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 v3 07/19] mm/shmem: never bypass the swap cache for SWP_SYNCHRONOUS_IO
On Tue, Dec 2, 2025 at 3:34 PM Baolin Wang
<baolin.wang@...ux.alibaba.com> wrote:
>
> Hi Kairui,
>
> On 2025/11/25 03:13, Kairui Song wrote:
> > From: Kairui Song <kasong@...cent.com>
> >
> > Now the overhead of the swap cache is trivial to none, bypassing the
> > swap cache is no longer a valid optimization.
> >
> > We have removed the cache bypass swapin for anon memory, now do the same
> > for shmem. Many helpers and functions can be dropped now.
> >
> > Signed-off-by: Kairui Song <kasong@...cent.com>
> > ---
>
> I'm glad to see we can remove the skip swapcache logic. I did a quick
> test, testing 1G shmem sequential swap-in with 64K mTHP and 2M mTHP, and
> I observed a slight drop, which could also be fluctuation. Can you also
> perform some measurements?
>
> 64K shmem mTHP:
> W/ patchset W/o patchset
> 154 ms 148 ms
>
> 2M shmem mTHP
> W/ patchset W/o patchset
> 117 ms 115 ms
Hi Baolin,
Thanks for testing! This patch (7/19) is still an intermediate step,
so we are still updating both swap_map and swap table with higher
overhead. And even with that, the performance change looks small
(~1-4% in the result you posted), close to noise level.
And after this whole series, the double update is *partially* dropped,
so the performance is almost identical to before:
tmpfs with transparent_hugepage_tmpfs=within_size, 3 test run on my machine:
Before [PATCH 7/19] [PATCH 19/19]
5.99s 6.29s 6.08s (~1%)
Note we are still using swap_map so there are double lookups
everywhere in this series, and I added more WARN_ON checks. Swap is
complex so being cautious is better I think. I've also mentioned
another valkey slight performance drop in the cover letter due to
this, which is also tiny and will be improved a lot in phase 3 by
removing swap_map and the double lookup, as demonstrated before:
https://lore.kernel.org/linux-mm/20250514201729.48420-1-ryncsn@gmail.com/
Last time I tested that branch it was a clear optimization for shmem,
some of the optimizations in that series were split or merged
separately so the performance may look go up / down in some
intermediate steps, the final result is good.
swap_cgroup_ctrl will be gone too, even later maybe though.
>
> Anyway I still hope we can remove the skip swapcache logic. The changes
> look good to me with one nit as below. Thanks for your work.
>
> > mm/shmem.c | 65 +++++++++++++++++------------------------------------------
> > mm/swap.h | 4 ----
> > mm/swapfile.c | 35 +++++++++-----------------------
> > 3 files changed, 27 insertions(+), 77 deletions(-)
> >
> > diff --git a/mm/shmem.c b/mm/shmem.c
> > index ad18172ff831..d08248fd67ff 100644
> > --- a/mm/shmem.c
> > +++ b/mm/shmem.c
> > @@ -2001,10 +2001,9 @@ static struct folio *shmem_swap_alloc_folio(struct inode *inode,
> > swp_entry_t entry, int order, gfp_t gfp)
> > {
> > struct shmem_inode_info *info = SHMEM_I(inode);
> > + struct folio *new, *swapcache;
> > int nr_pages = 1 << order;
> > - struct folio *new;
> > gfp_t alloc_gfp;
> > - void *shadow;
> >
> > /*
> > * We have arrived here because our zones are constrained, so don't
> > @@ -2044,34 +2043,19 @@ static struct folio *shmem_swap_alloc_folio(struct inode *inode,
> > goto fallback;
> > }
> >
> > - /*
> > - * Prevent parallel swapin from proceeding with the swap cache flag.
> > - *
> > - * Of course there is another possible concurrent scenario as well,
> > - * that is to say, the swap cache flag of a large folio has already
> > - * been set by swapcache_prepare(), while another thread may have
> > - * already split the large swap entry stored in the shmem mapping.
> > - * In this case, shmem_add_to_page_cache() will help identify the
> > - * concurrent swapin and return -EEXIST.
> > - */
> > - if (swapcache_prepare(entry, nr_pages)) {
> > + swapcache = swapin_folio(entry, new);
> > + if (swapcache != new) {
> > folio_put(new);
> > - new = ERR_PTR(-EEXIST);
> > - /* Try smaller folio to avoid cache conflict */
> > - goto fallback;
> > + if (!swapcache) {
> > + /*
> > + * The new folio is charged already, swapin can
> > + * only fail due to another raced swapin.
> > + */
> > + new = ERR_PTR(-EEXIST);
> > + goto fallback;
> > + }
> > }
> > -
> > - __folio_set_locked(new);
> > - __folio_set_swapbacked(new);
> > - new->swap = entry;
> > -
> > - memcg1_swapin(entry, nr_pages);
> > - shadow = swap_cache_get_shadow(entry);
> > - if (shadow)
> > - workingset_refault(new, shadow);
> > - folio_add_lru(new);
> > - swap_read_folio(new, NULL);
> > - return new;
> > + return swapcache;
> > fallback:
> > /* Order 0 swapin failed, nothing to fallback to, abort */
> > if (!order)
> > @@ -2161,8 +2145,7 @@ static int shmem_replace_folio(struct folio **foliop, gfp_t gfp,
> > }
> >
> > static void shmem_set_folio_swapin_error(struct inode *inode, pgoff_t index,
> > - struct folio *folio, swp_entry_t swap,
> > - bool skip_swapcache)
> > + struct folio *folio, swp_entry_t swap)
> > {
> > struct address_space *mapping = inode->i_mapping;
> > swp_entry_t swapin_error;
> > @@ -2178,8 +2161,7 @@ static void shmem_set_folio_swapin_error(struct inode *inode, pgoff_t index,
> >
> > nr_pages = folio_nr_pages(folio);
> > folio_wait_writeback(folio);
> > - if (!skip_swapcache)
> > - swap_cache_del_folio(folio);
> > + swap_cache_del_folio(folio);
> > /*
> > * Don't treat swapin error folio as alloced. Otherwise inode->i_blocks
> > * won't be 0 when inode is released and thus trigger WARN_ON(i_blocks)
> > @@ -2279,7 +2261,6 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
> > softleaf_t index_entry;
> > struct swap_info_struct *si;
> > struct folio *folio = NULL;
> > - bool skip_swapcache = false;
> > int error, nr_pages, order;
> > pgoff_t offset;
> >
> > @@ -2322,7 +2303,6 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
> > folio = NULL;
> > goto failed;
> > }
> > - skip_swapcache = true;
> > } else {
> > /* Cached swapin only supports order 0 folio */
> > folio = shmem_swapin_cluster(swap, gfp, info, index);
> > @@ -2378,9 +2358,8 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
> > * and swap cache folios are never partially freed.
> > */
> > folio_lock(folio);
> > - if ((!skip_swapcache && !folio_test_swapcache(folio)) ||
> > - shmem_confirm_swap(mapping, index, swap) < 0 ||
> > - folio->swap.val != swap.val) {
> > + if (!folio_matches_swap_entry(folio, swap) ||
> > + shmem_confirm_swap(mapping, index, swap) < 0) {
>
> We should still keep the '!folio_test_swapcache(folio)' check here?
Thanks for the review, this one is OK because folio_test_swapcache is
included in folio_matches_swap_entry already.
Powered by blists - more mailing lists