| Message ID | 20220610233533.3649584-7-ira.weiny@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | User pkey minor bug fixes | expand |
Le 11/06/2022 à 01:35, ira.weiny@intel.com a écrit : > From: Ira Weiny <ira.weiny@intel.com> > > 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 <dave.hansen@linux.intel.com> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > Suggested-by: Sohil Mehta <sohil.mehta@intel.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > 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, ira.weiny@intel.com a écrit : > > From: Ira Weiny <ira.weiny@intel.com> > > > > 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); > > > > /*
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); /*