On Thu, Dec 05, 2024 at 11:25:43PM -0500, Liam R. Howlett wrote:
> * jeffxu@xxxxxxxxxxxx <jeffxu@xxxxxxxxxxxx> [241205 20:39]:
> > From: Jeff Xu <jeffxu@xxxxxxxxxxxx>
> >
> > No code logic change.
> >
> > can_do_mseal is called exclusively by mseal.c,
> > and mseal.c is compiled only when CONFIG_64BIT flag is
> > set in makefile. Therefore, it is unnecessary to have
> > 32 bit stub function in the header file.
>
> There is no reason to keep this function at all; it is used in one
> place, and that place uses three lines of code as well.
>
> In fact, having it separate from the comment about flags being reserved
> makes the function very puzzling.
I entirely agree. Jeff - please just make this inline to do_mseal():
...
/* Flags are reserved. */
if (flags)
retrun -EINVAL;
...
If you do that then cool I'm happy for this patch to be taken.
An aside - I actually think we need to move the bulk of this code to
mm/vma.c - it makes absolutely no sense to keep the internals in this file,
and that way we can userland test mseal functionality.
I may submit a patch to this effect at some point.
Thanks, Lorenzo
>
> >
> > Signed-off-by: Jeff Xu <jeffxu@xxxxxxxxxxxx>
> > ---
> > mm/internal.h | 16 ----------------
> > mm/mseal.c | 8 ++++++++
> > 2 files changed, 8 insertions(+), 16 deletions(-)
> >
> > diff --git a/mm/internal.h b/mm/internal.h
> > index 74dc1c48fa31..5e4ef5ce9c0a 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -1457,22 +1457,6 @@ void __meminit __init_single_page(struct page *page, unsigned long pfn,
> > unsigned long shrink_slab(gfp_t gfp_mask, int nid, struct mem_cgroup *memcg,
> > int priority);
> >
> > -#ifdef CONFIG_64BIT
> > -static inline int can_do_mseal(unsigned long flags)
> > -{
> > - if (flags)
> > - return -EINVAL;
> > -
> > - return 0;
> > -}
> > -
> > -#else
> > -static inline int can_do_mseal(unsigned long flags)
> > -{
> > - return -EPERM;
> > -}
> > -#endif
> > -
> > #ifdef CONFIG_SHRINKER_DEBUG
> > static inline __printf(2, 0) int shrinker_debugfs_name_alloc(
> > struct shrinker *shrinker, const char *fmt, va_list ap)
> > diff --git a/mm/mseal.c b/mm/mseal.c
> > index 81d6e980e8a9..e167220a0bf0 100644
> > --- a/mm/mseal.c
> > +++ b/mm/mseal.c
> > @@ -158,6 +158,14 @@ static int apply_mm_seal(unsigned long start, unsigned long end)
> > return 0;
> > }
> >
> > +static inline int can_do_mseal(unsigned long flags)
It makes no sense for this to be inline.
> > +{
> > + if (flags)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * mseal(2) seals the VM's meta data from
> > * selected syscalls.
> > --
> > 2.47.0.338.g60cca15819-goog
> >
