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 5E276C369B2 for ; Thu, 17 Apr 2025 08:56:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E2CA2280156; Thu, 17 Apr 2025 04:56:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DD860280150; Thu, 17 Apr 2025 04:56:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BB644280156; Thu, 17 Apr 2025 04:56:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 89BF9280150 for ; Thu, 17 Apr 2025 04:56:10 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 3BB8A815C2 for ; Thu, 17 Apr 2025 08:56:12 +0000 (UTC) X-FDA: 83342928984.16.086E4E7 Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by imf06.hostedemail.com (Postfix) with ESMTP id 64BB9180006 for ; Thu, 17 Apr 2025 08:56:10 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=rDi4VqWK; spf=pass (imf06.hostedemail.com: domain of hughd@google.com designates 209.85.216.53 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=1744880170; 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=efcj6vWtk37SkWclqX9qLRQV+dZoYr5AI7grQeYeqEo=; b=1CgnGHjE+VK/cUHnZ7tNmGiQjgLmoDl5+Mu92x2l3tInrb8GOV+ZpzmkCvoq3O82cwUTJr a4Fe7duUYcczLfYxBeg4tEflEfcW4SpYwXrBF+AB8PXsfG+5Fxn/8R4x9ev/+Q7eScrvZ1 Lrc/98fFWeOAZUMKz9rMM9iPlFEfnxY= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=rDi4VqWK; spf=pass (imf06.hostedemail.com: domain of hughd@google.com designates 209.85.216.53 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=1744880170; a=rsa-sha256; cv=none; b=uOfG8WWC2PrKjl+K+WkxxkSV2Hc9aKSRpAAxIbkTLPlOPeQaS0rSX+4t16yFHoD8DDdSke tZP27VM6q6d7BBmUER2Nb5i4zUbPQ3JFCBof2pp8KXJ2o0puWPBnygD+EyBv3euiFVeEYo 9Uopf1tTM9pEV9MEDWSp6XtsnpzPUUI= Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-3014678689aso372202a91.0 for ; Thu, 17 Apr 2025 01:56:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1744880169; x=1745484969; 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=efcj6vWtk37SkWclqX9qLRQV+dZoYr5AI7grQeYeqEo=; b=rDi4VqWKzmR8MNzTdF72iMGLhRYsKZjd3mTQdYlP6eW/1Rn8zKxfuJQZDN2KVZr1fX s1NXeWlxNsOGedjLuCuPEjkdKRF6U5YYlIpdgP2w6c2CPQ5moF9Kt82wcaV96udy9+y0 co5sYVOZ2jaUIOY0mKUhTCkRY64+kaVFqBgvBDVPLz9Scds2bwMNkd+1VXZKvuOff16C EC5EmLHTG0bh3O+43PvOD14o6byQo/k+WmZ7wtLcn1xhVPTSTlzyo5HgHhxosS1JMUMo qXowqdOob59xz4/z9T+uAkfzUs1eBHLbdNYPMRN3Momyp6gwFPd5BMg+U7r++X9lsmbB b7+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1744880169; x=1745484969; 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=efcj6vWtk37SkWclqX9qLRQV+dZoYr5AI7grQeYeqEo=; b=VMEKUNGzdK/Pl0e39kXU3X3i68shcz3sqaJjBook8YWHt+WKwrhORfCIdoGZx47WHS X+pXzPC2fmoxufJqvYljrei8oduRKAetFzqIP4r72SScNeSSql4IPD5diawTVolH5cF8 WeM9G4AunliA7JrcVeMVJcfVCKmGBCKovWr7EHHj7sS02+T8B9l1zb+WCovoOYG20Ooh CRwidI0Fc9gt8wi7yizAdfqUKLHpmze9HbZzewjjLsPrbbbMzzYla8hkL9oVTB91Hcv/ 44Qd+85SAoUTbAg3lFe8xyQZjB1h0JOLiiyUUQGGTSg2zj0C1YtZXwco4aGl6kX+7SzY /SZQ== X-Forwarded-Encrypted: i=1; AJvYcCXe0viI9mrsZUW5X7Cj9vXl19wKOrj8Xo2GyeYn4vqAQ0Rhr7jRIkvubX8hk2igS1E4DizUKIyhYg==@kvack.org X-Gm-Message-State: AOJu0YzcQSLOcvIaDHYsY8VA6zsCde1vJfw8kuzpFldHgMwuV2I6zObW bmfWcePMycoZ4/MKJWo/RUharMzzAmmo3H2uvUTj1TJmtyfPCs3zjMFc4e2qJQ== X-Gm-Gg: ASbGncuiGjS/EDx1SWdyqeGWbDJ+wiDX09gdei5L92N7ZTnjZcZQYWP/DMnwzNfAaVX 3p9SwqkP2wanYS5TNAQm2fxQFbN07wV7rmL59+kzRHajgXTw0DU22SCkSE+XfO2lX7vS3ZPwx1f ZDVwBvkUdFaCbp/3DSjcRZz6+39ur9dpk+FJWAdVCffPMC2RczO5mhbAbNP8OWPk9y+VFl8h35y C/eGKlFnwZffY6NV7pwT1ekyo85QLPKAsM69X+9eyQur/E/ucMAJzF15sElHkchGacVHKr57HuI 3LMkQO+3eir+TArvZju9IF+u8x1N4Xm/zEaE2q4Q5tVlwmYzb+/a05GfPOpVs9kO/CyM9jxw5Cb zYm088wSwCSViaaLB0FYAm8Vm X-Google-Smtp-Source: AGHT+IHjDeiSaw/M3LU4aS8JzNJOh+g4/SSzXupIaI91vW+xXz6rwzm4LXBZfujOCargbQhV7m5Y/w== X-Received: by 2002:a17:90b:28c6:b0:2ff:7031:e380 with SMTP id 98e67ed59e1d1-30863f1b67bmr8001145a91.10.1744880169069; Thu, 17 Apr 2025 01:56:09 -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 98e67ed59e1d1-308545c894fsm2438265a91.1.2025.04.17.01.56.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Apr 2025 01:56:08 -0700 (PDT) Date: Thu, 17 Apr 2025 01:55:54 -0700 (PDT) From: Hugh Dickins To: David Hildenbrand cc: Hugh Dickins , Gavin Guo , linux-mm@kvack.org, akpm@linux-foundation.org, willy@infradead.org, ziy@nvidia.com, linmiaohe@huawei.com, revest@google.com, kernel-dev@igalia.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/huge_memory: fix dereferencing invalid pmd migration entry In-Reply-To: <98d1d195-7821-4627-b518-83103ade56c0@redhat.com> Message-ID: <7d0ef7b5-043b-beca-72a9-6ae98b0d55fb@google.com> References: <20250414072737.1698513-1-gavinguo@igalia.com> <27d13454-280f-4966-b694-d7e58d991547@redhat.com> <6787d0ea-a1b9-08cf-1f48-e361058eec20@google.com> <83f17b85-c9fa-43a0-bec1-22c8565b67ad@redhat.com> <98d1d195-7821-4627-b518-83103ade56c0@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Stat-Signature: gs3kp9qntt9fbby8e7bxmr5udjm18a1e X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 64BB9180006 X-Rspam-User: X-HE-Tag: 1744880170-107443 X-HE-Meta: U2FsdGVkX1+5XWeUlgkNeCXizzllCVuVr8K67ViqYW12+fiRpDbx7AsF1RxDNF/3xzNqYJrJxlVmOsm1rvQudwbHVotmQLyV2dmtn7gtv4z3YI3SjtC3xshxD9514/6AMpcYfYCuqmk7pfXysSAzR5IFHF8XBX0ayhtjS/6iQ9qmNQI6jn5sydXGBz8bVOuiu9tRNODHX6w/NuLQxN4NeU/icvsIuSbZ78SGOl/cgfqaeZZWrTI3SpFCh9INrpRoUpJOQsQvyd9RVgT4QeT4T5NQdqulH7D1ID7NLV5cGvFxv+tGGWZ9XFUBXl5jWmnIk4TnWoa8+l2x8y3HNxKV009NpnrlcnsrMnYZ1xRWq1pLFaDZF7o4Zxzaada6VTINfpxSfULDQFWxkqv+3Ga57EZyJa3UwArfNnPgjUNoknuZKNwsNAnRsbAbOCs94yZabOTzXzt8Eum/CYCBwck4MF48SqFLSIw+3sAHExgsiCxK4BEFZlpZo1Y9gjoPjpfb8nO8a79tTrAboKi7HHQT/RtDpGYaxJCaVfbLZPxKRKQiFh93xtFkb66RBn2/3e65ow9rS+cjf3P7wvTPE8NTZzpk1IiebtD7K1RKOLHNDBVcot074TkpxOy3Nj63wlaC3WNfg2yZNYDY5kwf4qRH38aLb1b6kaaz0g2wN+KJC+fJsXTBrQW91M5Xr6p7Xv7NgUW7c0lKVJUoU/ZPPF//rc+E7mrmc481xGBv01osiaGmj07YNHGn/up2aGi3CXLv8ShDIA8uFn8BLOMJpdRJeI91CsDvS07rr3t9ca2T9TXutvvotVYYbdFM9KRI4U5QxKwLEp+7YEBS83fluWJq5oiMjKXJn78s0To71+0kVOV2tFZMZcPAVV5IwMWzEukMCMfpGOf3IpY419zXx36fzgwlLXu3U7mqYOqYmSZZOXmsKTzWgP9rwQ37T7O37mBp4/mR9kWoq4iEkwN0UlL z1sZi33k 7ZqHd63UN/hlEtSZWjcwzkFKa2LWsYoLThJ81oQYP8CXSOE1XtLuQb3WpbJSi9eYZ3BZOfkHEhTrui1xJ+X9US82SIwq2mFFC+EyrH7ZmpJLu3CfYgPtklOktml+mEn7GxcAG3U6AGlNjlYjXLp+cdSS96aUMTgNASN+krOGJTdMZuQ15pXrFL/qP1Ez6CbdKyflRn9WkQBlChZvxW94umAL8IWE6tu9Mouku2etaQp7O2Csr2XdzLipZy24t60WIRR6Vhs/K4E1TDFJfhT6Nfx3Pmdc6M/vN4zwAdZVQYzUTg600eAINksRh91G748ykfRZQgQgb1r5jjvEkP/BXB2lJ79NRF0pq9iFR5hJnI8C5oLyrBrF0tPzVx9OuKDAZsCmOxpZHYoS1oaY1GFuT3uWAzA== 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 Thu, 17 Apr 2025, David Hildenbrand wrote: > On 17.04.25 09:18, David Hildenbrand wrote: > > On 17.04.25 07:36, Hugh Dickins wrote: > >> On Wed, 16 Apr 2025, David Hildenbrand wrote: > >>> > >>> Why not something like > >>> > >>> struct folio *entry_folio; > >>> > >>> if (folio) { > >>> if (is_pmd_migration_entry(*pmd)) > >>> entry_folio = pfn_swap_entry_folio(pmd_to_swp_entry(*pmd))); > >>> else > >>> entry_folio = pmd_folio(*pmd)); > >>> > >>> if (folio != entry_folio) > >>> return; > >>> } > >> > >> My own preference is to not add unnecessary code: > >> if folio and pmd_migration entry, we're not interested in entry_folio. > >> But yes it could be written in lots of other ways. > > > > While I don't disagree about "not adding unnecessary code" in general, > > in this particular case just looking the folio up properly might be the > > better alternative to reasoning about locking rules with conditional > > input parameters :) > > > > FWIW, I was wondering if we can rework that code, letting the caller to the > checking and getting rid of the folio parameter. Something like this > (incomplete, just to > discuss if we could move the TTU_SPLIT_HUGE_PMD handling). Yes, I too dislike the folio parameter used for a single case, and agree it's better for the caller who chose pmd to check that *pmd fits the folio. I haven't checked your code below, but it looks like a much better way to proceed, using the page_vma_mapped_walk() to get pmd lock and check; and cutting out two or more layers of split_huge_pmd obscurity. Way to go. However... what we want right now is a fix that can easily go to stable: the rearrangements here in 6.15-rc mean, I think, that whatever goes into the current tree will have to be placed differently for stable, no seamless backports; but Gavin's patch (reworked if you insist) can be adapted to stable (differently for different releases) more more easily than the future direction you're proposing here. (Hmm, that may be another reason for preferring the reasoning by folio lock: forgive me if I'm misremembering, but didn't those page migration swapops get renamed, some time around 5.11?) Hugh > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index 2a47682d1ab77..754aa3103e8bf 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -3075,22 +3075,11 @@ static void __split_huge_pmd_locked(struct > vm_area_struct *vma, pmd_t *pmd, > void split_huge_pmd_locked(struct vm_area_struct *vma, unsigned long address, > pmd_t *pmd, bool freeze, struct folio *folio) > { > - VM_WARN_ON_ONCE(folio && !folio_test_pmd_mappable(folio)); > VM_WARN_ON_ONCE(!IS_ALIGNED(address, HPAGE_PMD_SIZE)); > - VM_WARN_ON_ONCE(folio && !folio_test_locked(folio)); > - VM_BUG_ON(freeze && !folio); > - /* > - * When the caller requests to set up a migration entry, we > - * require a folio to check the PMD against. Otherwise, there > - * is a risk of replacing the wrong folio. > - */ > if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd) || > - is_pmd_migration_entry(*pmd)) { > - if (folio && folio != pmd_folio(*pmd)) > - return; > + is_pmd_migration_entry(*pmd)) > __split_huge_pmd_locked(vma, pmd, address, freeze); > - } > } > > void __split_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd, > diff --git a/mm/rmap.c b/mm/rmap.c > index 67bb273dfb80d..bf0320b03d615 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -2291,13 +2291,6 @@ static bool try_to_migrate_one(struct folio *folio, > struct vm_area_struct *vma, > if (flags & TTU_SYNC) > pvmw.flags = PVMW_SYNC; > - /* > - * unmap_page() in mm/huge_memory.c is the only user of migration with > - * TTU_SPLIT_HUGE_PMD and it wants to freeze. > - */ > - if (flags & TTU_SPLIT_HUGE_PMD) > - split_huge_pmd_address(vma, address, true, folio); > - > /* > * For THP, we have to assume the worse case ie pmd for invalidation. > * For hugetlb, it could be much worse if we need to do pud > @@ -2326,6 +2319,14 @@ static bool try_to_migrate_one(struct folio *folio, > struct vm_area_struct *vma, > #ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION > /* PMD-mapped THP migration entry */ > if (!pvmw.pte) { > + if (flags & TTU_SPLIT_HUGE_PMD) { > + split_huge_pmd_locked(vma, pmvw.address, > pvmw.pmd, > + true, NULL); > + ret = false; > + page_vma_mapped_walk_done(&pvmw); > + break; > + } > + > subpage = folio_page(folio, > pmd_pfn(*pvmw.pmd) - folio_pfn(folio)); > VM_BUG_ON_FOLIO(folio_test_hugetlb(folio) || > > > -- > Cheers, > > David / dhildenb