From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5227317D6 for ; Sat, 27 Jun 2026 00:04:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782518663; cv=none; b=Ll5c7quVnnCWdsXNXSlMrsbrUBJ5XjKF94U84S3kgb6IlNl7VSp8f2UE07xawmLJcO8jSzhgY2blbEfwbJMLd/F7MOnO2BqmiwI1LP0/f36SPeTvz1nZQcDkAvsfmnhdr+ac47K6t/TFv1yBMNLgNepdEiDCPlWyI9sTI08A048= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782518663; c=relaxed/simple; bh=kBfVJBlGGav0N2qGfFmCpE2tT1PkIQpIN7BLDxSy1Ho=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Nq7XpkpUy9Wj4TTG4+ztVRyoTxJqSVq/6QQSwS1s31DkjK34dvzp6bsAz3YqmOIFk/vnNinvgVjL1L7FIG5eXh1oVgnnkytvKnttSYDwJZ1hDaR0LtReTK7VAmj5Nq68ya1mSnoFhh5SdYHhO3IYyAJX2qiESwyIOVh6GWvwEE0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ALcTkGY7; arc=none smtp.client-ip=209.85.218.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ALcTkGY7" Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-c0868ca8738so250848666b.0 for ; Fri, 26 Jun 2026 17:04:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1782518660; x=1783123460; darn=vger.kernel.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=9aBtjyBhRXO4wcOB3SrA5AZCE73f5ZU7Ka9JtMA+a/E=; b=ALcTkGY7zpI24ke2ZfG+50z/VV2eo2b+v3hzreBwS/PXX+J0Y+j/D/qQCza0lEmh5Q +3vCYCIgpOd4Bum5P4Bl3zOwCqSaHj/MkNn42QFtk4h1E+Fa3ajwKt4bjkmAL9xY6W9U i/mD2pzIHxVhbE36tXtUzrigpTmu3St7Z1L3uPs56kGVFB/cdej/BZk1f5bD142HEs3u SsHqmSmcdD4C/k+13peTqz5y9JyjRiUCHQqZVYoAT9JgLdlGfqi6w9JNfjbZtKZuaMdU noN4FZKOY5Nj4GMI39Av27buImsCrZJzJogDi1O9wyX1r86mYGWnaBE+8nhClLNhfw0X i+fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782518660; x=1783123460; h=user-agent:in-reply-to:content-disposition:mime-version:references :reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=9aBtjyBhRXO4wcOB3SrA5AZCE73f5ZU7Ka9JtMA+a/E=; b=CzugbJZDmeBPTrItQIdZolpMr3P8rybFY+6gD1+DxUyXerO+Em58jsfq3H46Tvrnjt eCTAXLdXmx72rt6AHfMkqQsIPNgTAyQZLx5lxZsK6NNKKmvjuhKfd02pNBMrTpXsitFr nqepTlqbc+hh6Q3Iqx23V7V/bpBgPdWmxmLRJQ9e7M1bODnlDpSOZWhDJsbQDzc0CCI3 R/pNBT3+ElMYZpB25wHPpf9rPeGJLfsuZ5ETusuOJIka/tjyUmxgbDuzWk8LyPMC0VFD 3l1EDNff1y8KGro1K3E78SQF+PtKHL3H8ckg6EgKnYOYGc5/D06gjF+W8TA5vpEyW5jT g7/g== X-Forwarded-Encrypted: i=1; AHgh+RoqFg37DfML7fAoFXyXSN6vmqHFhFE2ELWwSO0COxGl7zDqgTEFLOdBzs8ICCxPKpRnWj0bNVG9eOzg6HQ=@vger.kernel.org X-Gm-Message-State: AOJu0YztZAIWQUBP5J7Bi0aE6EytVMukKLCu9N8zAAZuiGhyuGJHyA4r ag3bNnxcatdPcODwGLrctoIX8WYHXqsHE5pRIJ0Zvyak+VBFJGxr8QgY X-Gm-Gg: AfdE7clxbFIOJ/fK2UgfWqBfBpOLPuFCfR94k9MsX7j6QSHAYh9Ll9ta6iJM8NYBT3B jSUf8elnVEH39mL8iKUVe+qq8HNsm9A/8V5dkfpYeiD33w27cwV7NUKL2hwrPdWISM8+N/BnBiL kg2kmX3s0hzBnv5hYhhN4XtoxZd6BIJj7w06Rlu1LL1gXXUTCuN0Hp4yQz1+9JRDekVhfHNS6ZB usld86baN38CfSoVf931p43+RFi+78yo/q+JxZfmP1SVNSkGKFeXmyYsRSHdzHXHq3myDxyj3bn 7GMAwFnCiuLFq+lysd7lvqz9ZoE2yMITAPkRl9TwftvpmG9TpAgM1i2K+DmOuR6XDOeCfnVAR51 XlUw474v3l7gk8o7SxfS1aB+b02Wp87l4QcO1pFh6qyViBaTX7yCtJ6qvHL2uDxmbvG5t6MwoS2 bef79M3Yi6XcU= X-Received: by 2002:a17:907:94c1:b0:c12:2ce7:a5a8 with SMTP id a640c23a62f3a-c122ce7fcf6mr214659666b.25.1782518659316; Fri, 26 Jun 2026 17:04:19 -0700 (PDT) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-c11fbba8b61sm387627266b.4.2026.06.26.17.04.16 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Fri, 26 Jun 2026 17:04:17 -0700 (PDT) Date: Sat, 27 Jun 2026 00:04:15 +0000 From: Wei Yang To: "David Hildenbrand (Arm)" Cc: Wei Yang , akpm@linux-foundation.org, ljs@kernel.org, riel@surriel.com, liam@infradead.org, vbabka@kernel.org, harry@kernel.org, jannh@google.com, ziy@nvidia.com, sj@kernel.org, balbirs@nvidia.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, Lance Yang Subject: Re: [Patch mm-hotfixes v4] mm/page_vma_mapped: fix device-private PMD handling Message-ID: <20260627000415.xm4w3zzpithptv4i@master> Reply-To: Wei Yang References: <20260624065353.1622-1-richard.weiyang@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) On Fri, Jun 26, 2026 at 12:07:56PM +0200, David Hildenbrand (Arm) wrote: >On 6/24/26 08:53, Wei Yang wrote: >> Commit 65edfda6f3f2 ("mm/rmap: extend rmap and migration support >> device-private entries") introduced the concept of device-private >> PMD entries, but did not correctly update the rmap walk code to >> account for them. >> >> As a result, when page_vma_mapped_walk() encounters device-private >> PMD entries, it takes no action other than to acquire the PMD lock >> and exit. >> >> However this is highly problematic for two reasons - firstly, >> device private entries possess a PFN so check_pmd() needs to be >> called to ensure an overlapping PFN range. >> >> Secondly, and more importantly, if PVMW_MIGRATION is set the >> caller assumes the returned entry is a migration entry, resulting >> in memory corruption when the caller tries to interpret the device >> private entry as such. >> >> In addition, commit 146287290023 ("mm/huge_memory: implement >> device-private THP splitting") allowed device private PMDs to be >> split like THP mappings, but again did not update this code path. >> >> As a result, we might race a PMD split prior to acquiring the PMD >> lock. >> >> This patch addresses all of these issues by invoking check_pmd(), >> ensuring PMVW_MIGRATION is not set and checks whether a split raced >> us we do for PMD THP and migration entries. >> >> Fixes: 65edfda6f3f2 ("mm/rmap: extend rmap and migration support device-private entries") >> Cc: >> Signed-off-by: Wei Yang >> Suggested-by: David Hildenbrand >> Cc: David Hildenbrand >> Cc: Balbir Singh >> Cc: SeongJae Park >> Cc: Zi Yan >> Cc: Lorenzo Stoakes >> Cc: Lance Yang >> >> --- >> v4: >> * refine subject and commit log based on Lorenzo's suggestion >> * put pmd device-private entry handling in its own if branch, >> suggested by Lorenzo >> >> v3: >> * remove cleanup part, only fix the issue for device-private entry >> * refine user effect description based on Lorenzo's suggestion >> >> v2: https://lore.kernel.org/all/20260616063436.20455-1-richard.weiyang@gmail.com/T/#u >> * specify the possible error case of current code and user visible effect >> * besides fix, cleanup the pmd entry handling based on David's suggestion >> >> v1: https://lore.kernel.org/linux-mm/20260508013728.21285-1-richard.weiyang@gmail.com/ >> --- >> mm/page_vma_mapped.c | 20 +++++++++++++++----- >> 1 file changed, 15 insertions(+), 5 deletions(-) >> >> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c >> index 2ccbabfb2cc1..17dff8aab9f9 100644 >> --- a/mm/page_vma_mapped.c >> +++ b/mm/page_vma_mapped.c >> @@ -269,14 +269,24 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) >> /* THP pmd was split under us: handle on pte level */ >> spin_unlock(pvmw->ptl); >> pvmw->ptl = NULL; >> - } else if (!pmd_present(pmde)) { >> - const softleaf_t entry = softleaf_from_pmd(pmde); >> + } else if (pmd_is_device_private_entry(pmde)) { >> + softleaf_t entry; >> + >> + pvmw->ptl = pmd_lock(mm, pvmw->pmd); >> + pmde = *pvmw->pmd; >> + entry = softleaf_from_pmd(pmde); >> >> - if (softleaf_is_device_private(entry)) { >> - pvmw->ptl = pmd_lock(mm, pvmw->pmd); >> + if (likely(softleaf_is_device_private(entry))) { >> + if (pvmw->flags & PVMW_MIGRATION) >> + return not_found(pvmw); >> + if (!check_pmd(softleaf_to_pfn(entry), pvmw)) >> + return not_found(pvmw); >> return true; >> } >> - >> + /* device-private pmd was split under us: handle on pte level */ >> + spin_unlock(pvmw->ptl); >> + pvmw->ptl = NULL; >> + } else if (!pmd_present(pmde)) { >> if ((pvmw->flags & PVMW_SYNC) && >> thp_vma_suitable_order(vma, pvmw->address, >> PMD_ORDER) && > >This is extremely hard to review given the existing crap handling here. I'm >really sorry, but it makes my head hurt (I'm not kidding :) ). > >It's completely unclear why we only have to check for a subset of the cases >after taking the lock. > >Could we simply extend the existing migration pmd handling and leave the >!pmd_present() case for pmd_none()? > >That leaves no question to "which transitions are actually allowed", including >"could we accidentally assume something is a page table when really it isn't". > Consolidate all the cases in one place looks reasonable. And make the logic clearer. > >So what about something like the following? > >The "thp_migration_supported()" is not required when checking for >pmd_is_migration_entry(), as that defaults to "false" when not compiled in. > >Untested: > > >>>From 048ecd33673ec649e168fbbb97749a7c0e344fcd Mon Sep 17 00:00:00 2001 >From: "David Hildenbrand (Arm)" >Date: Fri, 26 Jun 2026 12:03:40 +0200 >Subject: [PATCH] tmp > >Signed-off-by: David Hildenbrand (Arm) >--- > mm/page_vma_mapped.c | 29 +++++++++++++++++------------ > 1 file changed, 17 insertions(+), 12 deletions(-) > >diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c >index 2ccbabfb2cc17..ed2a23a90e8dd 100644 >--- a/mm/page_vma_mapped.c >+++ b/mm/page_vma_mapped.c >@@ -243,21 +243,31 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) > */ > pmde = pmdp_get_lockless(pvmw->pmd); > >- if (pmd_trans_huge(pmde) || pmd_is_migration_entry(pmde)) { >+ if (pmd_trans_huge(pmde) || pmd_is_migration_entry(pmde) || >+ pmd_is_device_private_entry(pmde)) { > pvmw->ptl = pmd_lock(mm, pvmw->pmd); > pmde = *pvmw->pmd; >- if (!pmd_present(pmde)) { >+ if (pmd_is_migration_entry(pmde)) { > softleaf_t entry; > >- if (!thp_migration_supported() || >- !(pvmw->flags & PVMW_MIGRATION)) >+ if (!(pvmw->flags & PVMW_MIGRATION)) > return not_found(pvmw); > entry = softleaf_from_pmd(pmde); >+ if (!check_pmd(softleaf_to_pfn(entry), pvmw)) >+ return not_found(pvmw); >+ return true; >+ } else if (pmd_is_device_private_entry(pmde)) { >+ softleaf_t entry; > >- if (!softleaf_is_migration(entry) || >- !check_pmd(softleaf_to_pfn(entry), pvmw)) >+ if (pvmw->flags & PVMW_MIGRATION) >+ return not_found(pvmw); >+ entry = softleaf_from_pmd(pmde); >+ if (!check_pmd(softleaf_to_pfn(entry), pvmw)) > return not_found(pvmw); > return true; >+ } else if (!pmd_present(pmde) ){ >+ return not_found(pvmw); > } > if (likely(pmd_trans_huge(pmde))) { > if (pvmw->flags & PVMW_MIGRATION) >@@ -270,12 +280,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) > spin_unlock(pvmw->ptl); > pvmw->ptl = NULL; > } else if (!pmd_present(pmde)) { >- const softleaf_t entry = softleaf_from_pmd(pmde); >- >- if (softleaf_is_device_private(entry)) { >- pvmw->ptl = pmd_lock(mm, pvmw->pmd); >- return true; >- } > > if ((pvmw->flags & PVMW_SYNC) && > thp_vma_suitable_order(vma, pvmw->address, >-- >2.43.0 > Will prepare v5 based one this. Thanks. > >-- >Cheers, > >David -- Wei Yang Help you, Help me