| Message ID | 20240817-mseal-depessimize-v3-1-d8d2e037df30@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4d1b3416659be70a2251b494e85e25978de06519 |
| Headers | show |
| Series | mm: Optimize mseal checks | expand |
* Pedro Falcato <pedro.falcato@gmail.com> [240816 20:18]: > Move can_modify_vma to vma.h so it can be inlined properly (with > the intent to remove can_modify_mm callsites). > > Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com> > --- > mm/mseal.c | 17 ----------------- > mm/vma.h | 28 ++++++++++++++++++++++++++++ > 2 files changed, 28 insertions(+), 17 deletions(-) > > diff --git a/mm/mseal.c b/mm/mseal.c > index 15bba28acc00..2170e2139ca0 100644 > --- a/mm/mseal.c > +++ b/mm/mseal.c > @@ -16,28 +16,11 @@ > #include <linux/sched.h> > #include "internal.h" > > -static inline bool vma_is_sealed(struct vm_area_struct *vma) > -{ > - return (vma->vm_flags & VM_SEALED); > -} > - > static inline void set_vma_sealed(struct vm_area_struct *vma) > { > vm_flags_set(vma, VM_SEALED); > } > > -/* > - * check if a vma is sealed for modification. > - * return true, if modification is allowed. > - */ > -static bool can_modify_vma(struct vm_area_struct *vma) > -{ > - if (unlikely(vma_is_sealed(vma))) > - return false; > - > - return true; > -} > - > static bool is_madv_discard(int behavior) > { > switch (behavior) { > diff --git a/mm/vma.h b/mm/vma.h > index 6efdf1768a0a..e979015cc7fc 100644 > --- a/mm/vma.h > +++ b/mm/vma.h > @@ -361,4 +361,32 @@ struct vm_area_struct *vma_iter_prev_range(struct vma_iterator *vmi) > return mas_prev_range(&vmi->mas, 0); > } > > +#ifdef CONFIG_64BIT > + > +static inline bool vma_is_sealed(struct vm_area_struct *vma) > +{ > + return (vma->vm_flags & VM_SEALED); > +} If you respin, I'd support dropping this entirely as it seems unnecessary. Either way, Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com> > + > +/* > + * check if a vma is sealed for modification. > + * return true, if modification is allowed. > + */ > +static inline bool can_modify_vma(struct vm_area_struct *vma) > +{ > + if (unlikely(vma_is_sealed(vma))) > + return false; > + > + return true; > +} > + > +#else > + > +static inline bool can_modify_vma(struct vm_area_struct *vma) > +{ > + return true; > +} > + > +#endif > + > #endif /* __MM_VMA_H */ > > -- > 2.46.0 >
On Mon, Aug 19, 2024 at 9:15 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote: > > * Pedro Falcato <pedro.falcato@gmail.com> [240816 20:18]: > > Move can_modify_vma to vma.h so it can be inlined properly (with > > the intent to remove can_modify_mm callsites). > > > > Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com> > > --- > > mm/mseal.c | 17 ----------------- > > mm/vma.h | 28 ++++++++++++++++++++++++++++ > > 2 files changed, 28 insertions(+), 17 deletions(-) > > > > diff --git a/mm/mseal.c b/mm/mseal.c > > index 15bba28acc00..2170e2139ca0 100644 > > --- a/mm/mseal.c > > +++ b/mm/mseal.c > > @@ -16,28 +16,11 @@ > > #include <linux/sched.h> > > #include "internal.h" > > > > -static inline bool vma_is_sealed(struct vm_area_struct *vma) > > -{ > > - return (vma->vm_flags & VM_SEALED); > > -} > > - > > static inline void set_vma_sealed(struct vm_area_struct *vma) > > { > > vm_flags_set(vma, VM_SEALED); > > } > > > > -/* > > - * check if a vma is sealed for modification. > > - * return true, if modification is allowed. > > - */ > > -static bool can_modify_vma(struct vm_area_struct *vma) > > -{ > > - if (unlikely(vma_is_sealed(vma))) > > - return false; > > - > > - return true; > > -} > > - > > static bool is_madv_discard(int behavior) > > { > > switch (behavior) { > > diff --git a/mm/vma.h b/mm/vma.h > > index 6efdf1768a0a..e979015cc7fc 100644 > > --- a/mm/vma.h > > +++ b/mm/vma.h > > @@ -361,4 +361,32 @@ struct vm_area_struct *vma_iter_prev_range(struct vma_iterator *vmi) > > return mas_prev_range(&vmi->mas, 0); > > } > > > > +#ifdef CONFIG_64BIT > > + > > +static inline bool vma_is_sealed(struct vm_area_struct *vma) > > +{ > > + return (vma->vm_flags & VM_SEALED); > > +} > > If you respin, I'd support dropping this entirely as it seems > unnecessary. ACK, I'll fold this into the next patch if the need for v4 arises. > Either way, > Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com> Thank you for the speedy review(s)!
On Sat, Aug 17, 2024 at 01:18:28AM GMT, Pedro Falcato wrote: > Move can_modify_vma to vma.h so it can be inlined properly (with > the intent to remove can_modify_mm callsites). > > Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com> > --- > mm/mseal.c | 17 ----------------- > mm/vma.h | 28 ++++++++++++++++++++++++++++ > 2 files changed, 28 insertions(+), 17 deletions(-) > > diff --git a/mm/mseal.c b/mm/mseal.c > index 15bba28acc00..2170e2139ca0 100644 > --- a/mm/mseal.c > +++ b/mm/mseal.c > @@ -16,28 +16,11 @@ > #include <linux/sched.h> > #include "internal.h" > > -static inline bool vma_is_sealed(struct vm_area_struct *vma) > -{ > - return (vma->vm_flags & VM_SEALED); > -} > - > static inline void set_vma_sealed(struct vm_area_struct *vma) > { > vm_flags_set(vma, VM_SEALED); > } > > -/* > - * check if a vma is sealed for modification. > - * return true, if modification is allowed. > - */ > -static bool can_modify_vma(struct vm_area_struct *vma) > -{ > - if (unlikely(vma_is_sealed(vma))) > - return false; > - > - return true; > -} > - > static bool is_madv_discard(int behavior) > { > switch (behavior) { > diff --git a/mm/vma.h b/mm/vma.h > index 6efdf1768a0a..e979015cc7fc 100644 > --- a/mm/vma.h > +++ b/mm/vma.h > @@ -361,4 +361,32 @@ struct vm_area_struct *vma_iter_prev_range(struct vma_iterator *vmi) > return mas_prev_range(&vmi->mas, 0); > } > > +#ifdef CONFIG_64BIT > + > +static inline bool vma_is_sealed(struct vm_area_struct *vma) > +{ > + return (vma->vm_flags & VM_SEALED); > +} > + > +/* > + * check if a vma is sealed for modification. > + * return true, if modification is allowed. > + */ > +static inline bool can_modify_vma(struct vm_area_struct *vma) > +{ > + if (unlikely(vma_is_sealed(vma))) > + return false; > + > + return true; > +} > + > +#else > + > +static inline bool can_modify_vma(struct vm_area_struct *vma) > +{ > + return true; > +} > + > +#endif > + > #endif /* __MM_VMA_H */ > > -- > 2.46.0 > Thanks for moving to vma.h rather than internal.h! Definitely seems to me to be the correct place for it. Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
diff --git a/mm/mseal.c b/mm/mseal.c index 15bba28acc00..2170e2139ca0 100644 --- a/mm/mseal.c +++ b/mm/mseal.c @@ -16,28 +16,11 @@ #include <linux/sched.h> #include "internal.h" -static inline bool vma_is_sealed(struct vm_area_struct *vma) -{ - return (vma->vm_flags & VM_SEALED); -} - static inline void set_vma_sealed(struct vm_area_struct *vma) { vm_flags_set(vma, VM_SEALED); } -/* - * check if a vma is sealed for modification. - * return true, if modification is allowed. - */ -static bool can_modify_vma(struct vm_area_struct *vma) -{ - if (unlikely(vma_is_sealed(vma))) - return false; - - return true; -} - static bool is_madv_discard(int behavior) { switch (behavior) { diff --git a/mm/vma.h b/mm/vma.h index 6efdf1768a0a..e979015cc7fc 100644 --- a/mm/vma.h +++ b/mm/vma.h @@ -361,4 +361,32 @@ struct vm_area_struct *vma_iter_prev_range(struct vma_iterator *vmi) return mas_prev_range(&vmi->mas, 0); } +#ifdef CONFIG_64BIT + +static inline bool vma_is_sealed(struct vm_area_struct *vma) +{ + return (vma->vm_flags & VM_SEALED); +} + +/* + * check if a vma is sealed for modification. + * return true, if modification is allowed. + */ +static inline bool can_modify_vma(struct vm_area_struct *vma) +{ + if (unlikely(vma_is_sealed(vma))) + return false; + + return true; +} + +#else + +static inline bool can_modify_vma(struct vm_area_struct *vma) +{ + return true; +} + +#endif + #endif /* __MM_VMA_H */
Move can_modify_vma to vma.h so it can be inlined properly (with the intent to remove can_modify_mm callsites). Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com> --- mm/mseal.c | 17 ----------------- mm/vma.h | 28 ++++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 17 deletions(-)