[PATCH v2 04/11] mm: convert VM_PFNMAP tracking to pfnmap_track() + pfnmap_untrack()
David Hildenbrand
david at redhat.com
Tue May 13 09:10:45 UTC 2025
On 12.05.25 18:42, Lorenzo Stoakes wrote:
> On Mon, May 12, 2025 at 02:34:17PM +0200, David Hildenbrand wrote:
>> Let's use our new interface. In remap_pfn_range(), we'll now decide
>> whether we have to track (full VMA covered) or only lookup the
>> cachemode (partial VMA covered).
>>
>> Remember what we have to untrack by linking it from the VMA. When
>> duplicating VMAs (e.g., splitting, mremap, fork), we'll handle it similar
>> to anon VMA names, and use a kref to share the tracking.
>>
>> Once the last VMA un-refs our tracking data, we'll do the untracking,
>> which simplifies things a lot and should sort our various issues we saw
>> recently, for example, when partially unmapping/zapping a tracked VMA.
>>
>> This change implies that we'll keep tracking the original PFN range even
>> after splitting + partially unmapping it: not too bad, because it was
>> not working reliably before. The only thing that kind-of worked before
>> was shrinking such a mapping using mremap(): we managed to adjust the
>> reservation in a hacky way, now we won't adjust the reservation but
>> leave it around until all involved VMAs are gone.
>>
>> If that ever turns out to be an issue, we could hook into VM splitting
>> code and split the tracking; however, that adds complexity that might
>> not be required, so we'll keep it simple for now.
>>
>> Acked-by: Ingo Molnar <mingo at kernel.org> # x86 bits
>> Signed-off-by: David Hildenbrand <david at redhat.com>
>
> Other than small nit below,
>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes at oracle.com>
>
>> ---
>> include/linux/mm_inline.h | 2 +
>> include/linux/mm_types.h | 11 ++++++
>> mm/memory.c | 82 +++++++++++++++++++++++++++++++--------
>> mm/mmap.c | 5 ---
>> mm/mremap.c | 4 --
>> mm/vma_init.c | 50 ++++++++++++++++++++++++
>> 6 files changed, 129 insertions(+), 25 deletions(-)
>>
>> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
>> index f9157a0c42a5c..89b518ff097e6 100644
>> --- a/include/linux/mm_inline.h
>> +++ b/include/linux/mm_inline.h
>> @@ -447,6 +447,8 @@ static inline bool anon_vma_name_eq(struct anon_vma_name *anon_name1,
>>
>> #endif /* CONFIG_ANON_VMA_NAME */
>>
>> +void pfnmap_track_ctx_release(struct kref *ref);
>> +
>> static inline void init_tlb_flush_pending(struct mm_struct *mm)
>> {
>> atomic_set(&mm->tlb_flush_pending, 0);
>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>> index 15808cad2bc1a..3e934dc6057c4 100644
>> --- a/include/linux/mm_types.h
>> +++ b/include/linux/mm_types.h
>> @@ -763,6 +763,14 @@ struct vma_numab_state {
>> int prev_scan_seq;
>> };
>>
>> +#ifdef __HAVE_PFNMAP_TRACKING
>> +struct pfnmap_track_ctx {
>> + struct kref kref;
>> + unsigned long pfn;
>> + unsigned long size; /* in bytes */
>> +};
>> +#endif
>> +
>> /*
>> * Describes a VMA that is about to be mmap()'ed. Drivers may choose to
>> * manipulate mutable fields which will cause those fields to be updated in the
>> @@ -900,6 +908,9 @@ struct vm_area_struct {
>> struct anon_vma_name *anon_name;
>> #endif
>> struct vm_userfaultfd_ctx vm_userfaultfd_ctx;
>> +#ifdef __HAVE_PFNMAP_TRACKING
>> + struct pfnmap_track_ctx *pfnmap_track_ctx;
>> +#endif
>> } __randomize_layout;
>>
>> #ifdef CONFIG_NUMA
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 064fc55d8eab9..4cf4adb0de266 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -1371,7 +1371,7 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
>> struct mm_struct *dst_mm = dst_vma->vm_mm;
>> struct mm_struct *src_mm = src_vma->vm_mm;
>> struct mmu_notifier_range range;
>> - unsigned long next, pfn = 0;
>> + unsigned long next;
>> bool is_cow;
>> int ret;
>>
>> @@ -1381,12 +1381,6 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
>> if (is_vm_hugetlb_page(src_vma))
>> return copy_hugetlb_page_range(dst_mm, src_mm, dst_vma, src_vma);
>>
>> - if (unlikely(src_vma->vm_flags & VM_PFNMAP)) {
>> - ret = track_pfn_copy(dst_vma, src_vma, &pfn);
>> - if (ret)
>> - return ret;
>> - }
>> -
>> /*
>> * We need to invalidate the secondary MMU mappings only when
>> * there could be a permission downgrade on the ptes of the
>> @@ -1428,8 +1422,6 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
>> raw_write_seqcount_end(&src_mm->write_protect_seq);
>> mmu_notifier_invalidate_range_end(&range);
>> }
>> - if (ret && unlikely(src_vma->vm_flags & VM_PFNMAP))
>> - untrack_pfn_copy(dst_vma, pfn);
>> return ret;
>> }
>>
>> @@ -1924,9 +1916,6 @@ static void unmap_single_vma(struct mmu_gather *tlb,
>> if (vma->vm_file)
>> uprobe_munmap(vma, start, end);
>>
>> - if (unlikely(vma->vm_flags & VM_PFNMAP))
>> - untrack_pfn(vma, 0, 0, mm_wr_locked);
>> -
>> if (start != end) {
>> if (unlikely(is_vm_hugetlb_page(vma))) {
>> /*
>> @@ -2872,6 +2861,36 @@ int remap_pfn_range_notrack(struct vm_area_struct *vma, unsigned long addr,
>> return error;
>> }
>>
>> +#ifdef __HAVE_PFNMAP_TRACKING
>> +static inline struct pfnmap_track_ctx *pfnmap_track_ctx_alloc(unsigned long pfn,
>> + unsigned long size, pgprot_t *prot)
>> +{
>> + struct pfnmap_track_ctx *ctx;
>> +
>> + if (pfnmap_track(pfn, size, prot))
>> + return ERR_PTR(-EINVAL);
>> +
>> + ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
>> + if (unlikely(!ctx)) {
>> + pfnmap_untrack(pfn, size);
>> + return ERR_PTR(-ENOMEM);
>> + }
>> +
>> + ctx->pfn = pfn;
>> + ctx->size = size;
>> + kref_init(&ctx->kref);
>> + return ctx;
>> +}
>> +
>> +void pfnmap_track_ctx_release(struct kref *ref)
>> +{
>> + struct pfnmap_track_ctx *ctx = container_of(ref, struct pfnmap_track_ctx, kref);
>> +
>> + pfnmap_untrack(ctx->pfn, ctx->size);
>> + kfree(ctx);
>> +}
>> +#endif /* __HAVE_PFNMAP_TRACKING */
>> +
>> /**
>> * remap_pfn_range - remap kernel memory to userspace
>> * @vma: user vma to map to
>> @@ -2884,20 +2903,51 @@ int remap_pfn_range_notrack(struct vm_area_struct *vma, unsigned long addr,
>> *
>> * Return: %0 on success, negative error code otherwise.
>> */
>> +#ifdef __HAVE_PFNMAP_TRACKING
>> int remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
>> unsigned long pfn, unsigned long size, pgprot_t prot)
>> {
>> + struct pfnmap_track_ctx *ctx = NULL;
>> int err;
>>
>> - err = track_pfn_remap(vma, &prot, pfn, addr, PAGE_ALIGN(size));
>> - if (err)
>> + size = PAGE_ALIGN(size);
>> +
>> + /*
>> + * If we cover the full VMA, we'll perform actual tracking, and
>> + * remember to untrack when the last reference to our tracking
>> + * context from a VMA goes away. We'll keep tracking the whole pfn
>> + * range even during VMA splits and partial unmapping.
>> + *
>> + * If we only cover parts of the VMA, we'll only setup the cachemode
>> + * in the pgprot for the pfn range.
>> + */
>> + if (addr == vma->vm_start && addr + size == vma->vm_end) {
>> + if (vma->pfnmap_track_ctx)
>> + return -EINVAL;
>> + ctx = pfnmap_track_ctx_alloc(pfn, size, &prot);
>> + if (IS_ERR(ctx))
>> + return PTR_ERR(ctx);
>> + } else if (pfnmap_setup_cachemode(pfn, size, &prot)) {
>> return -EINVAL;
>> + }
>>
>> err = remap_pfn_range_notrack(vma, addr, pfn, size, prot);
>> - if (err)
>> - untrack_pfn(vma, pfn, PAGE_ALIGN(size), true);
>> + if (ctx) {
>> + if (err)
>> + kref_put(&ctx->kref, pfnmap_track_ctx_release);
>> + else
>> + vma->pfnmap_track_ctx = ctx;
>> + }
>> return err;
>> }
>> +
>> +#else
>> +int remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
>> + unsigned long pfn, unsigned long size, pgprot_t prot)
>> +{
>> + return remap_pfn_range_notrack(vma, addr, pfn, size, prot);
>> +}
>> +#endif
>> EXPORT_SYMBOL(remap_pfn_range);
>>
>> /**
>> diff --git a/mm/mmap.c b/mm/mmap.c
>> index 50f902c08341a..09c563c951123 100644
>> --- a/mm/mmap.c
>> +++ b/mm/mmap.c
>> @@ -1784,11 +1784,6 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
>> tmp = vm_area_dup(mpnt);
>> if (!tmp)
>> goto fail_nomem;
>> -
>> - /* track_pfn_copy() will later take care of copying internal state. */
>> - if (unlikely(tmp->vm_flags & VM_PFNMAP))
>> - untrack_pfn_clear(tmp);
>> -
>> retval = vma_dup_policy(mpnt, tmp);
>> if (retval)
>> goto fail_nomem_policy;
>> diff --git a/mm/mremap.c b/mm/mremap.c
>> index 7db9da609c84f..6e78e02f74bd3 100644
>> --- a/mm/mremap.c
>> +++ b/mm/mremap.c
>> @@ -1191,10 +1191,6 @@ static int copy_vma_and_data(struct vma_remap_struct *vrm,
>> if (is_vm_hugetlb_page(vma))
>> clear_vma_resv_huge_pages(vma);
>>
>> - /* Tell pfnmap has moved from this vma */
>> - if (unlikely(vma->vm_flags & VM_PFNMAP))
>> - untrack_pfn_clear(vma);
>> -
>> *new_vma_ptr = new_vma;
>> return err;
>> }
>> diff --git a/mm/vma_init.c b/mm/vma_init.c
>> index 967ca85179864..8e53c7943561e 100644
>> --- a/mm/vma_init.c
>> +++ b/mm/vma_init.c
>> @@ -71,7 +71,51 @@ static void vm_area_init_from(const struct vm_area_struct *src,
>> #ifdef CONFIG_NUMA
>> dest->vm_policy = src->vm_policy;
>> #endif
>> +#ifdef __HAVE_PFNMAP_TRACKING
>> + dest->pfnmap_track_ctx = NULL;
>> +#endif
>> +}
>> +
>> +#ifdef __HAVE_PFNMAP_TRACKING
>> +static inline int vma_pfnmap_track_ctx_dup(struct vm_area_struct *orig,
>> + struct vm_area_struct *new)
>> +{
>> + struct pfnmap_track_ctx *ctx = orig->pfnmap_track_ctx;
>> +
>> + if (likely(!ctx))
>> + return 0;
>> +
>> + /*
>> + * We don't expect to ever hit this. If ever required, we would have
>> + * to duplicate the tracking.
>> + */
>> + if (unlikely(kref_read(&ctx->kref) >= REFCOUNT_MAX))
>
> How not expected is this? :) maybe use WARN_ON_ONCE() if it really should
> never happen?
I guess if we mmap a large PFNMAP and then split it into individual
PTE-sized chunks, we could get many VMAs per-process referencing that
tracing.
Combine that with fork() and I assume one could hit this -- when really
trying hard to achieve it. (probably as privileged user to get a big
VM_PFNMAP, though, but not sure)
In that case, a WARN_ON_ONCE() would be bad -- because it could be
triggered by the user.
We could do a pr_warn_once() instead, stating that this is not supported
right now?
--
Cheers,
David / dhildenb
More information about the Intel-gfx
mailing list