On Tue, Sep 02, 2025 at 10:48:47AM +0800, Yafang Shao wrote:
> > >
> > > >
> > > > >
> > > > > However, when you switch the THP mode to "never", tasks that still
> > > > > have MMF_VM_HUGEPAGE remain on the khugepaged scan list. This isn’t an
> > > > > issue under the current global mode because khugepaged doesn’t run
> > > > > when THP is set to "never".
> > > > >
> > > > > The problem arises when we move from a global mode to a per-task mode.
> > > > > In that case, khugepaged may end up doing unnecessary work. For
> > > > > example, if the THP mode is "always", but some tasks are not allowed
> > > > > to allocate THP while still having MMF_VM_HUGEPAGE set, khugepaged
> > > > > will continue scanning them unnecessarily.
> > > >
> > > > But this can change right?
> > > >
> > > > I really don't like the idea _at all_ of overriding this hook to do things
> > > > other than what it says it does.
> > > >
> > > > It's 'set which order to use' except when it's this case then it's 'will we
> > > > do any work'.
> > > >
> > > > This should be a separate callback or we should drop this and live with the
> > > > possible additional work.
> > >
> > > Perhaps we could reuse the MMF_DISABLE_THP flag by introducing a new
> > > BPF helper to set it when we want to disable THP for a specific task.
> >
> > Interesting, yeah perhaps that could work, as long as we're in a sensible
> > context to be able to toggle this bit.
>
> Right, we can't set the mm->flags arbitrarily.
> Perhaps we should add a generic BPF hook in dup_mmap().
>
Yeah perhaps that could be a way forward :)
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 7a057e0e8da9..1b60bdb08de1 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1843,6 +1843,8 @@ __latent_entropy int dup_mmap(struct mm_struct
> *mm, struct mm_struct *oldmm)
> loop_out:
> vma_iter_free(&vmi);
> if (!retval) {
> + /* Allow a BPF program to modify the new mm_struct in fork. */
> + bpf_hook_mm_fork(mm, oldmm);
> mt_set_in_rcu(vmi.mas.tree);
> ksm_fork(mm, oldmm);
> khugepaged_fork(mm, oldmm);
>
> This provides a mechanism for BPF programs to configure the new
> mm_struct on demand, acting as a modern, flexible replacement for
> prctl() ;-)
Hahaha that's obviously very appealing to me :)))
>
> >
> > >
> > > Separately from this patchset, I realized we can optimize khugepaged
> > > handling for the MMF_DISABLE_THP case with the following changes:
> > >
> > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > > index 15203ea7d007..e9964edcee29 100644
> > > --- a/mm/khugepaged.c
> > > +++ b/mm/khugepaged.c
> > > @@ -402,6 +402,11 @@ void __init khugepaged_destroy(void)
> > > kmem_cache_destroy(mm_slot_cache);
> > > }
> > >
> > > +static inline int hpage_collapse_test_disable(struct mm_struct *mm)
> > > +{
> > > + return test_bit(MMF_DISABLE_THP, &mm->flags);
> > > +}
> > > +
> > > static inline int hpage_collapse_test_exit(struct mm_struct *mm)
> > > {
> > > return atomic_read(&mm->mm_users) == 0;
> > > @@ -1448,6 +1453,11 @@ static void collect_mm_slot(struct
> > > khugepaged_mm_slot *mm_slot)
> > > /* khugepaged_mm_lock actually not necessary for the below */
> > > mm_slot_free(mm_slot_cache, mm_slot);
> > > mmdrop(mm);
> > > + } else if (hpage_collapse_test_disable(mm)) {
> > > + hash_del(&slot->hash);
> > > + list_del(&slot->mm_node);
> > > + mm_flags_clear(MMF_VM_HUGEPAGE, mm);
> > > + mm_slot_free(mm_slot_cache, mm_slot);
> > > }
> > > }
> > >
> > > Specifically, if MMF_DISABLE_THP is set, we should remove it from
> > > mm_slot to prevent unnecessary khugepaged processing.
> >
> > Ohhh interesting, perhaps send as separate patch?
>
> sure, I will send it separately.
Thanks!
>
> --
> Regards
> Yafang
And overall - cheers for being an ABSOLUTE DELIGHT on review :) it's much
appreciated. I shall buy you a beer (or whatever is your preferred
beverage) at the next conference we are both at :)
Cheers, Lorenzo
