Re: [RFC PATCH v3 0/5] mm, bpf: BPF based THP adjustment

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

On Tue, Jul 22, 2025 at 07:56:21PM +0800, Yafang Shao wrote:
> On Tue, Jul 22, 2025 at 6:09 PM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > On Tue, Jul 22, 2025 at 09:28:02AM +0200, David Hildenbrand wrote:
> > > On 22.07.25 04:40, Yafang Shao wrote:
> > > > On Sun, Jul 20, 2025 at 11:56 PM David Hildenbrand <david@redhat.com> wrote:
> > > > >
> > > > > > >
> > > > > > > We discussed this yesterday at a THP upstream meeting, and what we
> > > > > > > should look into is:
> > > > > > >
> > > > > > > (1) Having a callback like
> > > > > > >
> > > > > > > unsigned int (*get_suggested_order)(.., bool in_pagefault);
> > > > > >
> > > > > > This interface meets our needs precisely, enabling allocation orders
> > > > > > of either 0 or 9 as required by our workloads.
> >
> > That's great to hear, and to be clear my views align with David on this - I
> > feel like having a _carefully chosen_ BPF interface could be valuable here,
> > especially in the short to medium term where it will allow us to more
> > rapidly iterate on an automated [m]THP mechanism.
> >
> > I think one key question here is - do we want to retain a _permanent_ BPF
> > hook here?
> >
> > In any cae, for the first experiments with this we absolutely _must_ be
> > able to express that this is going away, NO, not based on whether it's
> > widely used, it IS going away.
>
> If this BPF kfunc provides clear user value with minimal maintenance
> overhead, what would be the rationale for removing it? That said, if
> you and David both agree it should be deprecated, I won't object -
> though I'd suggest following the standard deprecation process.

You see herein lies the problem... :) from my point of view we want to have
the ability to choose, fundamentally.

We may find out the proposed interface is unworkable, or sets assumptions
we don't want to make.

So I think hiding ehhind a CONFIG_ flag is the best idea here to really
enforce that and make it clear.

Personally I have a sense that we _will_ introduce something permanent. We
just need to have the 'space' to positively decide to do that once the
experimentation is complete.

> > I find this documentation super contradictory. I'm sorry but you can't
> > have:
> >
> > "...can therefore be modified or removed by a maintainer of the subsystem
> >  they’re defined in when it’s deemed necessary."
> >
> > And:
> >
> > "kfuncs that are widely used or have been in the kernel for a long time
> > will be more difficult to justify being changed or removed by a
> > maintainer."
> >
> > At the same time. Let alone:
> >
> > "A kfunc will never have any hard stability guarantees. BPF APIs cannot and
> > will not ever hard-block a change in the kernel purely for stability
> > reasons"
> >
> > Make your mind up!!
> >
> > I mean the EXPORT_SYMBOL_GPL() example isn't accurate AT ALL - we can
> > _absolutely_ change or remove those _at will_ as we don't care about
> > external modules.
> >
> > Really this seems to be saying, in not so many words, that this is
> > basically a kAPI and you can't change it.
> >
> > So this strictly violates what we need here.
>
> The maintainers have the authority to make the final determination ;-)

Well the kernel doesn't entirely work this way... pressure can come which
impacts what others may do.

If you have people saying 'hey we rely on this and removing it will break
our cloud deployment' and 'hey I checked the docs and it says you guys have
to take this into account', I am not so sure Andrew or Linus will accept
the patch.

> > I wonder if we can use a CONFIG_xxx and put this behind that, which
> > specifically says 'WE WILL REMOVE THIS'
> > CONFIG_EXPERIMENTAL_DO_NOT_USE_THP_THINGY :P
>
> That's a reasonable suggestion. We could implement this function under
> CONFIG_EXPERIMENTAL to mark it as experimental infrastructure.

Thanks! Yes, I was looking for this flag :P didn't know if we still had
that or not actually...

But, yeah, putting it behind that explicitly also makes it very clearly.

CONFIG_EXPERIMENTAL_BPF_FAULT_ORDER relies on CONFIG_EXPERIMENTAL makes it
you know... pretty clear ;)

>
> --
> Regards
> Yafang

Cheers, Lorenzo