From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 130E2C021B8 for ; Tue, 4 Mar 2025 09:48:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 876B4280001; Tue, 4 Mar 2025 04:48:06 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 825BC6B0095; Tue, 4 Mar 2025 04:48:06 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6ECDA280001; Tue, 4 Mar 2025 04:48:06 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 52B226B0092 for ; Tue, 4 Mar 2025 04:48:06 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id D7344B7D03 for ; Tue, 4 Mar 2025 09:48:05 +0000 (UTC) X-FDA: 83183392530.18.FC37CB8 Received: from mail-ot1-f48.google.com (mail-ot1-f48.google.com [209.85.210.48]) by imf04.hostedemail.com (Postfix) with ESMTP id 0A61740003 for ; Tue, 4 Mar 2025 09:48:03 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=z8OWMD3K; spf=pass (imf04.hostedemail.com: domain of hughd@google.com designates 209.85.210.48 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741081684; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=8HOHs5ZstfJ5o568Xa5zyB7db/o03y3YlavBZxWfyZQ=; b=yh1rx6dYYi3Y1kCIBB1QoL9y4vGewt24yx8+y2L2MF71D1pCfhJ57127bHI9XpT1sgEzjA yJRD3IPRWMf4BULMsJPwI3ln2ouTtz9XFFeln2x4RltLm7QCNM7nX/B6mu9ADD9gS4f/Gx Cn7AAPfZdmxEwzRigmw86bcr27Xbe7g= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=z8OWMD3K; spf=pass (imf04.hostedemail.com: domain of hughd@google.com designates 209.85.210.48 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741081684; a=rsa-sha256; cv=none; b=lBfh3gF+uNpMLbmtenrN4L33uDZj9XTekJ1Eq2pRaATUEKrdibikr6/ReANwkItqpZjH/r gXFsb5bC23H2hHwA2vbprmwYcy4u1QKpS58qqsqqsk/eiwv2QiDVCDxobW+tO0nKj93eCn w3ntPVnsAUFtLy+NlrhBmiIvkpf30jU= Received: by mail-ot1-f48.google.com with SMTP id 46e09a7af769-72a1c42c70dso17416a34.1 for ; Tue, 04 Mar 2025 01:48:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1741081683; x=1741686483; darn=kvack.org; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=8HOHs5ZstfJ5o568Xa5zyB7db/o03y3YlavBZxWfyZQ=; b=z8OWMD3KNTsAjNBPsvzt7gKG9CJoEZz6gnRm0kT4gLgyrYluGmlp4vx1nOjGUxCAxG iN1H+KMWAm7vvwXvv1zXZsm+Scb2w+H47rrtrmMkb6ZF0sESc3HoOMe04UUpls5ohgv8 2R9873UHhAPRCoMh62oo+6s8oyrtVC8BnxKl9Fe//4cMFPYSCFTPB76TMmVl23nwPh4j d024aTAAcwg7D5FXBXqZ1Yh6l06ETDTOHjGvxfmd8pDTM/3NBhv3Hej4QsSC3h7UFsT/ Idi+kzbm9EEUMH+NcH43I7zsk8qkxuUSsoMfh3+bzmXvEoo9ND7e6ZtQEjR0oVwxDKWy v1xA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741081683; x=1741686483; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=8HOHs5ZstfJ5o568Xa5zyB7db/o03y3YlavBZxWfyZQ=; b=qmL3851hDkzgZ5bG2M1t5BsGAvJ1rFCBEgJa4YnvCQL0oHsNsk7PuSHmeRyRy45Gkp oa3KY3FI1GXPPkCL3oLmU0AaYosRQmSj51sE0enU0QBfQZQhEYT+XxynXXu+OQyiGVId gYA5FLQz83LTGafPKTP7X2dYz06t52N+5zK+i3tcCMCBsepx5LAJQimNogY3Czl6hjFI soUjq9GxQYJDR5Mvkcg61w1y77nLCfYDqEKqxZLpfXQEHUHPMVh6rLBfbm6Gft3nhoiT PLmPFI3PggANu016P64f5wvyN5gbtde9Nif8PweEaZONvbY1CgNMuTSqunbwDUtYOGNS RZxQ== X-Forwarded-Encrypted: i=1; AJvYcCVmdz5/z+D5iUgjmcqgRzSLcUQEroW++ldxrgyj61WhqoKz6Gq3s4f4IUYuLd+nN+k1THN9uVD2AQ==@kvack.org X-Gm-Message-State: AOJu0YzVpZMdSrJc4a1yIl3ZZfNbzkuvG0OARNCrxA02RpO+/5/CXEDe sJLvAI4Ui0SYoCJhF9sd3/NYOGdKbTRmSiSyM/1+juHcoiIKTgJDPORJbP2mcQ== X-Gm-Gg: ASbGncv1ttAa1AtpmqiieJmBkcXOOEP/jwuxhVEEsbETTasBBHzXVyFdPDyQiW8h3Yb 4G75xU/7GpsIGL23pPAF95934wrTA8mLjsnrSamfogDIj3y3FzPAXm+SfSB9Z4l3uoE2sOxmXia PWPpGBJgsUx3mhOKnjt2hE9qGj6L4WkVa0Kf+yYMNK7ztiUVMlEArk7fICeSg06/Npoe6VihC3C TPi8/Scv9gBrqHYTrLsuE/pfLXFvHA7e8J5+VW3f8VWP+T+unmMCjkl/0yZwumqy2ynejj/Pw7W mNFNvugAe8r0xa9oNt7zbc52+aBzPuLwXIHE2yRNrheRHYyV84Y4X9tBgoYid+50s/NoP0HdiL9 Pw5qkjQ71CfD6Mkg0rWcHsqRSsJo5 X-Google-Smtp-Source: AGHT+IGdMEBMjK0xYYVwrNxOf6XqwE8AYZxLYxN/SKUpsg+bPBtoAzbZx90I3qdmj2oNt7FksMqoPQ== X-Received: by 2002:a05:6830:91f:b0:728:b9d3:4330 with SMTP id 46e09a7af769-72a14efd17bmr1806518a34.1.1741081682851; Tue, 04 Mar 2025 01:48:02 -0800 (PST) Received: from darker.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-728afd0061dsm2071715a34.27.2025.03.04.01.47.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Mar 2025 01:48:01 -0800 (PST) Date: Tue, 4 Mar 2025 01:47:46 -0800 (PST) From: Hugh Dickins To: Zi Yan cc: Liu Shixin , Baolin Wang , linux-mm@kvack.org, Andrew Morton , Barry Song , David Hildenbrand , Kefeng Wang , Lance Yang , Ryan Roberts , Matthew Wilcox , Hugh Dickins , Charan Teja Kalla , linux-kernel@vger.kernel.org, Shivank Garg Subject: Re: [PATCH v2] mm/migrate: fix shmem xarray update during migration In-Reply-To: <20250228174953.2222831-1-ziy@nvidia.com> Message-ID: <23d65532-859a-e88f-9c24-06a6c7ff4006@google.com> References: <20250228174953.2222831-1-ziy@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspam-User: X-Rspamd-Queue-Id: 0A61740003 X-Rspamd-Server: rspam09 X-Stat-Signature: d7faywmiqxceusawtw1h5fan1ipxpac8 X-HE-Tag: 1741081683-690747 X-HE-Meta: U2FsdGVkX1+O7He/Vv+g3cweslZhR0ktLetcXQjToRT1FS/1r1JFcEHyPsNDPpcyLFqN7wAHbBTOZ3kLC3b/7eflSapaNAPmQhgpe3eX0lhzErRcwueFQ3hnHz15Lma3KPgRI7e+eKfYqYaL2ho5/WnZLWLiQklcqJoHFjTUdMfOOKhQhIcXYFcVt5DzbSTLGZ8mxdLAkPobzSyDh9ICiiOOPltiK7sTdtEKxeI0ori472XqHKrqK7Ozy6cAJ58SK4kvIqQkvea6CcMUHeKMyM515OgQIUuNRvNL9KiuGKYPdc+Bi6oEeI4qmiIa0mk2wDlDUhe7XqiTXDV6M3m8o3M8CAlIuuggCJUbI13R3NzreLzv+aTCvsksdVpGGt5AhFn2MC1KwcJXGiD7WU8aJxQ7MOssN0W3K1jIv8/orqvi7gJn/aSvgzSCc7Psh1u6ZgY+3e5861M5YBYaXfQZIekdMF81RwFbRaIAe7scj3Hsq24KRB7E6Lh8Y7KgKKM/otAVdaLIoGvmTF35QGoMnP+ENSVgW3H0azGZmjFJc9c151yQrrPN64tFuVx8++jMPja4SVI2FrhnE2GXEmwtYAYPpyIUGOmkV4Tb1TY/rr2GGd6o1v8DDwJKtTsJLgLIXVsh0rqrfI+Px5dsoa3uSn8Pc3uqFIL26ES1r2oIJQHFDBNHONSWYgtQepPl0puTujumT0tRkup+H/0I7iYr+YRs6MCEx39S4cMojUBmddJDhXE2DEGUb8WO9noV4mEMjUX4zHGCcGatRMtEepoqc5G3KeFuaIBb5lgFmUBWHWa0qRTIlnNOUWp7moiJopg2jNVEHHwPP5sbNuiNMh6hfmkOCTM6rTq+9HMsOxkFVdy9ULVawy/AuSfwmmCmS3CxQnNysAIJX+FDMYdXbfRs318tR3eGRepm889btFdU26TDI8EE2uHDRAHsd2BJtFDCj6K18n40DS3GicJtmcE WXD/JrU7 e3D8VuCAiy94mOrW1hJBrRgCiYqEv103+OL8SU9cZa2g+bAbRm15WH4KMYGQh4GBQvxZ3QL/y3BgIBRRBpiVdJuZfLM1YiER/YigF6Pm/UmK1TxZ/TwAuiGNxtNXWd1rcRRTDRlggzAb7xJnWWEcEEoMYlGWtt/qJKukN8zD4d39MB7rH/A7oQ7fC+zoAB/3lwXHLFQquaL/bSEarIQz0BQaD9Y/vNVJAiMw/c8huFuKrbkGd5yyEucTIBUSQN5fC9lg74D0a9Fr6F08vUGnTWWbRGd8fg/oDz71c46jqBfV0OF3KxhibMNiBmlpmavIEmsw8gYSBLcyVOX/qCdLsEt7OHYKPv1X/AhfAvTMV17jQiVzcl0lPbDTDdsHOffdotTcDTZNsflZ116n5ZnXmGOAOcZom0wa3S8ksDi0NbwHBOO144dYf0hUeFP7fqX8e7aE3 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, 28 Feb 2025, Zi Yan wrote: > Pagecache uses multi-index entries for large folio, so does shmem. Only > swap cache still stores multiple entries for a single large folio. > Commit fc346d0a70a1 ("mm: migrate high-order folios in swap cache correctly") > fixed swap cache but got shmem wrong by storing multiple entries for > a large shmem folio. Fix it by storing a single entry for a shmem > folio. > > Fixes: fc346d0a70a1 ("mm: migrate high-order folios in swap cache correctly") > Reported-by: Liu Shixin > Closes: https://lore.kernel.org/all/28546fb4-5210-bf75-16d6-43e1f8646080@huawei.com/ > Signed-off-by: Zi Yan > Reviewed-by: Shivank Garg It's a great find (I think), and your commit message is okay: but unless I'm much mistaken, NAK to the patch itself. First, I say "(I think)" there, because I don't actually know what the loop writing the same folio nr times to the multi-index entry does to the xarray: I can imagine it as being completely harmless, just nr times more work than was needed. But I guess it does something bad, since Matthew was horrified, and we have all found that your patch appears to improve behaviour (or at least improve behaviour in the context of your folio_split() series: none of us noticed a problem before that, but it may be that your new series is widening our exposure to existing bugs). Maybe your orginal patch, with the shmem_mapping(mapping) check there, was good, and it's only wrong when changed to !folio_test_anon(folio); but TBH I find it too confusing, with the conditionals the way they are. See my preferred alternative below. The vital point is that multi-index entries are not used in swap cache: whether the folio in question orginates from anon or from shmem. And it's easier to understand once you remember that a shmem folio is never in both page cache and swap cache at the same time (well, there may be an instant of transition from one to other while that folio is held locked) - once it's in swap cache, folio->mapping is NULL and it's no longer recognizable as from a shmem mapping. The way I read your patch originally, I thought it meant that shmem folios go into the swap cache as multi-index, but anon folios do not; which seemed a worrying mixture to me. But crashes on the VM_BUG_ON_PAGE(entry != folio, entry) in __delete_from_swap_cache() yesterday (with your patch in) led me to see how add_to_swap_cache() inserts multiple non-multi-index entries, whether for anon or for shmem. If this patch really is needed in old releases, then I suspect that mm/huge_memory.c needs correction there too; but let me explain in a response to your folio_split() series. > --- > mm/migrate.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 365c6daa8d1b..2c9669135a38 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -524,7 +524,11 @@ static int __folio_migrate_mapping(struct address_space *mapping, > folio_set_swapcache(newfolio); > newfolio->private = folio_get_private(folio); > } > - entries = nr; > + /* shmem uses high-order entry */ > + if (!folio_test_anon(folio)) > + entries = 1; > + else > + entries = nr; > } else { > VM_BUG_ON_FOLIO(folio_test_swapcache(folio), folio); > entries = 1; > -- > 2.47.2 NAK to that patch above, here's how I think it should be: Signed-off-by: Hugh Dickins --- mm/migrate.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mm/migrate.c b/mm/migrate.c index fb19a18892c8..822776819ca6 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -518,12 +518,12 @@ static int __folio_migrate_mapping(struct address_space *mapping, if (folio_test_anon(folio) && folio_test_large(folio)) mod_mthp_stat(folio_order(folio), MTHP_STAT_NR_ANON, 1); folio_ref_add(newfolio, nr); /* add cache reference */ - if (folio_test_swapbacked(folio)) { + if (folio_test_swapbacked(folio)) __folio_set_swapbacked(newfolio); - if (folio_test_swapcache(folio)) { - folio_set_swapcache(newfolio); - newfolio->private = folio_get_private(folio); - } + + if (folio_test_swapcache(folio)) { + folio_set_swapcache(newfolio); + newfolio->private = folio_get_private(folio); entries = nr; } else { VM_BUG_ON_FOLIO(folio_test_swapcache(folio), folio); -- 2.43.0