From patchwork Tue Jun 25 05:04:38 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hugh Dickins X-Patchwork-Id: 13710599 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 E2764C2BBCA for ; Tue, 25 Jun 2024 05:04:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 40F156B010B; Tue, 25 Jun 2024 01:04:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3976F6B0111; Tue, 25 Jun 2024 01:04:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1EA166B0115; Tue, 25 Jun 2024 01:04:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 022216B010B for ; Tue, 25 Jun 2024 01:04:44 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 7C3BB16168B for ; Tue, 25 Jun 2024 05:04:44 +0000 (UTC) X-FDA: 82268220888.17.FCD1855 Received: from mail-ot1-f49.google.com (mail-ot1-f49.google.com [209.85.210.49]) by imf17.hostedemail.com (Postfix) with ESMTP id CC6994000F for ; Tue, 25 Jun 2024 05:04:42 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=o5hLx7Rw; spf=pass (imf17.hostedemail.com: domain of hughd@google.com designates 209.85.210.49 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=1719291869; 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: references:dkim-signature; bh=9m+h23ZRroOWbviAMXoH/4GFpHDDmKODwkIHKZVa4Zc=; b=6pOG9A21wJZt5AXuSaK4GBWZsHkgCRWtNySRNELjeg+nYa87B726V8NpYA9K4+D6AsBTTQ Nmo8vkkJcdJ4mf0hFfw4O/wyTLjRp2y1ARuPCxtA6keVpkVFlBaO5DEVgSP/HNHARa6Wl2 pfVBE0lE8a2V0jizsBycXwzAMgPlH2w= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719291869; a=rsa-sha256; cv=none; b=35ql9ztKTRGAOFc7cx/ASJasjMY5oa07Gq0zWJjoJ9riSthKcbONUtMO5AW2A6Y+2LxF35 dPt/tkBNOiHQuwaLNppZwf46Y4MXdQ8rYHJIGZAtYGR3ihGPmMy79av36yaAwOlsizHUjH mU1BODtUO+PrJS3W0yMCXf7F6ATsde4= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=o5hLx7Rw; spf=pass (imf17.hostedemail.com: domain of hughd@google.com designates 209.85.210.49 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ot1-f49.google.com with SMTP id 46e09a7af769-6f99555c922so2981900a34.3 for ; Mon, 24 Jun 2024 22:04:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1719291882; x=1719896682; darn=kvack.org; h=mime-version:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to; bh=9m+h23ZRroOWbviAMXoH/4GFpHDDmKODwkIHKZVa4Zc=; b=o5hLx7RwAG84XOiZyuc4asSjq4hvSVYiYazW+GE+XqlJfLtYLhNNRlaanYw+LQJ4eH 0Uz6aMc8o8NgJb80+3YLZ8gP3gQZ42j0LtFik/O5+2tJe3M/5PDU+Vg5WL3rVkuY0fkY akkslISuFnAehDNdkUYDCVbNJggVN3dI5001FNsep/85MyU3+fj7fAPCa9wE7sIjfaiT R6O8dG2dpCBApC2EfepqPQn7fvr9lVWV1ZFi9MFoko57U94rKXhmE88jwEsmVTNtJVVJ B0C1MUFAddM9dYG20RwoaIh2x3DhmZ9ANSiu1Wugx1wgIB0OdIMu9SOJzDQIO4uKOFyF kEGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719291882; x=1719896682; h=mime-version:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=9m+h23ZRroOWbviAMXoH/4GFpHDDmKODwkIHKZVa4Zc=; b=hjlo4pAPf/gmk2RrbelJxPTKW2gDyOdpPv2Zvda7Y0q/ZzegxJULvqW5A8Prc/EV5s Uxx7o7T8KkTpEbbdU3QkruHmk4o0wPf9FNQ9VReB+ARO5hODNuNYs7PDqhk2Ck2/3e7E k9oiQXEY9spztfPucMDmbGBAeb4vGml5RyWaYWK8w2NuUSI9eu3goZUpszFjoyz+qDdu aNmwT7Iv5txvLx1D/mqhvwcuaiDfiwXsPesuTFQNJZoAwq0ljjE9YzVywYt94Fyzx3mr 9TJtWpW1OGjP/RMp7FNP5CodSP+d6+2QMvQVjCsH0Qq4Pv6ujN58PUM7bWO6t2ZfzjnH snbg== X-Forwarded-Encrypted: i=1; AJvYcCVwWnnhRk3NTP76mN5bGzp8Uf+XcqKunSmDC+skagg6668kV04TKhBthUuq7BNYLY0Okq9OtArpCt4cdOdrInih7rw= X-Gm-Message-State: AOJu0Yzozq7hbFRpjMTOCnh3sZIjEoZanPNSTzRwK9PxcMwl2vJ86EXM zEQFtCzCBT8r8wf1dZgXbKTaaAfmEzhvK7ErC3CttGptX8AGxTeakJh6vO9xCA== X-Google-Smtp-Source: AGHT+IEmsKqhNkPn/ShYayTp6XDm4pU1psy1WwQMM/A5zC3uMuOeSq2aqZlSy71bbkkU+cZ3USYCyw== X-Received: by 2002:a05:6870:1c8:b0:254:a7ef:b714 with SMTP id 586e51a60fabf-25d019b26ccmr7409341fac.58.1719291881411; Mon, 24 Jun 2024 22:04:41 -0700 (PDT) Received: from darker.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id 586e51a60fabf-25cd4987b4asm2315518fac.16.2024.06.24.22.04.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Jun 2024 22:04:40 -0700 (PDT) Date: Mon, 24 Jun 2024 22:04:38 -0700 (PDT) From: Hugh Dickins To: Andrew Morton cc: Kefeng Wang , Hugh Dickins , linux-mm@kvack.org, Tony Luck , Miaohe Lin , nao.horiguchi@gmail.com, Matthew Wilcox , David Hildenbrand , Muchun Song , Benjamin LaHaise , jglisse@redhat.com, Zi Yan , Jiaqi Yan , Vishal Moola , Alistair Popple , Jane Chu , Oscar Salvador Subject: [PATCH mm-unstable] mm: migrate: folio_ref_freeze() under xas_lock_irq() Message-ID: <07edaae7-ea5d-b6ae-3a10-f611946f9688@google.com> MIME-Version: 1.0 X-Rspamd-Queue-Id: CC6994000F X-Stat-Signature: eiod14h15dsb669nbcyaibukwkofko37 X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1719291882-111277 X-HE-Meta: U2FsdGVkX18//0jfiDx4sT22xSNHSynB1V5ZVSh3gEFJgaaBehbiRFIP8pNlUULiA+4WVJGocTyxwoEYi7jHZbkkgfErcJ7vzsh82iyQhGQ6LR0Oe0T3Y2LvyCtl7GCh7z6O7zhUXBvnuE81efVFZzHMagz0MEHhqEcdPjrUAhxmEQn1zXbnndTE+/sOm9ITgWMQk+TmJZPOIDZf3iY7iYr5WsPuyWepGsWf5C8rCLJMiU6H/d8fNfL241UzWMyqi6+YKB+GzsIWJf0mKcBJeAwn+Vtlf2zO2tdH5pzokxRhf33c8unvxbeXfe44vrIv1z+jBAybuoobMXxUQZMbhsCNQRjnsyW+pTWp6KIsa1i3PJ7NN9ESFbTLJUgQ4CclpQGGBYLMgtyCFYj0tWlixeblchzyfJDtUTSlcGuOOP37hxWie34A7Jggopy9LKzPpmlna2Y4ZPO2JeZYAYgNUD8Oie2ZYJQfZfBX9AqYv8huvVqwvPcIV+tydtjerQ5EYxDuCaHndyxQpD007YaGl4MdRftZ5itmRyR0JmhSff8/M0aqHqikElpp7V4dExx47TRz7VDyW8ztixTor+Gz+8lLSlmeREkLZJ0wlsiWlEy/ce8DFmSWGfFcNUW+dgI3q8S465L3fTBbEmUlHF545Avp7Smkyvk78Dqx9D3EqaH0r75jsmiZcaGZCMRUyQHUCLY6XMlcVq/ulC8bWOW6NA0D4GN7NkopGVEPw96L3LkK+LMRJ/RP969NHMP0WY64+3oggj9Z8PMrjK06zHh1Vhmy75uL1d6iff3bRyDYfaTZ6vlBCvRQL1PupubN0qsSO18QVmBoQcfkGpZVmRFouIbGL2f8fZSqqFSfEH7SFKxVfvaH2NAK9WHXLpVZ854iIrHs8zE//ivPpo9zm4TmfrfaUk+SZpBHNLzE05hgkTQw081GMJs0/MszVjklAqCkcjAYVxgkzt2j6upKx27 8kCcomVz 4UHwVf8vqmSfg9mARAfzWuiKPPTkfHPXd3nWCUooJwIZgu+DVw2uT1UHfZCwSnWXFf8BjGxAFsoBSILvsaK+lrjcoC+Dt/KUWwawiuDp9jVfPVW1aK7XNfAvT7sv2TIbc33ZOw5F5FUb8DmWHI6wcL6qY4u6VxDhEH4CNbzaU2j994PVvWD1akFMon+f/+HW+Q2sRGrDaUvdPmOkm6v9hqf+MvV5XWqSHMMqmFzTp1TIbTROVtoI4wkLi4R1IpbJ0OKTN6Ubkg4Apq554iIwyk1JUQ/Vi3xJYy5pcfOBeoGULGQxSP3PH9I1AVzAPTVTIGdVr9tDp33QQ7i2Qs7O/S9oI4TPkR1OvPj9jBazw35zj295813TIaKf7iSV9fi38jV0v5l6LIIApuSLSBJZvxhCR3gbUT1ddulvF9SEIAww2wWnUpVY3elDCvJmlmSdk6l8b 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: Commit "mm: migrate: split folio_migrate_mapping()" drew attention to "Note, the folio_ref_freeze() is moved out of xas_lock_irq(), Since the folio is already isolated and locked during migration, so suppose that there is no functional change." That was a mistake. Freezing a folio's refcount to 0 is much like taking a spinlock: see how filemap_get_entry() takes rcu_read_lock() then spins around until the folio is unfrozen. If the task freezing is preempted (or calls cond_resched(), as folio_mc_copy() may do), then it risks deadlock: in my case, one CPU in zap_pte_range() with free_swap_and_cache_nr() trying to reclaim swap while PTL is held, all the other CPUs in reclaim spinning for that PTL. I'm uncertain whether it's necessary for interrupts to be disabled as well as preemption, but since they have to be disabled for the page cache migration, it's much the best to do it all together as before. So revert to folio_ref_freeze() under xas_lock_irq(): but keep the preliminary folio_ref_count() check, which does make sense before trying to copy the folio's data. Use "expected_count" for the expected count throughout. Signed-off-by: Hugh Dickins --- mm/migrate.c | 59 +++++++++++++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/mm/migrate.c b/mm/migrate.c index 27f070f64f27..8beedbb42a93 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -400,8 +400,8 @@ static int folio_expected_refs(struct address_space *mapping, * 2 for folios with a mapping * 3 for folios with a mapping and PagePrivate/PagePrivate2 set. */ -static void __folio_migrate_mapping(struct address_space *mapping, - struct folio *newfolio, struct folio *folio, int expected_cnt) +static int __folio_migrate_mapping(struct address_space *mapping, + struct folio *newfolio, struct folio *folio, int expected_count) { XA_STATE(xas, &mapping->i_pages, folio_index(folio)); struct zone *oldzone, *newzone; @@ -415,13 +415,18 @@ static void __folio_migrate_mapping(struct address_space *mapping, newfolio->mapping = folio->mapping; if (folio_test_swapbacked(folio)) __folio_set_swapbacked(newfolio); - return; + return MIGRATEPAGE_SUCCESS; } oldzone = folio_zone(folio); newzone = folio_zone(newfolio); xas_lock_irq(&xas); + if (!folio_ref_freeze(folio, expected_count)) { + xas_unlock_irq(&xas); + return -EAGAIN; + } + /* Now we know that no one else is looking at the folio */ newfolio->index = folio->index; newfolio->mapping = folio->mapping; @@ -456,7 +461,7 @@ static void __folio_migrate_mapping(struct address_space *mapping, * old folio by unfreezing to one less reference. * We know this isn't the last reference. */ - folio_ref_unfreeze(folio, expected_cnt - nr); + folio_ref_unfreeze(folio, expected_count - nr); xas_unlock(&xas); /* Leave irq disabled to prevent preemption while updating stats */ @@ -504,23 +509,19 @@ static void __folio_migrate_mapping(struct address_space *mapping, } } local_irq_enable(); + + return MIGRATEPAGE_SUCCESS; } int folio_migrate_mapping(struct address_space *mapping, struct folio *newfolio, struct folio *folio, int extra_count) { - int expected_cnt = folio_expected_refs(mapping, folio) + extra_count; + int expected_count = folio_expected_refs(mapping, folio) + extra_count; - if (!mapping) { - if (folio_ref_count(folio) != expected_cnt) - return -EAGAIN; - } else { - if (!folio_ref_freeze(folio, expected_cnt)) - return -EAGAIN; - } + if (folio_ref_count(folio) != expected_count) + return -EAGAIN; - __folio_migrate_mapping(mapping, newfolio, folio, expected_cnt); - return MIGRATEPAGE_SUCCESS; + return __folio_migrate_mapping(mapping, newfolio, folio, expected_count); } EXPORT_SYMBOL(folio_migrate_mapping); @@ -534,16 +535,18 @@ int migrate_huge_page_move_mapping(struct address_space *mapping, XA_STATE(xas, &mapping->i_pages, folio_index(src)); int ret, expected_count = folio_expected_refs(mapping, src); - if (!folio_ref_freeze(src, expected_count)) + if (folio_ref_count(src) != expected_count) return -EAGAIN; ret = folio_mc_copy(dst, src); - if (unlikely(ret)) { - folio_ref_unfreeze(src, expected_count); + if (unlikely(ret)) return ret; - } xas_lock_irq(&xas); + if (!folio_ref_freeze(src, expected_count)) { + xas_unlock_irq(&xas); + return -EAGAIN; + } dst->index = src->index; dst->mapping = src->mapping; @@ -660,24 +663,18 @@ static int __migrate_folio(struct address_space *mapping, struct folio *dst, struct folio *src, void *src_private, enum migrate_mode mode) { - int ret, expected_cnt = folio_expected_refs(mapping, src); + int ret, expected_count = folio_expected_refs(mapping, src); - if (!mapping) { - if (folio_ref_count(src) != expected_cnt) - return -EAGAIN; - } else { - if (!folio_ref_freeze(src, expected_cnt)) - return -EAGAIN; - } + if (folio_ref_count(src) != expected_count) + return -EAGAIN; ret = folio_mc_copy(dst, src); - if (unlikely(ret)) { - if (mapping) - folio_ref_unfreeze(src, expected_cnt); + if (unlikely(ret)) return ret; - } - __folio_migrate_mapping(mapping, dst, src, expected_cnt); + ret = __folio_migrate_mapping(mapping, dst, src, expected_count); + if (ret != MIGRATEPAGE_SUCCESS) + return ret; if (src_private) folio_attach_private(dst, folio_detach_private(src));