Re: [PATCH 1/6] prctl: introduce PR_THP_POLICY_DEFAULT_HUGE for the process

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Thu, May 15, 2025 at 09:12:13PM +0200, David Hildenbrand wrote:
> On 15.05.25 20:08, Lorenzo Stoakes wrote:
> > On Thu, May 15, 2025 at 06:11:55PM +0200, David Hildenbrand wrote:
> > > > > > So if you're not overriding VM_NOHUGEPAGE, the whole point of this exercise
> > > > > > is to override global 'never'?
> > > > > >
> > > > >
> > > > > Again, I am not overriding never.
> > > > >
> > > > > hugepage_global_always and hugepage_global_enabled will evaluate to false
> > > > > and you will not get a hugepage.
> > > >
> > > > Yeah, again ack, but I kind of hate that we set VM_HUGEPAGE everywhere even
> > > > if the policy is never.
> > >
> > > I think it should behave just as if someone does manually an madvise(). So
> > > whatever we do here during an madvise, we should try to do the same thing
> > > here.
> >
> > Ack I agree with this.
> >
> > It actually simplifies things a LOT to view it this way - we're saying 'by
> > default apply madvise(...) to new VMAs'.
> >
> > Hm I wonder if we could have a more generic version of this...
> >
> > Note though that we're not _quite_ doing this.
> >
> > So in hugepage_madvise():
> >
> > int hugepage_madvise(struct vm_area_struct *vma,
> > 		     unsigned long *vm_flags, int advice)
> > {
> > 	...
> >
> > 	switch (advice) {
> > 	case MADV_HUGEPAGE:
> > 		*vm_flags &= ~VM_NOHUGEPAGE;
> > 		*vm_flags |= VM_HUGEPAGE;
> >
> > 		...
> >
> > 		break;
> >
> > 		...
> > 	}
> >
> > 	...
> > }
> >
> > So here we're actually clearing VM_NOHUGEPAGE and overriding it, but in the
> > proposed code we're not.
>
> Yeah, I think I suggested that, but probably we should just do exactly what
> madvise() does.

Yes, agreed.

Usama - do you have any issue with us switching to how madvise() does it?

>
> >
> > So we're back into confusing territory again :)
> >
> > I wonder if we could...
> >
> > 1. Add an MADV_xxx that mimics the desired behaviour here.
> >
> > 2. Add a generic 'madvise() by default' thing at a process level?
> >
> > Is this crazy?
>
> I think that's what I had in mind, just a bit twisted.
>
> What could work is
>
> 1) prctl to set the default
>
> 2) madvise() to adjust all existing VMAs
>
>
> We might have to teach 2) to ignore non-compatible VMAs / holes. Maybe not,
> worth an investigation.

Yeah, I think it'd _probably_ be ok except on s390 (which can fail, and so
we'd have to be able to say - skip on error, carry on).

We'll just get an -ENOMEM at the end for the gaps (god how I hate
that). Otherwise I don't think MADV_HUGEPAGE actually is really that
restrictive.

That would simplify :)

But I still so hate using prctl()... this might be one of those cases where
we simply figure out we have no other choice.

But when you put it as simply as this maybe it's not so bad. With the
flags2 gone by fixing this stupid 32-bit limit it's less awful.

Perhaps worth seeing what an improved RFC of this series looks like with
all the various bits fixed to give an idea.

But you do then wonder if we could make this _generic_ for _any_ madvise(),
and how _that_ would look.

But perhaps that's insane because many VMAs would simply not be suited to
having certain madvise flags set hmm.

Maybe let me have a think about an improved madvise() interface along these
lines anyway in general... interesting thought experiment :)

>
>
> --
> Cheers,
>
> David / dhildenb
>