Re: [PATCH 1/6] mm: Rename try_alloc_pages() to alloc_pages_nolock()

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Tue, May 6, 2025 at 1:26 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 5/1/25 05:27, Alexei Starovoitov wrote:
> > From: Alexei Starovoitov <ast@kernel.org>
> >
> > The "try_" prefix is confusing, since it made people believe
> > that try_alloc_pages() is analogous to spin_trylock() and
> > NULL return means EAGAIN. This is not the case. If it returns
> > NULL there is no reason to call it again. It will most likely
> > return NULL again. Hence rename it to alloc_pages_nolock()
> > to make it symmetrical to free_pages_nolock() and document that
> > NULL means ENOMEM.
> >
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
>
> > @@ -7378,20 +7378,21 @@ static bool __free_unaccepted(struct page *page)
> >  #endif /* CONFIG_UNACCEPTED_MEMORY */
> >
> >  /**
> > - * try_alloc_pages - opportunistic reentrant allocation from any context
> > + * alloc_pages_nolock - opportunistic reentrant allocation from any context
> >   * @nid: node to allocate from
> >   * @order: allocation order size
> >   *
> >   * Allocates pages of a given order from the given node. This is safe to
> >   * call from any context (from atomic, NMI, and also reentrant
> > - * allocator -> tracepoint -> try_alloc_pages_noprof).
> > + * allocator -> tracepoint -> alloc_pages_nolock_noprof).
> >   * Allocation is best effort and to be expected to fail easily so nobody should
> >   * rely on the success. Failures are not reported via warn_alloc().
> >   * See always fail conditions below.
> >   *
> > - * Return: allocated page or NULL on failure.
> > + * Return: allocated page or NULL on failure. NULL does not mean EBUSY or EAGAIN.
> > + * It means ENOMEM. There is no reason to call it again and expect !NULL.
>
> Should we explain that the "ENOMEM" doesn't necessarily mean the system is
> out of memory, but also that the calling context might be simply unlucky
> (preempted someone with the lock) and retrying in the same context can't
> help it?

Technically correct, but it opens the door for "retry" thinking:
"I called it and got unlucky, maybe I should retry once.. I promise
I won't loop forever".
So I really think the doc should say "ENOMEM. no reason to retry" like above.