[PATCH v2 0/3] remove mmap_action success, error hooks

[PATCH v2 0/3] remove mmap_action success, error hooks

[Lorenzo Stoakes]

The mmap_action->success_hook was a strange beast added to enable code
which appeared to absolutely require access to a VMA pointer to work
correctly.

Primarily this was for hugetlb, however a different approach will be taken
there, as clearly more work is required to figure out a sensible way of
converting hugetlb to use mmap_prepare.

The other user was the memory char driver, specifically /dev/zero which has
the unusual property of explicitly setting file-backed VMAs anonymous.

Providing the success hook was always foolish, as it allowed drivers a way to
workaround the restriction that they should not access a pointer to a
not-yet-correctly-initialised VMA - which defeats the purpose of the
mmap_prepare work.

We can achieve the same thing in memory char driver without needing the
success hook, so this series removes that, then removes the success hook
altogether.

The error hook is also unnecessary - the motivation for this was for
functions which need to filter the error code when performing an mmap
action in order to avoid breaking userspace.

We can achieve this by just providing a field for the error code. Doing
this means we don't have to worry about the hook doing anything odd.

We also add a check to ensure the error code is in fact valid.

Again the memory char driver is the only current user of this, so this
series updates it to use that.

After this change mmap_action has no custom hooks at all, which seems
rather more cromulent than before.

v2:
* Fold fix-patch in, defaulting the vma descriptor vm_ops field to the
  dummy VMA operations.
* Update commit message in 1/3 to reflect this.

v1:
https://lore.kernel.org/all/cover.1779379804.git.ljs@kernel.org

Lorenzo Stoakes (3):
  drivers/char/mem: eliminate unnecessary use of success_hook
  mm/vma: remove mmap_action->success_hook
  mm/vma: eliminate mmap_action->error_hook, introduce error_filter

 drivers/char/mem.c              | 25 ++++++-------------------
 include/linux/mm.h              |  5 +++++
 include/linux/mm_types.h        | 19 +++----------------
 mm/util.c                       | 32 ++++++++++++++++++++++----------
 mm/vma.c                        |  3 +++
 tools/testing/vma/include/dup.h | 20 ++++----------------
 6 files changed, 43 insertions(+), 61 deletions(-)

--
2.54.0

[PATCH v2 1/3] drivers/char/mem: eliminate unnecessary use of success_hook

[Lorenzo Stoakes]

/dev/zero, uniquely, marks memory mapped there as anonymous. This is
currently achieved using the mmap_action->success_hook.

However this hook circumvents the abstraction of VMA initialisation so it's
preferable to do things a different way.

To achieve this, this patch firstly defaults the VMA descriptor's vm_ops
field to the dummy VMA operations, which is what file-backed VMAs default
this field to.

That way, we can detect whether a driver sets this field to NULL in order
to mark it anonymous.

We then introduce vma_desc_set_anonymous() to do this explicitly, and
invoke it in mmap_zero_prepare().

This way, any driver which does not explicitly set desc->vm_ops, retains
the dummy vm_ops as they would previously.

We also update set_vma_user_defined_fields() to make clear that we are
either setting vma->vm_ops to what is provided by the driver (or defaulting
to dummy_vm_ops if not set), or setting the VMA anonymous.

This lays the groundwork for removing the success hook.

Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
---
 drivers/char/mem.c              | 17 +++++------------
 include/linux/mm.h              |  5 +++++
 mm/util.c                       |  1 +
 mm/vma.c                        |  3 +++
 tools/testing/vma/include/dup.h |  1 +
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 5fd421e48c04..a4297eb39887 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -504,17 +504,6 @@ static ssize_t read_zero(struct file *file, char __user *buf,
 	return cleared;
 }
 
-static int mmap_zero_private_success(const struct vm_area_struct *vma)
-{
-	/*
-	 * This is a highly unique situation where we mark a MAP_PRIVATE mapping
-	 * of /dev/zero anonymous, despite it not being.
-	 */
-	vma_set_anonymous((struct vm_area_struct *)vma);
-
-	return 0;
-}
-
 static int mmap_zero_prepare(struct vm_area_desc *desc)
 {
 #ifndef CONFIG_MMU
@@ -523,7 +512,11 @@ static int mmap_zero_prepare(struct vm_area_desc *desc)
 	if (vma_desc_test(desc, VMA_SHARED_BIT))
 		return shmem_zero_setup_desc(desc);
 
-	desc->action.success_hook = mmap_zero_private_success;
+	/*
+	 * This is a highly unique situation where we mark a MAP_PRIVATE mapping
+	 * of /dev/zero anonymous, despite it not being.
+	 */
+	vma_desc_set_anonymous(desc);
 	return 0;
 }
 
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 9cedc5e75aa9..2138c86403f5 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1489,6 +1489,11 @@ static inline void vma_set_anonymous(struct vm_area_struct *vma)
 	vma->vm_ops = NULL;
 }
 
+static inline void vma_desc_set_anonymous(struct vm_area_desc *desc)
+{
+	desc->vm_ops = NULL;
+}
+
 static inline bool vma_is_anonymous(struct vm_area_struct *vma)
 {
 	return !vma->vm_ops;
diff --git a/mm/util.c b/mm/util.c
index 3cc949a0b7ed..2b2a9df689d7 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1192,6 +1192,7 @@ void compat_set_desc_from_vma(struct vm_area_desc *desc,
 	desc->vm_file = vma->vm_file;
 	desc->vma_flags = vma->flags;
 	desc->page_prot = vma->vm_page_prot;
+	desc->vm_ops = vma->vm_ops;
 
 	/* Default. */
 	desc->action.type = MMAP_NOTHING;
diff --git a/mm/vma.c b/mm/vma.c
index d90791b00a7b..9eea2850818a 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2697,6 +2697,8 @@ static void set_vma_user_defined_fields(struct vm_area_struct *vma,
 {
 	if (map->vm_ops)
 		vma->vm_ops = map->vm_ops;
+	else	/* Only /dev/zero should do this. */
+		vma_set_anonymous(vma);
 	vma->vm_private_data = map->vm_private_data;
 }
 
@@ -2744,6 +2746,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
 		.action = {
 			.type = MMAP_NOTHING, /* Default to no further action. */
 		},
+		.vm_ops = &vma_dummy_vm_ops,
 	};
 	bool allocated_new = false;
 	int error;
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index 9e0dfd3a85b0..306171d061e7 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -1303,6 +1303,7 @@ static inline void compat_set_desc_from_vma(struct vm_area_desc *desc,
 	desc->vm_file = vma->vm_file;
 	desc->vma_flags = vma->flags;
 	desc->page_prot = vma->vm_page_prot;
+	desc->vm_ops = vma->vm_ops;
 
 	/* Default. */
 	desc->action.type = MMAP_NOTHING;
-- 
2.54.0

[PATCH v2 2/3] mm/vma: remove mmap_action->success_hook

[Lorenzo Stoakes]

This hook was introduced to work around code that seemed to absolutely
require access to a VMA pointer upon mmap().

However, providing this hook leaves a backdoor to drivers getting access to
the very thing mmap_prepare eliminates - a pointer to the VMA.

Let's solve this contradiction by removing it. The key intended user was
hugetlb, however it seems that the best course now is to avoid allowing all
drivers the ability to work around mmap_prepare, and find a different
solution there.

Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
---
 include/linux/mm_types.h        | 10 ----------
 mm/util.c                       |  2 --
 tools/testing/vma/include/dup.h | 10 ----------
 3 files changed, 22 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index a308e2c23b82..945c0a5386d6 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -843,16 +843,6 @@ struct mmap_action {
 	};
 	enum mmap_action_type type;
 
-	/*
-	 * If specified, this hook is invoked after the selected action has been
-	 * successfully completed. Note that the VMA write lock still held.
-	 *
-	 * The absolute minimum ought to be done here.
-	 *
-	 * Returns 0 on success, or an error code.
-	 */
-	int (*success_hook)(const struct vm_area_struct *vma);
-
 	/*
 	 * If specified, this hook is invoked when an error occurred when
 	 * attempting the selected action.
diff --git a/mm/util.c b/mm/util.c
index 2b2a9df689d7..4e172990afcd 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1397,8 +1397,6 @@ static int mmap_action_finish(struct vm_area_struct *vma,
 
 	if (!err)
 		err = call_vma_mapped(vma);
-	if (!err && action->success_hook)
-		err = action->success_hook(vma);
 
 	/* do_munmap() might take rmap lock, so release if held. */
 	maybe_rmap_unlock_action(vma, action);
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index 306171d061e7..fddfd1b57c09 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -482,16 +482,6 @@ struct mmap_action {
 	};
 	enum mmap_action_type type;
 
-	/*
-	 * If specified, this hook is invoked after the selected action has been
-	 * successfully completed. Note that the VMA write lock still held.
-	 *
-	 * The absolute minimum ought to be done here.
-	 *
-	 * Returns 0 on success, or an error code.
-	 */
-	int (*success_hook)(const struct vm_area_struct *vma);
-
 	/*
 	 * If specified, this hook is invoked when an error occurred when
 	 * attempting the selection action.
-- 
2.54.0

[PATCH v2 3/3] mm/vma: eliminate mmap_action->error_hook, introduce error_filter

[Lorenzo Stoakes]

Rather than providing a hook, simplify things by providing the ability to
filter errors. This allows us to more carefully validate the value provided
and thus ensure only a valid error code is specified, and simplifies the
interface.

This way, we eliminate all hooks but mmap_prepare and allow only mmap
actions to be specified (which core mm controls).

This significantly improves robustness and eliminates any unnecessary code
duplication in driver mmap hooks.

We also update the /dev/mem logic (the only user) to use
mmap_action->error_filter instead.

Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
---
 drivers/char/mem.c              |  8 +-------
 include/linux/mm_types.h        |  9 +++------
 mm/util.c                       | 29 +++++++++++++++++++++--------
 tools/testing/vma/include/dup.h |  9 +++------
 4 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index a4297eb39887..11639d988e47 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -322,11 +322,6 @@ static const struct vm_operations_struct mmap_mem_ops = {
 #endif
 };
 
-static int mmap_filter_error(int err)
-{
-	return -EAGAIN;
-}
-
 static int mmap_mem_prepare(struct vm_area_desc *desc)
 {
 	struct file *file = desc->file;
@@ -362,8 +357,7 @@ static int mmap_mem_prepare(struct vm_area_desc *desc)
 
 	/* Remap-pfn-range will mark the range with the I/O flag. */
 	mmap_action_remap_full(desc, desc->pgoff);
-	/* We filter remap errors to -EAGAIN. */
-	desc->action.error_hook = mmap_filter_error;
+	desc->action.error_filter = -EAGAIN;
 
 	return 0;
 }
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 945c0a5386d6..8d1fb85e7684 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -844,13 +844,10 @@ struct mmap_action {
 	enum mmap_action_type type;
 
 	/*
-	 * If specified, this hook is invoked when an error occurred when
-	 * attempting the selected action.
-	 *
-	 * The hook can return an error code in order to filter the error, but
-	 * it is not valid to clear the error here.
+	 * If non-zero, filter errors that arise from mmap actions such that we
+	 * return error_filter instead. Only valid error codes may be specified.
 	 */
-	int (*error_hook)(int err);
+	int error_filter;
 
 	/*
 	 * This should be set in rare instances where the operation required
diff --git a/mm/util.c b/mm/util.c
index 4e172990afcd..9b4e5432d45a 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1414,16 +1414,22 @@ static int mmap_action_finish(struct vm_area_struct *vma,
 	 */
 	len = vma_pages(vma) << PAGE_SHIFT;
 	do_munmap(current->mm, vma->vm_start, len, NULL);
-	if (action->error_hook) {
-		/* We may want to filter the error. */
-		err = action->error_hook(err);
-		/* The caller should not clear the error. */
-		VM_WARN_ON_ONCE(!err);
-	}
-	return err;
+
+	return action->error_filter ?: err;
 }
 
 #ifdef CONFIG_MMU
+
+static int check_mmap_action(struct mmap_action *action)
+{
+	const unsigned long filter = action->error_filter;
+
+	if (WARN_ON_ONCE(filter && !IS_ERR_VALUE(filter)))
+		return -EINVAL;
+
+	return 0;
+}
+
 /**
  * mmap_action_prepare - Perform preparatory setup for an VMA descriptor
  * action which need to be performed.
@@ -1433,7 +1439,14 @@ static int mmap_action_finish(struct vm_area_struct *vma,
  */
 int mmap_action_prepare(struct vm_area_desc *desc)
 {
-	switch (desc->action.type) {
+	struct mmap_action *action = &desc->action;
+	int err;
+
+	err = check_mmap_action(action);
+	if (err)
+		return err;
+
+	switch (action->type) {
 	case MMAP_NOTHING:
 		return 0;
 	case MMAP_REMAP_PFN:
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index fddfd1b57c09..bcd569a72087 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -483,13 +483,10 @@ struct mmap_action {
 	enum mmap_action_type type;
 
 	/*
-	 * If specified, this hook is invoked when an error occurred when
-	 * attempting the selection action.
-	 *
-	 * The hook can return an error code in order to filter the error, but
-	 * it is not valid to clear the error here.
+	 * If non-zero, filter errors that arise from mmap actions such that we
+	 * return error_filter instead. Only valid error codes may be specified.
 	 */
-	int (*error_hook)(int err);
+	int error_filter;
 
 	/*
 	 * This should be set in rare instances where the operation required
-- 
2.54.0

Re: [PATCH v2 0/3] remove mmap_action success, error hooks

[Lorenzo Stoakes]

FYI re: the sashiko reports on the v1:

https://sashiko.dev/#/patchset/cover.1779379804.git.ljs%40kernel.org

1. This was the bug the fix-patch fixed, it seems sashiko doesn't update when
   these are sent (which I cannot blame it for :).

   Anyway, I did say I felt fix-patches should just not be accepted any more to
   avoid various forms of confusion, so in future will just reply to ACK a
   thing like that, then send out a respin the next day :>)

2. The 'the series didn't introduce this' thread can just end right there ;)
   It's an existing behaviour that's been there from... very early on. It can
   wait. I will put it (probably quite far down) on my TODO to examine that.

Thanks, Lorenzo

Re: [PATCH v2 1/3] drivers/char/mem: eliminate unnecessary use of success_hook

[David Hildenbrand (Arm)]

On 5/22/26 18:00, Lorenzo Stoakes wrote:
> /dev/zero, uniquely, marks memory mapped there as anonymous. This is
> currently achieved using the mmap_action->success_hook.
> 
> However this hook circumvents the abstraction of VMA initialisation so it's
> preferable to do things a different way.
> 
> To achieve this, this patch firstly defaults the VMA descriptor's vm_ops
> field to the dummy VMA operations, which is what file-backed VMAs default
> this field to.
> 
> That way, we can detect whether a driver sets this field to NULL in order
> to mark it anonymous.
> 
> We then introduce vma_desc_set_anonymous() to do this explicitly, and
> invoke it in mmap_zero_prepare().
> 
> This way, any driver which does not explicitly set desc->vm_ops, retains
> the dummy vm_ops as they would previously.
> 
> We also update set_vma_user_defined_fields() to make clear that we are
> either setting vma->vm_ops to what is provided by the driver (or defaulting
> to dummy_vm_ops if not set), or setting the VMA anonymous.
> 
> This lays the groundwork for removing the success hook.
> 
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---

Looks good

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH v2 2/3] mm/vma: remove mmap_action->success_hook

[David Hildenbrand (Arm)]

On 5/22/26 18:00, Lorenzo Stoakes wrote:
> This hook was introduced to work around code that seemed to absolutely
> require access to a VMA pointer upon mmap().
> 
> However, providing this hook leaves a backdoor to drivers getting access to
> the very thing mmap_prepare eliminates - a pointer to the VMA.

Good!

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH v2 3/3] mm/vma: eliminate mmap_action->error_hook, introduce error_filter

[David Hildenbrand (Arm)]

On 5/22/26 18:00, Lorenzo Stoakes wrote:
> Rather than providing a hook, simplify things by providing the ability to
> filter errors. This allows us to more carefully validate the value provided
> and thus ensure only a valid error code is specified, and simplifies the
> interface.
> 
> This way, we eliminate all hooks but mmap_prepare and allow only mmap
> actions to be specified (which core mm controls).
> 
> This significantly improves robustness and eliminates any unnecessary code
> duplication in driver mmap hooks.
> 
> We also update the /dev/mem logic (the only user) to use
> mmap_action->error_filter instead.
> 
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
>  drivers/char/mem.c              |  8 +-------
>  include/linux/mm_types.h        |  9 +++------
>  mm/util.c                       | 29 +++++++++++++++++++++--------
>  tools/testing/vma/include/dup.h |  9 +++------
>  4 files changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index a4297eb39887..11639d988e47 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -322,11 +322,6 @@ static const struct vm_operations_struct mmap_mem_ops = {
>  #endif
>  };
>  
> -static int mmap_filter_error(int err)
> -{
> -	return -EAGAIN;
> -}
> -
>  static int mmap_mem_prepare(struct vm_area_desc *desc)
>  {
>  	struct file *file = desc->file;
> @@ -362,8 +357,7 @@ static int mmap_mem_prepare(struct vm_area_desc *desc)
>  
>  	/* Remap-pfn-range will mark the range with the I/O flag. */
>  	mmap_action_remap_full(desc, desc->pgoff);
> -	/* We filter remap errors to -EAGAIN. */
> -	desc->action.error_hook = mmap_filter_error;
> +	desc->action.error_filter = -EAGAIN;
>  
>  	return 0;
>  }
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 945c0a5386d6..8d1fb85e7684 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -844,13 +844,10 @@ struct mmap_action {
>  	enum mmap_action_type type;
>  
>  	/*
> -	 * If specified, this hook is invoked when an error occurred when
> -	 * attempting the selected action.
> -	 *
> -	 * The hook can return an error code in order to filter the error, but
> -	 * it is not valid to clear the error here.
> +	 * If non-zero, filter errors that arise from mmap actions such that we
> +	 * return error_filter instead. Only valid error codes may be specified.

Is that really a filter or rather an "error conversion" / "error override".

"Filter" to my German brain implies that we would ... filter selected error
codes, not convert them to something else?

>  	 */
> -	int (*error_hook)(int err);
> +	int error_filter;
>  
>  	/*
>  	 * This should be set in rare instances where the operation required
> diff --git a/mm/util.c b/mm/util.c
> index 4e172990afcd..9b4e5432d45a 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -1414,16 +1414,22 @@ static int mmap_action_finish(struct vm_area_struct *vma,
>  	 */
>  	len = vma_pages(vma) << PAGE_SHIFT;
>  	do_munmap(current->mm, vma->vm_start, len, NULL);
> -	if (action->error_hook) {
> -		/* We may want to filter the error. */
> -		err = action->error_hook(err);
> -		/* The caller should not clear the error. */
> -		VM_WARN_ON_ONCE(!err);
> -	}
> -	return err;
> +
> +	return action->error_filter ?: err;
>  }

Out of interest, why does dev/mem require this monstrosity?

If it's really a dev/mem specialty, you could just make it less generic and call
the property

"bool eagain_on_error;"

because surely, we don't want any more such monstrosity?

-- 
Cheers,

David

Re: [PATCH v2 3/3] mm/vma: eliminate mmap_action->error_hook, introduce error_filter

[Lorenzo Stoakes]

On Mon, Jun 01, 2026 at 05:25:46PM +0200, David Hildenbrand (Arm) wrote:
> On 5/22/26 18:00, Lorenzo Stoakes wrote:
> > Rather than providing a hook, simplify things by providing the ability to
> > filter errors. This allows us to more carefully validate the value provided
> > and thus ensure only a valid error code is specified, and simplifies the
> > interface.
> >
> > This way, we eliminate all hooks but mmap_prepare and allow only mmap
> > actions to be specified (which core mm controls).
> >
> > This significantly improves robustness and eliminates any unnecessary code
> > duplication in driver mmap hooks.
> >
> > We also update the /dev/mem logic (the only user) to use
> > mmap_action->error_filter instead.
> >
> > Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> > ---
> >  drivers/char/mem.c              |  8 +-------
> >  include/linux/mm_types.h        |  9 +++------
> >  mm/util.c                       | 29 +++++++++++++++++++++--------
> >  tools/testing/vma/include/dup.h |  9 +++------
> >  4 files changed, 28 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> > index a4297eb39887..11639d988e47 100644
> > --- a/drivers/char/mem.c
> > +++ b/drivers/char/mem.c
> > @@ -322,11 +322,6 @@ static const struct vm_operations_struct mmap_mem_ops = {
> >  #endif
> >  };
> >
> > -static int mmap_filter_error(int err)
> > -{
> > -	return -EAGAIN;
> > -}
> > -
> >  static int mmap_mem_prepare(struct vm_area_desc *desc)
> >  {
> >  	struct file *file = desc->file;
> > @@ -362,8 +357,7 @@ static int mmap_mem_prepare(struct vm_area_desc *desc)
> >
> >  	/* Remap-pfn-range will mark the range with the I/O flag. */
> >  	mmap_action_remap_full(desc, desc->pgoff);
> > -	/* We filter remap errors to -EAGAIN. */
> > -	desc->action.error_hook = mmap_filter_error;
> > +	desc->action.error_filter = -EAGAIN;
> >
> >  	return 0;
> >  }
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 945c0a5386d6..8d1fb85e7684 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -844,13 +844,10 @@ struct mmap_action {
> >  	enum mmap_action_type type;
> >
> >  	/*
> > -	 * If specified, this hook is invoked when an error occurred when
> > -	 * attempting the selected action.
> > -	 *
> > -	 * The hook can return an error code in order to filter the error, but
> > -	 * it is not valid to clear the error here.
> > +	 * If non-zero, filter errors that arise from mmap actions such that we
> > +	 * return error_filter instead. Only valid error codes may be specified.
>
> Is that really a filter or rather an "error conversion" / "error override".
>
> "Filter" to my German brain implies that we would ... filter selected error
> codes, not convert them to something else?

I thik it's what people tend to call this, it's what I've seen it called anyway!
I mean your German brain is being logical :) but by convention going with this
terminology.

>
> >  	 */
> > -	int (*error_hook)(int err);
> > +	int error_filter;
> >
> >  	/*
> >  	 * This should be set in rare instances where the operation required
> > diff --git a/mm/util.c b/mm/util.c
> > index 4e172990afcd..9b4e5432d45a 100644
> > --- a/mm/util.c
> > +++ b/mm/util.c
> > @@ -1414,16 +1414,22 @@ static int mmap_action_finish(struct vm_area_struct *vma,
> >  	 */
> >  	len = vma_pages(vma) << PAGE_SHIFT;
> >  	do_munmap(current->mm, vma->vm_start, len, NULL);
> > -	if (action->error_hook) {
> > -		/* We may want to filter the error. */
> > -		err = action->error_hook(err);
> > -		/* The caller should not clear the error. */
> > -		VM_WARN_ON_ONCE(!err);
> > -	}
> > -	return err;
> > +
> > +	return action->error_filter ?: err;
> >  }
>
> Out of interest, why does dev/mem require this monstrosity?

I'm not sure if it's really necessary, but I don't want to break userspace here
by suddenly changing something that very plausible userspace might treat
differently from other error codes (-EAGAIN).

>
> If it's really a dev/mem specialty, you could just make it less generic and call
> the property
>
> "bool eagain_on_error;"
>
> because surely, we don't want any more such monstrosity?

I'd rather not, because I suspect there might be some other cases of this, and
it makes this option kinda horrible, as if it were a sane idea to convert errors
to -EAGAIN :P

Later on, once I've done more conversions, can obviously figure out a better way
or whether we can just remove it.

For now I want to eliminate these two horrible success/error hook hacks so we
can move ahead more sanely (and I can in particular figure out a less horrible
way of making the hugetlb 'vma lock' horror show work with mmap_prepare :)

>
> --
> Cheers,
>
> David

Cheers, Lorenzo