LKML Archive mirror
 help / color / mirror / Atom feed
From: Hugh Dickins <hughd@google.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Mike Kravetz <mike.kravetz@oracle.com>,
	Mike Rapoport <rppt@kernel.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Matthew Wilcox <willy@infradead.org>,
	David Hildenbrand <david@redhat.com>,
	Suren Baghdasaryan <surenb@google.com>,
	Qi Zheng <zhengqi.arch@bytedance.com>,
	Yang Shi <shy828301@gmail.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	Peter Xu <peterx@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Will Deacon <will@kernel.org>, Yu Zhao <yuzhao@google.com>,
	Alistair Popple <apopple@nvidia.com>,
	Ralph Campbell <rcampbell@nvidia.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Steven Price <steven.price@arm.com>,
	SeongJae Park <sj@kernel.org>,
	Lorenzo Stoakes <lstoakes@gmail.com>,
	Huang Ying <ying.huang@intel.com>,
	Naoya Horiguchi <naoya.horiguchi@nec.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Zack Rusin <zackr@vmware.com>, Jason Gunthorpe <jgg@ziepe.ca>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Pasha Tatashin <pasha.tatashin@soleen.com>,
	Miaohe Lin <linmiaohe@huawei.com>,
	Minchan Kim <minchan@kernel.org>,
	Christoph Hellwig <hch@infradead.org>, Song Liu <song@kernel.org>,
	Thomas Hellstrom <thomas.hellstrom@linux.intel.com>,
	Ryan Roberts <ryan.roberts@arm.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: [PATCH v2 30/32] mm/pgtable: delete pmd_trans_unstable() and friends
Date: Thu, 8 Jun 2023 18:50:37 -0700 (PDT)	[thread overview]
Message-ID: <5abdab3-3136-b42e-274d-9c6281bfb79@google.com> (raw)
In-Reply-To: <c1c9a74a-bc5b-15ea-e5d2-8ec34bc921d@google.com>

Delete pmd_trans_unstable, pmd_none_or_trans_huge_or_clear_bad() and
pmd_devmap_trans_unstable(), all now unused.

With mixed feelings, delete all the comments on pmd_trans_unstable().
That was very good documentation of a subtle state, and this series does
not even eliminate that state: but rather, normalizes and extends it,
asking pte_offset_map[_lock]() callers to anticipate failure, without
regard for whether mmap_read_lock() or mmap_write_lock() is held.

Retain pud_trans_unstable(), which has one use in __handle_mm_fault(),
but delete its equivalent pud_none_or_trans_huge_or_dev_or_clear_bad().
While there, move the default arch_needs_pgtable_deposit() definition
up near where pgtable_trans_huge_deposit() and withdraw() are declared.

Signed-off-by: Hugh Dickins <hughd@google.com>
---
 include/linux/pgtable.h | 103 +++-------------------------------------
 mm/khugepaged.c         |   4 --
 2 files changed, 7 insertions(+), 100 deletions(-)

diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index 3fabbb018557..a1326e61d7ee 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -599,6 +599,10 @@ extern void pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
 extern pgtable_t pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
 #endif
 
+#ifndef arch_needs_pgtable_deposit
+#define arch_needs_pgtable_deposit() (false)
+#endif
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 /*
  * This is an implementation of pmdp_establish() that is only suitable for an
@@ -1300,9 +1304,10 @@ static inline int pud_trans_huge(pud_t pud)
 }
 #endif
 
-/* See pmd_none_or_trans_huge_or_clear_bad for discussion. */
-static inline int pud_none_or_trans_huge_or_dev_or_clear_bad(pud_t *pud)
+static inline int pud_trans_unstable(pud_t *pud)
 {
+#if defined(CONFIG_TRANSPARENT_HUGEPAGE) && \
+	defined(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD)
 	pud_t pudval = READ_ONCE(*pud);
 
 	if (pud_none(pudval) || pud_trans_huge(pudval) || pud_devmap(pudval))
@@ -1311,104 +1316,10 @@ static inline int pud_none_or_trans_huge_or_dev_or_clear_bad(pud_t *pud)
 		pud_clear_bad(pud);
 		return 1;
 	}
-	return 0;
-}
-
-/* See pmd_trans_unstable for discussion. */
-static inline int pud_trans_unstable(pud_t *pud)
-{
-#if defined(CONFIG_TRANSPARENT_HUGEPAGE) &&			\
-	defined(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD)
-	return pud_none_or_trans_huge_or_dev_or_clear_bad(pud);
-#else
-	return 0;
 #endif
-}
-
-#ifndef arch_needs_pgtable_deposit
-#define arch_needs_pgtable_deposit() (false)
-#endif
-/*
- * This function is meant to be used by sites walking pagetables with
- * the mmap_lock held in read mode to protect against MADV_DONTNEED and
- * transhuge page faults. MADV_DONTNEED can convert a transhuge pmd
- * into a null pmd and the transhuge page fault can convert a null pmd
- * into an hugepmd or into a regular pmd (if the hugepage allocation
- * fails). While holding the mmap_lock in read mode the pmd becomes
- * stable and stops changing under us only if it's not null and not a
- * transhuge pmd. When those races occurs and this function makes a
- * difference vs the standard pmd_none_or_clear_bad, the result is
- * undefined so behaving like if the pmd was none is safe (because it
- * can return none anyway). The compiler level barrier() is critically
- * important to compute the two checks atomically on the same pmdval.
- *
- * For 32bit kernels with a 64bit large pmd_t this automatically takes
- * care of reading the pmd atomically to avoid SMP race conditions
- * against pmd_populate() when the mmap_lock is hold for reading by the
- * caller (a special atomic read not done by "gcc" as in the generic
- * version above, is also needed when THP is disabled because the page
- * fault can populate the pmd from under us).
- */
-static inline int pmd_none_or_trans_huge_or_clear_bad(pmd_t *pmd)
-{
-	pmd_t pmdval = pmdp_get_lockless(pmd);
-	/*
-	 * !pmd_present() checks for pmd migration entries
-	 *
-	 * The complete check uses is_pmd_migration_entry() in linux/swapops.h
-	 * But using that requires moving current function and pmd_trans_unstable()
-	 * to linux/swapops.h to resolve dependency, which is too much code move.
-	 *
-	 * !pmd_present() is equivalent to is_pmd_migration_entry() currently,
-	 * because !pmd_present() pages can only be under migration not swapped
-	 * out.
-	 *
-	 * pmd_none() is preserved for future condition checks on pmd migration
-	 * entries and not confusing with this function name, although it is
-	 * redundant with !pmd_present().
-	 */
-	if (pmd_none(pmdval) || pmd_trans_huge(pmdval) ||
-		(IS_ENABLED(CONFIG_ARCH_ENABLE_THP_MIGRATION) && !pmd_present(pmdval)))
-		return 1;
-	if (unlikely(pmd_bad(pmdval))) {
-		pmd_clear_bad(pmd);
-		return 1;
-	}
 	return 0;
 }
 
-/*
- * This is a noop if Transparent Hugepage Support is not built into
- * the kernel. Otherwise it is equivalent to
- * pmd_none_or_trans_huge_or_clear_bad(), and shall only be called in
- * places that already verified the pmd is not none and they want to
- * walk ptes while holding the mmap sem in read mode (write mode don't
- * need this). If THP is not enabled, the pmd can't go away under the
- * code even if MADV_DONTNEED runs, but if THP is enabled we need to
- * run a pmd_trans_unstable before walking the ptes after
- * split_huge_pmd returns (because it may have run when the pmd become
- * null, but then a page fault can map in a THP and not a regular page).
- */
-static inline int pmd_trans_unstable(pmd_t *pmd)
-{
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-	return pmd_none_or_trans_huge_or_clear_bad(pmd);
-#else
-	return 0;
-#endif
-}
-
-/*
- * the ordering of these checks is important for pmds with _page_devmap set.
- * if we check pmd_trans_unstable() first we will trip the bad_pmd() check
- * inside of pmd_none_or_trans_huge_or_clear_bad(). this will end up correctly
- * returning 1 but not before it spams dmesg with the pmd_clear_bad() output.
- */
-static inline int pmd_devmap_trans_unstable(pmd_t *pmd)
-{
-	return pmd_devmap(*pmd) || pmd_trans_unstable(pmd);
-}
-
 #ifndef CONFIG_NUMA_BALANCING
 /*
  * Technically a PTE can be PROTNONE even when not doing NUMA balancing but
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index c11db2e78e95..1083f0e38a07 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -946,10 +946,6 @@ static int hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address,
 	return SCAN_SUCCEED;
 }
 
-/*
- * See pmd_trans_unstable() for how the result may change out from
- * underneath us, even if we hold mmap_lock in read.
- */
 static int find_pmd_or_thp_or_none(struct mm_struct *mm,
 				   unsigned long address,
 				   pmd_t **pmd)
-- 
2.35.3


  parent reply	other threads:[~2023-06-09  1:51 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-09  0:54 [PATCH v2 00/32] mm: allow pte_offset_map[_lock]() to fail Hugh Dickins
2023-06-09  1:06 ` [PATCH v2 01/32] mm: use pmdp_get_lockless() without surplus barrier() Hugh Dickins
2023-06-09  1:08 ` [PATCH v2 02/32] mm/migrate: remove cruft from migration_entry_wait()s Hugh Dickins
2023-06-09  1:09 ` [PATCH v2 03/32] mm/pgtable: kmap_local_page() instead of kmap_atomic() Hugh Dickins
2023-06-09  1:10 ` [PATCH v2 04/32] mm/pgtable: allow pte_offset_map[_lock]() to fail Hugh Dickins
2023-07-11  1:23   ` Zi Yan
2023-07-28 13:53   ` Yongqin Liu
2023-07-28 14:05     ` Matthew Wilcox
2023-07-28 16:58       ` Hugh Dickins
2023-08-05 16:06         ` Yongqin Liu
2023-08-05 17:07           ` Matthew Wilcox
2023-08-08  0:29             ` John Hubbard
2023-06-09  1:11 ` [PATCH v2 05/32] mm/filemap: allow pte_offset_map_lock() " Hugh Dickins
2023-07-11  1:34   ` Zi Yan
2023-07-11  5:21     ` Hugh Dickins
2023-06-09  1:12 ` [PATCH v2 06/32] mm/page_vma_mapped: delete bogosity in page_vma_mapped_walk() Hugh Dickins
2023-07-11  1:47   ` Zi Yan
2023-06-09  1:14 ` [PATCH v2 07/32] mm/page_vma_mapped: reformat map_pte() with less indentation Hugh Dickins
2023-07-11  1:56   ` Zi Yan
2023-06-09  1:15 ` [PATCH v2 08/32] mm/page_vma_mapped: pte_offset_map_nolock() not pte_lockptr() Hugh Dickins
2023-06-09  1:17 ` [PATCH v2 09/32] mm/pagewalkers: ACTION_AGAIN if pte_offset_map_lock() fails Hugh Dickins
2023-06-09  1:18 ` [PATCH v2 10/32] mm/pagewalk: walk_pte_range() allow for pte_offset_map() Hugh Dickins
2023-06-09  1:20 ` [PATCH v2 11/32] mm/vmwgfx: simplify pmd & pud mapping dirty helpers Hugh Dickins
2023-06-09  1:21 ` [PATCH v2 12/32] mm/vmalloc: vmalloc_to_page() use pte_offset_kernel() Hugh Dickins
2023-07-10 14:42   ` Mark Brown
2023-07-10 17:18     ` Lorenzo Stoakes
2023-07-10 17:33       ` Mark Brown
2023-07-11  4:34         ` Hugh Dickins
2023-07-11 15:34           ` Mark Brown
2023-07-11 16:13             ` Hugh Dickins
2023-07-11 16:34               ` Mark Brown
2023-07-11 17:57               ` Mark Brown
2023-07-13 11:19                 ` Linux regression tracking #update (Thorsten Leemhuis)
2023-07-20 10:32                 ` Will Deacon
2023-07-20 12:06                   ` Mark Brown
2023-08-08  5:52                     ` Linux regression tracking (Thorsten Leemhuis)
2023-08-08 11:09                       ` Mark Brown
2023-08-11  8:00                         ` Linux regression tracking #update (Thorsten Leemhuis)
2023-07-11 14:48     ` Linux regression tracking #adding (Thorsten Leemhuis)
2023-06-09  1:23 ` [PATCH v2 13/32] mm/hmm: retry if pte_offset_map() fails Hugh Dickins
2023-06-09  1:24 ` [PATCH v2 14/32] mm/userfaultfd: " Hugh Dickins
2023-06-09  1:26 ` [PATCH v2 15/32] mm/userfaultfd: allow pte_offset_map_lock() to fail Hugh Dickins
2023-06-09  1:27 ` [PATCH v2 16/32] mm/debug_vm_pgtable,page_table_check: warn pte map fails Hugh Dickins
2023-06-09  1:29 ` [PATCH v2 17/32] mm/various: give up if pte_offset_map[_lock]() fails Hugh Dickins
2023-06-09  1:30 ` [PATCH v2 18/32] mm/mprotect: delete pmd_none_or_clear_bad_unless_trans_huge() Hugh Dickins
2023-06-09  1:32 ` [PATCH v2 19/32] mm/mremap: retry if either pte_offset_map_*lock() fails Hugh Dickins
2023-06-09  1:34 ` [PATCH v2 20/32] mm/madvise: clean up pte_offset_map_lock() scans Hugh Dickins
2023-06-09  1:35 ` [PATCH v2 21/32] mm/madvise: clean up force_shm_swapin_readahead() Hugh Dickins
2023-06-09  1:36 ` [PATCH v2 22/32] mm/swapoff: allow pte_offset_map[_lock]() to fail Hugh Dickins
2023-06-09  1:37 ` [PATCH v2 23/32] mm/mglru: allow pte_offset_map_nolock() " Hugh Dickins
2023-06-09  1:38 ` [PATCH v2 24/32] mm/migrate_device: allow pte_offset_map_lock() " Hugh Dickins
2023-06-09  1:39 ` [PATCH v2 25/32] mm/gup: remove FOLL_SPLIT_PMD use of pmd_trans_unstable() Hugh Dickins
2023-06-09 18:24   ` Yang Shi
2023-06-09  1:41 ` [PATCH v2 26/32] mm/huge_memory: split huge pmd under one pte_offset_map() Hugh Dickins
2023-06-09  1:42 ` [PATCH v2 27/32] mm/khugepaged: allow pte_offset_map[_lock]() to fail Hugh Dickins
2023-06-09  1:43 ` [PATCH v2 28/32] mm/memory: " Hugh Dickins
2023-06-09 20:06   ` Andrew Morton
2023-06-09 20:11     ` Hugh Dickins
2023-06-12  9:10       ` Ryan Roberts
2023-06-15 23:10   ` [PATCH v2 28/32 fix] mm/memory: allow pte_offset_map[_lock]() to fail: fix Hugh Dickins
2023-06-09  1:45 ` [PATCH v2 29/32] mm/memory: handle_pte_fault() use pte_offset_map_nolock() Hugh Dickins
2023-06-09  1:50 ` Hugh Dickins [this message]
2023-06-09  1:52 ` [PATCH v2 31/32] mm/swap: swap_vma_readahead() do the pte_offset_map() Hugh Dickins
2023-06-12  8:03   ` Huang, Ying
2023-06-14  3:58     ` Hugh Dickins
2023-06-09  1:53 ` [PATCH v2 32/32] perf/core: Allow pte_offset_map() to fail Hugh Dickins
2023-06-20  6:50 ` [PATCH] mm/swapfile: delete outdated pte_offset_map() comment Hugh Dickins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5abdab3-3136-b42e-274d-9c6281bfb79@google.com \
    --to=hughd@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=anshuman.khandual@arm.com \
    --cc=apopple@nvidia.com \
    --cc=axelrasmussen@google.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=david@redhat.com \
    --cc=hch@infradead.org \
    --cc=ira.weiny@intel.com \
    --cc=jgg@ziepe.ca \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linmiaohe@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lstoakes@gmail.com \
    --cc=mgorman@techsingularity.net \
    --cc=mike.kravetz@oracle.com \
    --cc=minchan@kernel.org \
    --cc=naoya.horiguchi@nec.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=peterx@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rcampbell@nvidia.com \
    --cc=rppt@kernel.org \
    --cc=ryan.roberts@arm.com \
    --cc=shy828301@gmail.com \
    --cc=sj@kernel.org \
    --cc=song@kernel.org \
    --cc=steven.price@arm.com \
    --cc=surenb@google.com \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=will@kernel.org \
    --cc=willy@infradead.org \
    --cc=ying.huang@intel.com \
    --cc=yuzhao@google.com \
    --cc=zackr@vmware.com \
    --cc=zhengqi.arch@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).