From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-111.freemail.mail.aliyun.com (out30-111.freemail.mail.aliyun.com [115.124.30.111]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B749A4594A for ; Tue, 2 Dec 2025 07:34:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.111 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764660849; cv=none; b=o/OW+O3F4TOPvi7BSZGqF9NWFFpHwKSdUbL17di1vrwaDF/1HZexcOjiHZ2Vd6GCcdW5ET8PUK/wpwh9m6CauzxvzBQJk5sJZojgW61N/yLHAyiId/E49ejCkvsux9pgI7CF8dmqv0fCoTzJHmDMg/1AYEC56gzau3PTCWKxYNc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764660849; c=relaxed/simple; bh=RAWh3Gl3d5ARHdv/V7cLeakY6HbwEc7/ifRruxslk2o=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=lkur8oHkE0h1A0K26DewF3BmzdVl5HU1z+iRRc7HQckftUvBCIMvt07qgUBU0GvODF8AiBqvVsFModlUG2T0UMdX+FujStGM+ZfI9J3kk+8UN9alkuf3hYinKCHRh4DhOn91GFh8F6TgbiOiaJnlRwdsFAxtfAryTrPNvU+E3hg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=pauT2DHV; arc=none smtp.client-ip=115.124.30.111 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="pauT2DHV" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1764660844; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=tCXyg/6ZMlU25GnQZwOaK4qzfWyNJCeNzBA0R6k4M/w=; b=pauT2DHVLOBXVMzLQgZvJxkVzJMYsBeRKO0KTDIDD+40lplfCWfmLqypT+quBacjAC33FZpIuN7ObxCPWmHacERhytilebVQEcLq2WzH7zWM9cePXVlsjMuog0+05TG8JFc33YzGooyvqpEHXJnMJzEybh/GFWADO5gDasVP2Es= Received: from 30.74.144.119(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0WtvV4rn_1764660841 cluster:ay36) by smtp.aliyun-inc.com; Tue, 02 Dec 2025 15:34:02 +0800 Message-ID: <64ae5450-b74d-452e-a9ae-486c57efa092@linux.alibaba.com> Date: Tue, 2 Dec 2025 15:34:01 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 07/19] mm/shmem: never bypass the swap cache for SWP_SYNCHRONOUS_IO To: Kairui Song , linux-mm@kvack.org Cc: Andrew Morton , Baoquan He , Barry Song , Chris Li , Nhat Pham , Yosry Ahmed , David Hildenbrand , Johannes Weiner , Youngjun Park , Hugh Dickins , Ying Huang , Kemeng Shi , Lorenzo Stoakes , "Matthew Wilcox (Oracle)" , linux-kernel@vger.kernel.org, Kairui Song References: <20251125-swap-table-p2-v3-0-33f54f707a5c@tencent.com> <20251125-swap-table-p2-v3-7-33f54f707a5c@tencent.com> From: Baolin Wang In-Reply-To: <20251125-swap-table-p2-v3-7-33f54f707a5c@tencent.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Kairui, On 2025/11/25 03:13, Kairui Song wrote: > From: Kairui Song > > 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 > --- 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?