| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0f40850a-13fd-44ae-805a-f2ffb30a44e5@linux.dev>
Date: Tue, 20 Jan 2026 11:30:47 +0800
From: Qi Zheng <qi.zheng@...ux.dev>
To: "David Hildenbrand (Red Hat)" <david@...nel.org>,
linux-kernel@...r.kernel.org
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>, Vlastimil Babka
<vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>
Subject: Re: [PATCH v1 1/2] mm: move pte table reclaim code to memory.c
On 1/20/26 6:07 AM, David Hildenbrand (Red Hat) wrote:
> The pte-table reclaim code is only called from memory.c, while zapping
> pages, and it better also stays that way in the long run. If we ever
> have to call it from other files, we should expose proper high-level
> helpers for zapping if the existing helpers are not good enough.
>
> So, let's move the code over (it's not a lot) and slightly clean it up a
> bit by:
> - Renaming the functions.
> - Dropping the "Check if it is empty PTE page" comment, which is now
> self-explaining given the function name.
> - Making zap_pte_table_if_empty() return whether zapping worked so the
> caller can free it.
> - Adding a comment in pte_table_reclaim_possible().
> - Inlining free_pte() in the last remaining user.
> - In zap_empty_pte_table(), switch from pmdp_get_lcokless() to
> pmd_clear(), we are holding the PMD PT lock.
>
> By moving the code over, compilers can also easily figure out when
> zap_empty_pte_table() does not initialize the pmdval variable, avoiding
> false-positive warnings about the variable possibly not being
> initialized.
>
> Signed-off-by: David Hildenbrand (Red Hat) <david@...nel.org>
> ---
> MAINTAINERS | 1 -
> mm/Makefile | 1 -
> mm/internal.h | 18 -------------
> mm/memory.c | 68 +++++++++++++++++++++++++++++++++++++++++-----
> mm/pt_reclaim.c | 72 -------------------------------------------------
> 5 files changed, 62 insertions(+), 98 deletions(-)
> delete mode 100644 mm/pt_reclaim.c
Reviewed-by: Qi Zheng <zhengqi.arch@...edance.com>
Thanks!
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 11720728d92f2..28e8e28bca3e5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16692,7 +16692,6 @@ R: Shakeel Butt <shakeel.butt@...ux.dev>
> R: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> L: linux-mm@...ck.org
> S: Maintained
> -F: mm/pt_reclaim.c
> F: mm/vmscan.c
> F: mm/workingset.c
>
> diff --git a/mm/Makefile b/mm/Makefile
> index 0d85b10dbdde4..53ca5d4b1929b 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -146,5 +146,4 @@ obj-$(CONFIG_GENERIC_IOREMAP) += ioremap.o
> obj-$(CONFIG_SHRINKER_DEBUG) += shrinker_debug.o
> obj-$(CONFIG_EXECMEM) += execmem.o
> obj-$(CONFIG_TMPFS_QUOTA) += shmem_quota.o
> -obj-$(CONFIG_PT_RECLAIM) += pt_reclaim.o
> obj-$(CONFIG_LAZY_MMU_MODE_KUNIT_TEST) += tests/lazy_mmu_mode_kunit.o
> diff --git a/mm/internal.h b/mm/internal.h
> index 9508dbaf47cd4..ef71a1d9991f2 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1745,24 +1745,6 @@ int walk_page_range_debug(struct mm_struct *mm, unsigned long start,
> unsigned long end, const struct mm_walk_ops *ops,
> pgd_t *pgd, void *private);
>
> -/* pt_reclaim.c */
> -bool try_get_and_clear_pmd(struct mm_struct *mm, pmd_t *pmd, pmd_t *pmdval);
> -void free_pte(struct mm_struct *mm, unsigned long addr, struct mmu_gather *tlb,
> - pmd_t pmdval);
> -void try_to_free_pte(struct mm_struct *mm, pmd_t *pmd, unsigned long addr,
> - struct mmu_gather *tlb);
> -
> -#ifdef CONFIG_PT_RECLAIM
> -bool reclaim_pt_is_enabled(unsigned long start, unsigned long end,
> - struct zap_details *details);
> -#else
> -static inline bool reclaim_pt_is_enabled(unsigned long start, unsigned long end,
> - struct zap_details *details)
> -{
> - return false;
> -}
> -#endif /* CONFIG_PT_RECLAIM */
> -
> void dup_mm_exe_file(struct mm_struct *mm, struct mm_struct *oldmm);
> int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm);
>
> diff --git a/mm/memory.c b/mm/memory.c
> index f2e9e05388743..c3055b2577c27 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1824,11 +1824,68 @@ static inline int do_zap_pte_range(struct mmu_gather *tlb,
> return nr;
> }
>
> +static bool pte_table_reclaim_possible(unsigned long start, unsigned long end,
> + struct zap_details *details)
> +{
> + if (!IS_ENABLED(CONFIG_PT_RECLAIM))
> + return false;
> + /* Only zap if we are allowed to and cover the full page table. */
> + return details && details->reclaim_pt && (end - start >= PMD_SIZE);
> +}
> +
> +static bool zap_empty_pte_table(struct mm_struct *mm, pmd_t *pmd, pmd_t *pmdval)
> +{
> + spinlock_t *pml = pmd_lockptr(mm, pmd);
> +
> + if (!spin_trylock(pml))
> + return false;
> +
> + *pmdval = pmdp_get(pmd);
> + pmd_clear(pmd);
> + spin_unlock(pml);
> + return true;
> +}
> +
> +static bool zap_pte_table_if_empty(struct mm_struct *mm, pmd_t *pmd,
> + unsigned long addr, pmd_t *pmdval)
> +{
> + spinlock_t *pml, *ptl = NULL;
> + pte_t *start_pte, *pte;
> + int i;
> +
> + pml = pmd_lock(mm, pmd);
> + start_pte = pte_offset_map_rw_nolock(mm, pmd, addr, pmdval, &ptl);
> + if (!start_pte)
> + goto out_ptl;
> + if (ptl != pml)
> + spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
> +
> + for (i = 0, pte = start_pte; i < PTRS_PER_PTE; i++, pte++) {
> + if (!pte_none(ptep_get(pte)))
> + goto out_ptl;
> + }
> + pte_unmap(start_pte);
> +
> + pmd_clear(pmd);
> +
> + if (ptl != pml)
> + spin_unlock(ptl);
> + spin_unlock(pml);
> + return true;
> +out_ptl:
> + if (start_pte)
> + pte_unmap_unlock(start_pte, ptl);
> + if (ptl != pml)
> + spin_unlock(pml);
> + return false;
> +}
> +
> static unsigned long zap_pte_range(struct mmu_gather *tlb,
> struct vm_area_struct *vma, pmd_t *pmd,
> unsigned long addr, unsigned long end,
> struct zap_details *details)
> {
> + bool can_reclaim_pt = pte_table_reclaim_possible(addr, end, details);
> bool force_flush = false, force_break = false;
> struct mm_struct *mm = tlb->mm;
> int rss[NR_MM_COUNTERS];
> @@ -1837,7 +1894,6 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
> pte_t *pte;
> pmd_t pmdval;
> unsigned long start = addr;
> - bool can_reclaim_pt = reclaim_pt_is_enabled(start, end, details);
> bool direct_reclaim = true;
> int nr;
>
> @@ -1878,7 +1934,7 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
> * from being repopulated by another thread.
> */
> if (can_reclaim_pt && direct_reclaim && addr == end)
> - direct_reclaim = try_get_and_clear_pmd(mm, pmd, &pmdval);
> + direct_reclaim = zap_empty_pte_table(mm, pmd, &pmdval);
>
> add_mm_rss_vec(mm, rss);
> lazy_mmu_mode_disable();
> @@ -1907,10 +1963,10 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
> }
>
> if (can_reclaim_pt) {
> - if (direct_reclaim)
> - free_pte(mm, start, tlb, pmdval);
> - else
> - try_to_free_pte(mm, pmd, start, tlb);
> + if (direct_reclaim || zap_pte_table_if_empty(mm, pmd, start, &pmdval)) {
> + pte_free_tlb(tlb, pmd_pgtable(pmdval), addr);
> + mm_dec_nr_ptes(mm);
> + }
> }
>
> return addr;
> diff --git a/mm/pt_reclaim.c b/mm/pt_reclaim.c
> deleted file mode 100644
> index 46771cfff8239..0000000000000
> --- a/mm/pt_reclaim.c
> +++ /dev/null
> @@ -1,72 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/hugetlb.h>
> -#include <linux/pgalloc.h>
> -
> -#include <asm/tlb.h>
> -
> -#include "internal.h"
> -
> -bool reclaim_pt_is_enabled(unsigned long start, unsigned long end,
> - struct zap_details *details)
> -{
> - return details && details->reclaim_pt && (end - start >= PMD_SIZE);
> -}
> -
> -bool try_get_and_clear_pmd(struct mm_struct *mm, pmd_t *pmd, pmd_t *pmdval)
> -{
> - spinlock_t *pml = pmd_lockptr(mm, pmd);
> -
> - if (!spin_trylock(pml))
> - return false;
> -
> - *pmdval = pmdp_get_lockless(pmd);
> - pmd_clear(pmd);
> - spin_unlock(pml);
> -
> - return true;
> -}
> -
> -void free_pte(struct mm_struct *mm, unsigned long addr, struct mmu_gather *tlb,
> - pmd_t pmdval)
> -{
> - pte_free_tlb(tlb, pmd_pgtable(pmdval), addr);
> - mm_dec_nr_ptes(mm);
> -}
> -
> -void try_to_free_pte(struct mm_struct *mm, pmd_t *pmd, unsigned long addr,
> - struct mmu_gather *tlb)
> -{
> - pmd_t pmdval;
> - spinlock_t *pml, *ptl = NULL;
> - pte_t *start_pte, *pte;
> - int i;
> -
> - pml = pmd_lock(mm, pmd);
> - start_pte = pte_offset_map_rw_nolock(mm, pmd, addr, &pmdval, &ptl);
> - if (!start_pte)
> - goto out_ptl;
> - if (ptl != pml)
> - spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
> -
> - /* Check if it is empty PTE page */
> - for (i = 0, pte = start_pte; i < PTRS_PER_PTE; i++, pte++) {
> - if (!pte_none(ptep_get(pte)))
> - goto out_ptl;
> - }
> - pte_unmap(start_pte);
> -
> - pmd_clear(pmd);
> -
> - if (ptl != pml)
> - spin_unlock(ptl);
> - spin_unlock(pml);
> -
> - free_pte(mm, addr, tlb, pmdval);
> -
> - return;
> -out_ptl:
> - if (start_pte)
> - pte_unmap_unlock(start_pte, ptl);
> - if (ptl != pml)
> - spin_unlock(pml);
> -}
Powered by blists - more mailing lists