[PATCH 01/15] mm: various small mmap_prepare cleanups
Suren Baghdasaryan
surenb at google.com
Sun Mar 15 15:56:54 PDT 2026
On Thu, Mar 12, 2026 at 1:27 PM Lorenzo Stoakes (Oracle) <ljs at kernel.org> wrote:
>
> Rather than passing arbitrary fields, pass an mmap_action field directly to
> mmap prepare and complete helpers to put all the action-specific logic in
> the function actually doing the work.
>
> Additionally, allow mmap prepare functions to return an error so we can
> error out as soon as possible if there is something logically incorrect in
> the input.
>
> Update remap_pfn_range_prepare() to properly check the input range for the
> CoW case.
By "properly check" do you mean the replacement of desc->start and
desc->end with action->remap.start and action->remap.start +
action->remap.size when calling get_remap_pgoff() from
remap_pfn_range_prepare()?
>
> While we're here, make remap_pfn_range_prepare_vma() a little neater, and
> pass mmap_action directly to call_action_complete().
>
> Then, update compat_vma_mmap() to perform its logic directly, as
> __compat_vma_map() is not used by anything so we don't need to export it.
Not directly related to this patch but while reviewing, I was also
checking vma locking rules in this mmap_prepare() + mmap() sequence
and I noticed that the new VMA flag modification functions like
vma_set_flags_mask() do assert vma_assert_locked(vma). It would be
useful to add these but as a separate change. I will add it to my todo
list.
>
> Also update compat_vma_mmap() to use vfs_mmap_prepare() rather than calling
> the mmap_prepare op directly.
>
> Finally, update the VMA userland tests to reflect the changes.
>
> Signed-off-by: Lorenzo Stoakes (Oracle) <ljs at kernel.org>
> ---
> include/linux/fs.h | 2 -
> include/linux/mm.h | 8 +--
> mm/internal.h | 28 +++++---
> mm/memory.c | 45 +++++++-----
> mm/util.c | 112 +++++++++++++-----------------
> mm/vma.c | 21 +++---
> tools/testing/vma/include/dup.h | 9 ++-
> tools/testing/vma/include/stubs.h | 9 +--
> 8 files changed, 123 insertions(+), 111 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 8b3dd145b25e..a2628a12bd2b 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2058,8 +2058,6 @@ static inline bool can_mmap_file(struct file *file)
> return true;
> }
>
> -int __compat_vma_mmap(const struct file_operations *f_op,
> - struct file *file, struct vm_area_struct *vma);
> int compat_vma_mmap(struct file *file, struct vm_area_struct *vma);
>
> static inline int vfs_mmap(struct file *file, struct vm_area_struct *vma)
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 4c4fd55fc823..cc5960a84382 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4116,10 +4116,10 @@ static inline void mmap_action_ioremap_full(struct vm_area_desc *desc,
> mmap_action_ioremap(desc, desc->start, start_pfn, vma_desc_size(desc));
> }
>
> -void mmap_action_prepare(struct mmap_action *action,
> - struct vm_area_desc *desc);
> -int mmap_action_complete(struct mmap_action *action,
> - struct vm_area_struct *vma);
> +int mmap_action_prepare(struct vm_area_desc *desc,
> + struct mmap_action *action);
> +int mmap_action_complete(struct vm_area_struct *vma,
> + struct mmap_action *action);
>
> /* Look up the first VMA which exactly match the interval vm_start ... vm_end */
> static inline struct vm_area_struct *find_exact_vma(struct mm_struct *mm,
> diff --git a/mm/internal.h b/mm/internal.h
> index 95b583e7e4f7..7bfa85b5e78b 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1775,26 +1775,32 @@ int walk_page_range_debug(struct mm_struct *mm, unsigned long start,
> void dup_mm_exe_file(struct mm_struct *mm, struct mm_struct *oldmm);
> int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm);
>
> -void remap_pfn_range_prepare(struct vm_area_desc *desc, unsigned long pfn);
> -int remap_pfn_range_complete(struct vm_area_struct *vma, unsigned long addr,
> - unsigned long pfn, unsigned long size, pgprot_t pgprot);
> +int remap_pfn_range_prepare(struct vm_area_desc *desc,
> + struct mmap_action *action);
> +int remap_pfn_range_complete(struct vm_area_struct *vma,
> + struct mmap_action *action);
>
> -static inline void io_remap_pfn_range_prepare(struct vm_area_desc *desc,
> - unsigned long orig_pfn, unsigned long size)
> +static inline int io_remap_pfn_range_prepare(struct vm_area_desc *desc,
> + struct mmap_action *action)
> {
> + const unsigned long orig_pfn = action->remap.start_pfn;
> + const unsigned long size = action->remap.size;
> const unsigned long pfn = io_remap_pfn_range_pfn(orig_pfn, size);
>
> - return remap_pfn_range_prepare(desc, pfn);
> + action->remap.start_pfn = pfn;
> + return remap_pfn_range_prepare(desc, action);
> }
>
> static inline int io_remap_pfn_range_complete(struct vm_area_struct *vma,
> - unsigned long addr, unsigned long orig_pfn, unsigned long size,
> - pgprot_t orig_prot)
> + struct mmap_action *action)
> {
> - const unsigned long pfn = io_remap_pfn_range_pfn(orig_pfn, size);
> - const pgprot_t prot = pgprot_decrypted(orig_prot);
> + const unsigned long size = action->remap.size;
> + const unsigned long orig_pfn = action->remap.start_pfn;
> + const pgprot_t orig_prot = vma->vm_page_prot;
>
> - return remap_pfn_range_complete(vma, addr, pfn, size, prot);
> + action->remap.pgprot = pgprot_decrypted(orig_prot);
> + action->remap.start_pfn = io_remap_pfn_range_pfn(orig_pfn, size);
> + return remap_pfn_range_complete(vma, action);
> }
>
> #ifdef CONFIG_MMU_NOTIFIER
> diff --git a/mm/memory.c b/mm/memory.c
> index 6aa0ea4af1fc..364fa8a45360 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3099,26 +3099,34 @@ static int do_remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
> }
> #endif
>
> -void remap_pfn_range_prepare(struct vm_area_desc *desc, unsigned long pfn)
> +int remap_pfn_range_prepare(struct vm_area_desc *desc,
> + struct mmap_action *action)
> {
> - /*
> - * We set addr=VMA start, end=VMA end here, so this won't fail, but we
> - * check it again on complete and will fail there if specified addr is
> - * invalid.
> - */
> - get_remap_pgoff(vma_desc_is_cow_mapping(desc), desc->start, desc->end,
> - desc->start, desc->end, pfn, &desc->pgoff);
> + const unsigned long start = action->remap.start;
> + const unsigned long end = start + action->remap.size;
> + const unsigned long pfn = action->remap.start_pfn;
> + const bool is_cow = vma_desc_is_cow_mapping(desc);
I was trying to figure out who sets action->remap.start and
action->remap.size and if they somehow guaranteed to be always equal
to desc->start and (desc->end - desc->start). My understanding is that
action->remap.start and action->remap.size are set by
f_op->mmap_prepare() but I'm not sure if they are always the same as
desc->start and (desc->end - desc->start) and if so, how do we enforce
that.
> + int err;
> +
> + err = get_remap_pgoff(is_cow, start, end, desc->start, desc->end, pfn,
> + &desc->pgoff);
> + if (err)
> + return err;
> +
> vma_desc_set_flags_mask(desc, VMA_REMAP_FLAGS);
> + return 0;
> }
>
> -static int remap_pfn_range_prepare_vma(struct vm_area_struct *vma, unsigned long addr,
> - unsigned long pfn, unsigned long size)
> +static int remap_pfn_range_prepare_vma(struct vm_area_struct *vma,
> + unsigned long addr, unsigned long pfn,
> + unsigned long size)
> {
> - unsigned long end = addr + PAGE_ALIGN(size);
> + const unsigned long end = addr + PAGE_ALIGN(size);
> + const bool is_cow = is_cow_mapping(vma->vm_flags);
> int err;
>
> - err = get_remap_pgoff(is_cow_mapping(vma->vm_flags), addr, end,
> - vma->vm_start, vma->vm_end, pfn, &vma->vm_pgoff);
> + err = get_remap_pgoff(is_cow, addr, end, vma->vm_start, vma->vm_end,
> + pfn, &vma->vm_pgoff);
> if (err)
> return err;
>
> @@ -3151,10 +3159,15 @@ int remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
> }
> EXPORT_SYMBOL(remap_pfn_range);
>
> -int remap_pfn_range_complete(struct vm_area_struct *vma, unsigned long addr,
> - unsigned long pfn, unsigned long size, pgprot_t prot)
> +int remap_pfn_range_complete(struct vm_area_struct *vma,
> + struct mmap_action *action)
> {
> - return do_remap_pfn_range(vma, addr, pfn, size, prot);
> + const unsigned long start = action->remap.start;
> + const unsigned long pfn = action->remap.start_pfn;
> + const unsigned long size = action->remap.size;
> + const pgprot_t prot = action->remap.pgprot;
> +
> + return do_remap_pfn_range(vma, start, pfn, size, prot);
> }
>
> /**
> diff --git a/mm/util.c b/mm/util.c
> index ce7ae80047cf..dba1191725b6 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -1163,43 +1163,6 @@ void flush_dcache_folio(struct folio *folio)
> EXPORT_SYMBOL(flush_dcache_folio);
> #endif
>
> -/**
> - * __compat_vma_mmap() - See description for compat_vma_mmap()
> - * for details. This is the same operation, only with a specific file operations
> - * struct which may or may not be the same as vma->vm_file->f_op.
> - * @f_op: The file operations whose .mmap_prepare() hook is specified.
> - * @file: The file which backs or will back the mapping.
> - * @vma: The VMA to apply the .mmap_prepare() hook to.
> - * Returns: 0 on success or error.
> - */
> -int __compat_vma_mmap(const struct file_operations *f_op,
> - struct file *file, struct vm_area_struct *vma)
> -{
> - struct vm_area_desc desc = {
> - .mm = vma->vm_mm,
> - .file = file,
> - .start = vma->vm_start,
> - .end = vma->vm_end,
> -
> - .pgoff = vma->vm_pgoff,
> - .vm_file = vma->vm_file,
> - .vma_flags = vma->flags,
> - .page_prot = vma->vm_page_prot,
> -
> - .action.type = MMAP_NOTHING, /* Default */
> - };
> - int err;
> -
> - err = f_op->mmap_prepare(&desc);
> - if (err)
> - return err;
> -
> - mmap_action_prepare(&desc.action, &desc);
> - set_vma_from_desc(vma, &desc);
> - return mmap_action_complete(&desc.action, vma);
> -}
> -EXPORT_SYMBOL(__compat_vma_mmap);
> -
> /**
> * compat_vma_mmap() - Apply the file's .mmap_prepare() hook to an
> * existing VMA and execute any requested actions.
> @@ -1228,7 +1191,31 @@ EXPORT_SYMBOL(__compat_vma_mmap);
> */
> int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
> {
> - return __compat_vma_mmap(file->f_op, file, vma);
> + struct vm_area_desc desc = {
> + .mm = vma->vm_mm,
> + .file = file,
> + .start = vma->vm_start,
> + .end = vma->vm_end,
> +
> + .pgoff = vma->vm_pgoff,
> + .vm_file = vma->vm_file,
> + .vma_flags = vma->flags,
> + .page_prot = vma->vm_page_prot,
> +
> + .action.type = MMAP_NOTHING, /* Default */
> + };
> + int err;
> +
> + err = vfs_mmap_prepare(file, &desc);
> + if (err)
> + return err;
> +
> + err = mmap_action_prepare(&desc, &desc.action);
> + if (err)
> + return err;
> +
> + set_vma_from_desc(vma, &desc);
> + return mmap_action_complete(vma, &desc.action);
> }
> EXPORT_SYMBOL(compat_vma_mmap);
>
> @@ -1320,8 +1307,8 @@ void snapshot_page(struct page_snapshot *ps, const struct page *page)
> }
> }
>
> -static int mmap_action_finish(struct mmap_action *action,
> - const struct vm_area_struct *vma, int err)
> +static int mmap_action_finish(struct vm_area_struct *vma,
> + struct mmap_action *action, int err)
> {
> /*
> * If an error occurs, unmap the VMA altogether and return an error. We
> @@ -1355,35 +1342,36 @@ static int mmap_action_finish(struct mmap_action *action,
> * action which need to be performed.
> * @desc: The VMA descriptor to prepare for @action.
> * @action: The action to perform.
> + *
> + * Returns: 0 on success, otherwise error.
> */
> -void mmap_action_prepare(struct mmap_action *action,
> - struct vm_area_desc *desc)
> +int mmap_action_prepare(struct vm_area_desc *desc,
> + struct mmap_action *action)
Any reason you are swapping the arguments?
It also looks like we always call mmap_action_prepare() with action ==
desc->action, like this: mmap_action_prepare(&desc.action, &desc). Why
don't we eliminate the action parameter altogether and use desc.action
from inside the function?
> +
extra new line.
> {
> switch (action->type) {
> case MMAP_NOTHING:
> - break;
> + return 0;
> case MMAP_REMAP_PFN:
> - remap_pfn_range_prepare(desc, action->remap.start_pfn);
> - break;
> + return remap_pfn_range_prepare(desc, action);
> case MMAP_IO_REMAP_PFN:
> - io_remap_pfn_range_prepare(desc, action->remap.start_pfn,
> - action->remap.size);
> - break;
> + return io_remap_pfn_range_prepare(desc, action);
> }
> }
> EXPORT_SYMBOL(mmap_action_prepare);
>
> /**
> * mmap_action_complete - Execute VMA descriptor action.
> - * @action: The action to perform.
> * @vma: The VMA to perform the action upon.
> + * @action: The action to perform.
> *
> * Similar to mmap_action_prepare().
> *
> * Return: 0 on success, or error, at which point the VMA will be unmapped.
> */
> -int mmap_action_complete(struct mmap_action *action,
> - struct vm_area_struct *vma)
> +int mmap_action_complete(struct vm_area_struct *vma,
> + struct mmap_action *action)
> +
> {
> int err = 0;
>
> @@ -1391,23 +1379,19 @@ int mmap_action_complete(struct mmap_action *action,
> case MMAP_NOTHING:
> break;
> case MMAP_REMAP_PFN:
> - err = remap_pfn_range_complete(vma, action->remap.start,
> - action->remap.start_pfn, action->remap.size,
> - action->remap.pgprot);
> + err = remap_pfn_range_complete(vma, action);
> break;
> case MMAP_IO_REMAP_PFN:
> - err = io_remap_pfn_range_complete(vma, action->remap.start,
> - action->remap.start_pfn, action->remap.size,
> - action->remap.pgprot);
> + err = io_remap_pfn_range_complete(vma, action);
> break;
> }
>
> - return mmap_action_finish(action, vma, err);
> + return mmap_action_finish(vma, action, err);
> }
> EXPORT_SYMBOL(mmap_action_complete);
> #else
> -void mmap_action_prepare(struct mmap_action *action,
> - struct vm_area_desc *desc)
> +int mmap_action_prepare(struct vm_area_desc *desc,
> + struct mmap_action *action)
> {
> switch (action->type) {
> case MMAP_NOTHING:
> @@ -1417,11 +1401,13 @@ void mmap_action_prepare(struct mmap_action *action,
> WARN_ON_ONCE(1); /* nommu cannot handle these. */
> break;
> }
> +
> + return 0;
> }
> EXPORT_SYMBOL(mmap_action_prepare);
>
> -int mmap_action_complete(struct mmap_action *action,
> - struct vm_area_struct *vma)
> +int mmap_action_complete(struct vm_area_struct *vma,
> + struct mmap_action *action)
> {
> int err = 0;
>
> @@ -1436,7 +1422,7 @@ int mmap_action_complete(struct mmap_action *action,
> break;
> }
>
> - return mmap_action_finish(action, vma, err);
> + return mmap_action_finish(vma, action, err);
> }
> EXPORT_SYMBOL(mmap_action_complete);
> #endif
> diff --git a/mm/vma.c b/mm/vma.c
> index be64f781a3aa..054cf1d262fb 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -2613,15 +2613,19 @@ static void __mmap_complete(struct mmap_state *map, struct vm_area_struct *vma)
> vma_set_page_prot(vma);
> }
>
> -static void call_action_prepare(struct mmap_state *map,
> - struct vm_area_desc *desc)
> +static int call_action_prepare(struct mmap_state *map,
> + struct vm_area_desc *desc)
> {
> struct mmap_action *action = &desc->action;
> + int err;
>
> - mmap_action_prepare(action, desc);
> + err = mmap_action_prepare(desc, action);
> + if (err)
> + return err;
>
> if (action->hide_from_rmap_until_complete)
> map->hold_file_rmap_lock = true;
> + return 0;
> }
>
> /*
> @@ -2645,7 +2649,9 @@ static int call_mmap_prepare(struct mmap_state *map,
> if (err)
> return err;
>
> - call_action_prepare(map, desc);
> + err = call_action_prepare(map, desc);
> + if (err)
> + return err;
>
> /* Update fields permitted to be changed. */
> map->pgoff = desc->pgoff;
> @@ -2700,13 +2706,12 @@ static bool can_set_ksm_flags_early(struct mmap_state *map)
> }
>
> static int call_action_complete(struct mmap_state *map,
> - struct vm_area_desc *desc,
> + struct mmap_action *action,
> struct vm_area_struct *vma)
> {
> - struct mmap_action *action = &desc->action;
> int ret;
>
> - ret = mmap_action_complete(action, vma);
> + ret = mmap_action_complete(vma, action);
>
> /* If we held the file rmap we need to release it. */
> if (map->hold_file_rmap_lock) {
> @@ -2768,7 +2773,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
> __mmap_complete(&map, vma);
>
> if (have_mmap_prepare && allocated_new) {
> - error = call_action_complete(&map, &desc, vma);
> + error = call_action_complete(&map, &desc.action, vma);
>
> if (error)
> return error;
> diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
> index 5eb313beb43d..908beb263307 100644
> --- a/tools/testing/vma/include/dup.h
> +++ b/tools/testing/vma/include/dup.h
> @@ -1106,7 +1106,7 @@ static inline int __compat_vma_mmap(const struct file_operations *f_op,
>
> .pgoff = vma->vm_pgoff,
> .vm_file = vma->vm_file,
> - .vm_flags = vma->vm_flags,
> + .vma_flags = vma->flags,
> .page_prot = vma->vm_page_prot,
>
> .action.type = MMAP_NOTHING, /* Default */
> @@ -1117,9 +1117,12 @@ static inline int __compat_vma_mmap(const struct file_operations *f_op,
> if (err)
> return err;
>
> - mmap_action_prepare(&desc.action, &desc);
> + err = mmap_action_prepare(&desc, &desc.action);
> + if (err)
> + return err;
> +
> set_vma_from_desc(vma, &desc);
> - return mmap_action_complete(&desc.action, vma);
> + return mmap_action_complete(vma, &desc.action);
> }
>
> static inline int compat_vma_mmap(struct file *file,
> diff --git a/tools/testing/vma/include/stubs.h b/tools/testing/vma/include/stubs.h
> index 947a3a0c2566..76c4b668bc62 100644
> --- a/tools/testing/vma/include/stubs.h
> +++ b/tools/testing/vma/include/stubs.h
> @@ -81,13 +81,14 @@ static inline void free_anon_vma_name(struct vm_area_struct *vma)
> {
> }
>
> -static inline void mmap_action_prepare(struct mmap_action *action,
> - struct vm_area_desc *desc)
> +static inline int mmap_action_prepare(struct vm_area_desc *desc,
> + struct mmap_action *action)
> {
> + return 0;
> }
>
> -static inline int mmap_action_complete(struct mmap_action *action,
> - struct vm_area_struct *vma)
> +static inline int mmap_action_complete(struct vm_area_struct *vma,
> + struct mmap_action *action)
> {
> return 0;
> }
> --
> 2.53.0
>
More information about the linux-afs
mailing list