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.
--
Regards
Yafang
