| 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: <64ae5450-b74d-452e-a9ae-486c57efa092@linux.alibaba.com>
Date: Tue, 2 Dec 2025 15:34:01 +0800
From: Baolin Wang <baolin.wang@...ux.alibaba.com>
To: Kairui Song <ryncsn@...il.com>, linux-mm@...ck.org
Cc: 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, Kairui Song <kasong@...cent.com>
Subject: Re: [PATCH v3 07/19] mm/shmem: never bypass the swap cache for
SWP_SYNCHRONOUS_IO
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
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?
Powered by blists - more mailing lists