Message-ID: <683f254b-2a6a-46e2-b11b-e0514c1b076f@lucifer.local>
Date: Mon, 28 Apr 2025 21:12:42 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: David Hildenbrand <david@...hat.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org,
        intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-trace-kernel@...r.kernel.org,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
        Borislav Petkov <bp@...en8.de>, "H. Peter Anvin" <hpa@...or.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Tvrtko Ursulin <tursulin@...ulin.net>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        Vlastimil Babka <vbabka@...e.cz>, Jann Horn <jannh@...gle.com>,
        Pedro Falcato <pfalcato@...e.de>, Peter Xu <peterx@...hat.com>
Subject: Re: [PATCH v1 06/11] x86/mm/pat: remove old pfnmap tracking interface

On Fri, Apr 25, 2025 at 10:17:10AM +0200, David Hildenbrand wrote:
> We can now get rid of the old interface along with get_pat_info() and
> follow_phys().
>
> Signed-off-by: David Hildenbrand <david@...hat.com>

Oh what a glorious glorious screen of red I see before me... deleted code
is the best code!

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>

> ---
>  arch/x86/mm/pat/memtype.c | 147 --------------------------------------
>  include/linux/pgtable.h   |  66 -----------------
>  2 files changed, 213 deletions(-)
>
> diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c
> index c011d8dd8f441..668ebf0065157 100644
> --- a/arch/x86/mm/pat/memtype.c
> +++ b/arch/x86/mm/pat/memtype.c
> @@ -933,119 +933,6 @@ static void free_pfn_range(u64 paddr, unsigned long size)
>  		memtype_free(paddr, paddr + size);
>  }
>
> -static int follow_phys(struct vm_area_struct *vma, unsigned long *prot,
> -		resource_size_t *phys)
> -{
> -	struct follow_pfnmap_args args = { .vma = vma, .address = vma->vm_start };
> -
> -	if (follow_pfnmap_start(&args))
> -		return -EINVAL;
> -
> -	/* Never return PFNs of anon folios in COW mappings. */
> -	if (!args.special) {
> -		follow_pfnmap_end(&args);
> -		return -EINVAL;
> -	}
> -
> -	*prot = pgprot_val(args.pgprot);
> -	*phys = (resource_size_t)args.pfn << PAGE_SHIFT;
> -	follow_pfnmap_end(&args);
> -	return 0;
> -}
> -
> -static int get_pat_info(struct vm_area_struct *vma, resource_size_t *paddr,
> -		pgprot_t *pgprot)
> -{
> -	unsigned long prot;
> -
> -	VM_WARN_ON_ONCE(!(vma->vm_flags & VM_PAT));
> -
> -	/*
> -	 * We need the starting PFN and cachemode used for track_pfn_remap()
> -	 * that covered the whole VMA. For most mappings, we can obtain that
> -	 * information from the page tables. For COW mappings, we might now
> -	 * suddenly have anon folios mapped and follow_phys() will fail.
> -	 *
> -	 * Fallback to using vma->vm_pgoff, see remap_pfn_range_notrack(), to
> -	 * detect the PFN. If we need the cachemode as well, we're out of luck
> -	 * for now and have to fail fork().
> -	 */
> -	if (!follow_phys(vma, &prot, paddr)) {
> -		if (pgprot)
> -			*pgprot = __pgprot(prot);
> -		return 0;
> -	}
> -	if (is_cow_mapping(vma->vm_flags)) {
> -		if (pgprot)
> -			return -EINVAL;
> -		*paddr = (resource_size_t)vma->vm_pgoff << PAGE_SHIFT;
> -		return 0;
> -	}
> -	WARN_ON_ONCE(1);
> -	return -EINVAL;
> -}
> -
> -int track_pfn_copy(struct vm_area_struct *dst_vma,
> -		struct vm_area_struct *src_vma, unsigned long *pfn)
> -{
> -	const unsigned long vma_size = src_vma->vm_end - src_vma->vm_start;
> -	resource_size_t paddr;
> -	pgprot_t pgprot;
> -	int rc;
> -
> -	if (!(src_vma->vm_flags & VM_PAT))
> -		return 0;
> -
> -	/*
> -	 * Duplicate the PAT information for the dst VMA based on the src
> -	 * VMA.
> -	 */
> -	if (get_pat_info(src_vma, &paddr, &pgprot))
> -		return -EINVAL;
> -	rc = reserve_pfn_range(paddr, vma_size, &pgprot, 1);
> -	if (rc)
> -		return rc;
> -
> -	/* Reservation for the destination VMA succeeded. */
> -	vm_flags_set(dst_vma, VM_PAT);
> -	*pfn = PHYS_PFN(paddr);
> -	return 0;
> -}
> -
> -void untrack_pfn_copy(struct vm_area_struct *dst_vma, unsigned long pfn)
> -{
> -	untrack_pfn(dst_vma, pfn, dst_vma->vm_end - dst_vma->vm_start, true);
> -	/*
> -	 * Reservation was freed, any copied page tables will get cleaned
> -	 * up later, but without getting PAT involved again.
> -	 */
> -}
> -
> -/*
> - * prot is passed in as a parameter for the new mapping. If the vma has
> - * a linear pfn mapping for the entire range, or no vma is provided,
> - * reserve the entire pfn + size range with single reserve_pfn_range
> - * call.
> - */
> -int track_pfn_remap(struct vm_area_struct *vma, pgprot_t *prot,
> -		    unsigned long pfn, unsigned long addr, unsigned long size)
> -{
> -	resource_size_t paddr = (resource_size_t)pfn << PAGE_SHIFT;
> -
> -	/* reserve the whole chunk starting from paddr */
> -	if (!vma || (addr == vma->vm_start
> -				&& size == (vma->vm_end - vma->vm_start))) {
> -		int ret;
> -
> -		ret = reserve_pfn_range(paddr, size, prot, 0);
> -		if (ret == 0 && vma)
> -			vm_flags_set(vma, VM_PAT);
> -		return ret;
> -	}
> -
> -	return pfnmap_sanitize_pgprot(pfn, size, prot);
> -}
> -
>  int pfnmap_sanitize_pgprot(unsigned long pfn, unsigned long size, pgprot_t *prot)
>  {
>  	resource_size_t paddr = (resource_size_t)pfn << PAGE_SHIFT;
> @@ -1082,40 +969,6 @@ void pfnmap_untrack(unsigned long pfn, unsigned long size)
>  	free_pfn_range(paddr, size);
>  }
>
> -/*
> - * untrack_pfn is called while unmapping a pfnmap for a region.
> - * untrack can be called for a specific region indicated by pfn and size or
> - * can be for the entire vma (in which case pfn, size are zero).
> - */
> -void untrack_pfn(struct vm_area_struct *vma, unsigned long pfn,
> -		 unsigned long size, bool mm_wr_locked)
> -{
> -	resource_size_t paddr;
> -
> -	if (vma && !(vma->vm_flags & VM_PAT))
> -		return;
> -
> -	/* free the chunk starting from pfn or the whole chunk */
> -	paddr = (resource_size_t)pfn << PAGE_SHIFT;
> -	if (!paddr && !size) {
> -		if (get_pat_info(vma, &paddr, NULL))
> -			return;
> -		size = vma->vm_end - vma->vm_start;
> -	}
> -	free_pfn_range(paddr, size);
> -	if (vma) {
> -		if (mm_wr_locked)
> -			vm_flags_clear(vma, VM_PAT);
> -		else
> -			__vm_flags_mod(vma, 0, VM_PAT);
> -	}
> -}
> -
> -void untrack_pfn_clear(struct vm_area_struct *vma)
> -{
> -	vm_flags_clear(vma, VM_PAT);
> -}
> -
>  pgprot_t pgprot_writecombine(pgprot_t prot)
>  {
>  	pgprot_set_cachemode(&prot, _PAGE_CACHE_MODE_WC);
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 898a3ab195578..0ffc6b9339182 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -1489,17 +1489,6 @@ static inline pmd_t pmd_swp_clear_soft_dirty(pmd_t pmd)
>   * vmf_insert_pfn.
>   */
>
> -/*
> - * track_pfn_remap is called when a _new_ pfn mapping is being established
> - * by remap_pfn_range() for physical range indicated by pfn and size.
> - */
> -static inline int track_pfn_remap(struct vm_area_struct *vma, pgprot_t *prot,
> -				  unsigned long pfn, unsigned long addr,
> -				  unsigned long size)
> -{
> -	return 0;
> -}
> -
>  static inline int pfnmap_sanitize_pgprot(unsigned long pfn, unsigned long size,
>  		pgprot_t *prot)
>  {
> @@ -1515,55 +1504,7 @@ static inline int pfnmap_track(unsigned long pfn, unsigned long size,
>  static inline void pfnmap_untrack(unsigned long pfn, unsigned long size)
>  {
>  }
> -
> -/*
> - * track_pfn_copy is called when a VM_PFNMAP VMA is about to get the page
> - * tables copied during copy_page_range(). Will store the pfn to be
> - * passed to untrack_pfn_copy() only if there is something to be untracked.
> - * Callers should initialize the pfn to 0.
> - */
> -static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
> -		struct vm_area_struct *src_vma, unsigned long *pfn)
> -{
> -	return 0;
> -}
> -
> -/*
> - * untrack_pfn_copy is called when a VM_PFNMAP VMA failed to copy during
> - * copy_page_range(), but after track_pfn_copy() was already called. Can
> - * be called even if track_pfn_copy() did not actually track anything:
> - * handled internally.
> - */
> -static inline void untrack_pfn_copy(struct vm_area_struct *dst_vma,
> -		unsigned long pfn)
> -{
> -}
> -
> -/*
> - * untrack_pfn is called while unmapping a pfnmap for a region.
> - * untrack can be called for a specific region indicated by pfn and size or
> - * can be for the entire vma (in which case pfn, size are zero).
> - */
> -static inline void untrack_pfn(struct vm_area_struct *vma,
> -			       unsigned long pfn, unsigned long size,
> -			       bool mm_wr_locked)
> -{
> -}
> -
> -/*
> - * untrack_pfn_clear is called in the following cases on a VM_PFNMAP VMA:
> - *
> - * 1) During mremap() on the src VMA after the page tables were moved.
> - * 2) During fork() on the dst VMA, immediately after duplicating the src VMA.
> - */
> -static inline void untrack_pfn_clear(struct vm_area_struct *vma)
> -{
> -}
>  #else
> -extern int track_pfn_remap(struct vm_area_struct *vma, pgprot_t *prot,
> -			   unsigned long pfn, unsigned long addr,
> -			   unsigned long size);
> -
>  /**
>   * pfnmap_sanitize_pgprot - sanitize the pgprot for a pfn range
>   * @pfn: the start of the pfn range
> @@ -1603,13 +1544,6 @@ int pfnmap_track(unsigned long pfn, unsigned long size, pgprot_t *prot);
>   * un-doing any reservation.
>   */
>  void pfnmap_untrack(unsigned long pfn, unsigned long size);
> -extern int track_pfn_copy(struct vm_area_struct *dst_vma,
> -		struct vm_area_struct *src_vma, unsigned long *pfn);
> -extern void untrack_pfn_copy(struct vm_area_struct *dst_vma,
> -		unsigned long pfn);
> -extern void untrack_pfn(struct vm_area_struct *vma, unsigned long pfn,
> -			unsigned long size, bool mm_wr_locked);
> -extern void untrack_pfn_clear(struct vm_area_struct *vma);
>  #endif
>
>  #ifdef CONFIG_MMU
> --
> 2.49.0
>

Powered by blists - more mailing lists