On Thu, Jul 17, 2025 at 11:30 PM Usama Arif <usamaarif642@xxxxxxxxx> wrote:
>
>
>
> On 08/06/2025 08:35, Yafang Shao wrote:
> > A new hook bpf_thp_allocator() is added to determine if the THP is
> > allocated by khugepaged or by the current task.
>
> I would add in the summary why we need this. I am assuming I will find out
> when reviewing the next few patches, but would be good to know here.
>
> >
> > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> > ---
> > include/linux/huge_mm.h | 10 ++++++++++
> > mm/khugepaged.c | 2 ++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> > index 2f190c90192d..db2eadd3f65b 100644
> > --- a/include/linux/huge_mm.h
> > +++ b/include/linux/huge_mm.h
> > @@ -190,6 +190,14 @@ static inline bool hugepage_global_always(void)
> > (1<<TRANSPARENT_HUGEPAGE_FLAG);
> > }
> >
> > +#define THP_ALLOC_KHUGEPAGED (1 << 1)
> > +#define THP_ALLOC_CURRENT (1 << 2)
> > +static inline int bpf_thp_allocator(unsigned long vm_flags,
> > + unsigned long tva_flags)
> > +{
> > + return THP_ALLOC_KHUGEPAGED | THP_ALLOC_CURRENT;
>
> You dont use either vm_flags or tva_flags in this function?
> I am guessing you wanted to check if these bits are set here?
>
>
> But you dont seem to be setting these flags anywhere? I am guessing
> its in a future patch. If it is, I would move the setting of these bits
> here as its confusing to only see the check without knowing where its
>
> I feel this patch is broken and needs to be rewritten.
The `bpf_thp_allocator()` function serves as a placeholder that will
be implemented by a BPF program. The BPF implementation will use the
`@vm_flags` and `@tva_flags` parameters to determine whether a task
qualifies for THP allocation.
I see how this could be confusing.
> > +}
> > +
> > static inline int highest_order(unsigned long orders)
> > {
> > return fls_long(orders) - 1;
> > @@ -290,6 +298,8 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,
> > if ((tva_flags & TVA_ENFORCE_SYSFS) && vma_is_anonymous(vma)) {
> > unsigned long mask = READ_ONCE(huge_anon_orders_always);
> >
> > + if (!(bpf_thp_allocator(vm_flags, tva_flags) & THP_ALLOC_CURRENT))
> > + return 0;
>
> I am assuming that this is the point to check for allocation, but thp_vma_allowable_orders
> is not just used for allocation, its used for in other places as well, like hugepage_vma_revalidate
> and swap.
Agreed, some adjustments are necessary.
--
Regards
Yafang
