From: Ira Weiny <[email protected]>
Now that the pkey arch support is no longer checked in mm_pkey_free()
there is no reason to have it return int.
Change the return value to void.
Cc: Dave Hansen <[email protected]>
Cc: Aneesh Kumar K.V <[email protected]>
Suggested-by: Sohil Mehta <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>
---
arch/powerpc/include/asm/pkeys.h | 4 +---
arch/x86/include/asm/pkeys.h | 4 +---
include/linux/pkeys.h | 5 +----
mm/mprotect.c | 6 ++++--
4 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index e96aa91f817b..4d01a48ab941 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -105,11 +105,9 @@ static inline int mm_pkey_alloc(struct mm_struct *mm)
return ret;
}
-static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
+static inline void mm_pkey_free(struct mm_struct *mm, int pkey)
{
__mm_pkey_free(mm, pkey);
-
- return 0;
}
/*
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index da02737cc4d1..1f408f46fa9a 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -105,11 +105,9 @@ int mm_pkey_alloc(struct mm_struct *mm)
}
static inline
-int mm_pkey_free(struct mm_struct *mm, int pkey)
+void mm_pkey_free(struct mm_struct *mm, int pkey)
{
mm_set_pkey_free(mm, pkey);
-
- return 0;
}
static inline int vma_pkey(struct vm_area_struct *vma)
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index 86be8bf27b41..bf98c50a3437 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -30,10 +30,7 @@ static inline int mm_pkey_alloc(struct mm_struct *mm)
return -1;
}
-static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
-{
- return -EINVAL;
-}
+static inline void mm_pkey_free(struct mm_struct *mm, int pkey) { }
static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val)
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 41458e729c27..e872bdd2e228 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -809,8 +809,10 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
return ret;
mmap_write_lock(current->mm);
- if (mm_pkey_is_allocated(current->mm, pkey))
- ret = mm_pkey_free(current->mm, pkey);
+ if (mm_pkey_is_allocated(current->mm, pkey)) {
+ mm_pkey_free(current->mm, pkey);
+ ret = 0;
+ }
mmap_write_unlock(current->mm);
/*
--
2.35.1
Le 11/06/2022 à 01:35, [email protected] a écrit :
> From: Ira Weiny <[email protected]>
>
> Now that the pkey arch support is no longer checked in mm_pkey_free()
> there is no reason to have it return int.
Right, I see this is doing what I commented in previous patch.
>
> Change the return value to void.
>
> Cc: Dave Hansen <[email protected]>
> Cc: Aneesh Kumar K.V <[email protected]>
> Suggested-by: Sohil Mehta <[email protected]>
> Signed-off-by: Ira Weiny <[email protected]>
> ---
> arch/powerpc/include/asm/pkeys.h | 4 +---
> arch/x86/include/asm/pkeys.h | 4 +---
> include/linux/pkeys.h | 5 +----
> mm/mprotect.c | 6 ++++--
> 4 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index e96aa91f817b..4d01a48ab941 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -105,11 +105,9 @@ static inline int mm_pkey_alloc(struct mm_struct *mm)
> return ret;
> }
>
> -static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
> +static inline void mm_pkey_free(struct mm_struct *mm, int pkey)
> {
> __mm_pkey_free(mm, pkey);
> -
> - return 0;
> }
>
> /*
> diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
> index da02737cc4d1..1f408f46fa9a 100644
> --- a/arch/x86/include/asm/pkeys.h
> +++ b/arch/x86/include/asm/pkeys.h
> @@ -105,11 +105,9 @@ int mm_pkey_alloc(struct mm_struct *mm)
> }
>
> static inline
> -int mm_pkey_free(struct mm_struct *mm, int pkey)
> +void mm_pkey_free(struct mm_struct *mm, int pkey)
> {
> mm_set_pkey_free(mm, pkey);
> -
> - return 0;
> }
>
> static inline int vma_pkey(struct vm_area_struct *vma)
> diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
> index 86be8bf27b41..bf98c50a3437 100644
> --- a/include/linux/pkeys.h
> +++ b/include/linux/pkeys.h
> @@ -30,10 +30,7 @@ static inline int mm_pkey_alloc(struct mm_struct *mm)
> return -1;
> }
>
> -static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
> -{
> - return -EINVAL;
> -}
> +static inline void mm_pkey_free(struct mm_struct *mm, int pkey) { }
>
> static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> unsigned long init_val)
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 41458e729c27..e872bdd2e228 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -809,8 +809,10 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
> return ret;
>
> mmap_write_lock(current->mm);
> - if (mm_pkey_is_allocated(current->mm, pkey))
> - ret = mm_pkey_free(current->mm, pkey);
> + if (mm_pkey_is_allocated(current->mm, pkey)) {
> + mm_pkey_free(current->mm, pkey);
> + ret = 0;
> + }
Or you could have ret = 0 by default and do
if (mm_pkey_is_allocated(current->mm, pkey))
mm_pkey_free(current->mm, pkey);
else
ret = -EINVAL;
> mmap_write_unlock(current->mm);
>
> /*
On Mon, Jun 13, 2022 at 09:17:06AM +0000, Christophe Leroy wrote:
>
>
> Le 11/06/2022 ? 01:35, [email protected] a ?crit?:
> > From: Ira Weiny <[email protected]>
> >
> > Now that the pkey arch support is no longer checked in mm_pkey_free()
> > there is no reason to have it return int.
>
> Right, I see this is doing what I commented in previous patch.
Yes because it was suggested by Sohil I decided to make it a separate patch to
make the credit easier.
> > diff --git a/mm/mprotect.c b/mm/mprotect.c
> > index 41458e729c27..e872bdd2e228 100644
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -809,8 +809,10 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
> > return ret;
> >
> > mmap_write_lock(current->mm);
> > - if (mm_pkey_is_allocated(current->mm, pkey))
> > - ret = mm_pkey_free(current->mm, pkey);
> > + if (mm_pkey_is_allocated(current->mm, pkey)) {
> > + mm_pkey_free(current->mm, pkey);
> > + ret = 0;
> > + }
>
> Or you could have ret = 0 by default and do
>
> if (mm_pkey_is_allocated(current->mm, pkey))
> mm_pkey_free(current->mm, pkey);
> else
> ret = -EINVAL;
Yes that fits the kernel style better.
Thanks for the review!
Ira
>
> > mmap_write_unlock(current->mm);
> >
> > /*