On Fri, Sep 12, 2025 at 04:03:32PM +0800, Yafang Shao wrote:
> On Thu, Sep 11, 2025 at 10:51 PM Lorenzo Stoakes
> <lorenzo.stoakes@xxxxxxxxxx> wrote:
> >
> > On Wed, Sep 10, 2025 at 10:44:39AM +0800, Yafang Shao wrote:
> > > diff --git a/mm/huge_memory_bpf.c b/mm/huge_memory_bpf.c
> > > new file mode 100644
> > > index 000000000000..525ee22ab598
> > > --- /dev/null
> > > +++ b/mm/huge_memory_bpf.c
> >
> > [snip]
> >
> > > +unsigned long bpf_hook_thp_get_orders(struct vm_area_struct *vma,
> > > + vm_flags_t vma_flags,
> > > + enum tva_type tva_type,
> > > + unsigned long orders)
> > > +{
> > > + thp_order_fn_t *bpf_hook_thp_get_order;
> > > + unsigned long thp_orders = orders;
> > > + enum bpf_thp_vma_type vma_type;
> > > + int thp_order;
> > > +
> > > + /* No BPF program is attached */
> > > + if (!test_bit(TRANSPARENT_HUGEPAGE_BPF_ATTACHED,
> > > + &transparent_hugepage_flags))
> > > + return orders;
> > > +
> > > + if (vma_flags & VM_HUGEPAGE)
> > > + vma_type = BPF_THP_VM_HUGEPAGE;
> > > + else if (vma_flags & VM_NOHUGEPAGE)
> > > + vma_type = BPF_THP_VM_NOHUGEPAGE;
> > > + else
> > > + vma_type = BPF_THP_VM_NONE;
> > > +
> > > + rcu_read_lock();
> > > + bpf_hook_thp_get_order = rcu_dereference(bpf_thp.thp_get_order);
> > > + if (!bpf_hook_thp_get_order)
> > > + goto out;
> > > +
> > > + thp_order = bpf_hook_thp_get_order(vma, vma_type, tva_type, orders);
> > > + if (thp_order < 0)
> > > + goto out;
> > > + /*
> > > + * The maximum requested order is determined by the callsite. E.g.:
> > > + * - PMD-mapped THP uses PMD_ORDER
> > > + * - mTHP uses (PMD_ORDER - 1)
> > > + *
> > > + * We must respect this upper bound to avoid undefined behavior. So the
> > > + * highest suggested order can't exceed the highest requested order.
> > > + */
> > > + if (thp_order <= highest_order(orders))
> > > + thp_orders = BIT(thp_order);
> >
> > OK so looking at Lance's reply re: setting 0 and what we're doing here in
> > general - this seems a bit weird to me.
> >
> > Shouldn't orders be specifying a _mask_ as to which orders are _available_,
> > rather than allowing a user to specify an arbitrary order?
> >
> > So if you're a position where the only possible order is PMD sized, now this
> > would let you arbitrarily select an mTHP right? That does no seem correct.
> >
> > And as per Lance, if we cannot satisfy the requested order, we shouldn't fall
> > back to available orders, we should take that as a signal that we cannot have
> > THP at all.
> >
> > So shouldn't this just be:
> >
> > thp_orders = orders & BIT(thp_order);
>
> That's better. I will change it.
Great I agree (obviously :P)
Thanks!
>
> --
> Regards
> Yafang
Cheers, Lorenzo
