On Wed, Dec 18, 2024 at 4:10 PM Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote:
>
> On Tue, Dec 17, 2024 at 07:07:14PM -0800, alexei.starovoitov@xxxxxxxxx wrote:
> > From: Alexei Starovoitov <ast@xxxxxxxxxx>
> >
> [...]
> > +
> > +struct page *try_alloc_pages_noprof(int nid, unsigned int order)
> > +{
> > + gfp_t alloc_gfp = __GFP_NOWARN | __GFP_ZERO |
> > + __GFP_NOMEMALLOC | __GFP_TRYLOCK;
>
> I think the above needs a comment to be more clear. Basically why zero,
> nomemalloc and no warn? Otherwise this looks good. I don't have a strong
> opinion on __GFP_TRYLOCK and maybe just ALLOC_TRYLOCK would be good
> enough.
__GFP_NOWARN is what bpf uses almost everywhere. There is never a reason
to warn. Also warn means printk() which is unsafe from various places.
Cannot really use printk_deferred_enter() either. The running ctx is unknown.
__GFP_ZERO is to make sure that call to kmsan_alloc_page() is safe
and it's necessary for bpf anyway.
Initially __GFP_NOMEMALLOC was added to avoid ALLOC_HIGHATOMIC paths
when __alloc_pages_slowpath() was still there in try_alloc_pages().
Later the slowpath was removed, but I left __GFP_NOMEMALLOC
as a self-explanatory statement that try_alloc_pages()
doesn't want to deplete reserves.
I'll add a comment in the next version.
