From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-133.freemail.mail.aliyun.com (out30-133.freemail.mail.aliyun.com [115.124.30.133]) (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 2E77833FE33 for ; Thu, 4 Dec 2025 12:30:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.133 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764851427; cv=none; b=I8ryjEf58IIunGgDp4asxGF4/k4KeDE9YZH3uGNNmpnCWz6vNAYHsBy6+3DmUqu0QLiSqhs2e6I7WKzJhnBKkCE8BSlvtHH8RGSjQ/fSuDLoiXr+BoqvkPYtXWzIQ0f2r9xX2vAcVeIrEU3DsxIJ+mpIeZJx4HntHOGY+Talbqc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764851427; c=relaxed/simple; bh=408Caw9HjI/wzZGnTlrT4ngSTLCbjxA01OjiSUNHaGQ=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=QZwh6p9En5JyCH1Wi66AbOiYLXCC6Rw88kjCF74duvmdI5k3xUNC46ODemDygAZRKHMQ+mz9lYwBGcKeZyEKACytQ6g5tV9HbjVHmmK6iJ3nexUluvPxWpct9AQ7+UcvhkH7EhLo3bNq0xQn3BTvk/eLDka+DVcYM4iItDBh9GA= 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=hKjyCYxR; arc=none smtp.client-ip=115.124.30.133 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="hKjyCYxR" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1764851415; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=8n8MncgiagcATAavJwnGHHjCaEvtYSrB3GNKh7foFns=; b=hKjyCYxRT59tY1q5lwbIrJ0jFHWcym7sOwSBXWBU73qYkf/7pBPPTrMcvXJ7LUiIzVvFKs0bWVWO4P3ICm7gw7iW2as0pJFb3I0bfZZkTo6cvWM06bmCzQsjUvpQ8AoNWcJJv6ywWGavQRdZDamG+49LgXdGRd0wIM/0uiIg/Ro= Received: from 30.28.193.34(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0Wu4HxQj_1764851411 cluster:ay36) by smtp.aliyun-inc.com; Thu, 04 Dec 2025 20:30:13 +0800 Message-ID: <6d88bc71-bc5c-4f5f-8ca9-5bd0e2677fb6@linux.alibaba.com> Date: Thu, 4 Dec 2025 20:30:11 +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 Cc: linux-mm@kvack.org, 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 References: <20251125-swap-table-p2-v3-0-33f54f707a5c@tencent.com> <20251125-swap-table-p2-v3-7-33f54f707a5c@tencent.com> <64ae5450-b74d-452e-a9ae-486c57efa092@linux.alibaba.com> From: Baolin Wang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 2025/12/3 13:33, Kairui Song wrote: > On Tue, Dec 2, 2025 at 3:34 PM Baolin Wang > wrote: >> >> 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 > > 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. Sounds good. Better to mention this (including your data) in the commit message. With that, please feel free to add: Reviewed-by: Baolin Wang Tested-by: Baolin Wang > 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. OK.