Re: [PATCH v1] ALSA: memalloc: Fix indefinite hang in non-iommu case

[Hillf Danton]

On Fri, 16 Feb 2024 09:35:32 +0100 Takashi Iwai <tiwai@xxxxxxx> wrote:
> On Fri, 16 Feb 2024 05:34:24 +0100, Hillf Danton wrote:
> > On Thu, 15 Feb 2024 18:03:01 +0100 Takashi Iwai <tiwai@xxxxxxx> wrote:
> > > So it sounds like that we should go back for __GFP_NORETRY in general
> > > for non-zero order allocations, not only the call you changed, as
> > > __GFP_RETRY_MAYFAIL doesn't guarantee the stuck.
> > > 
> > > How about the changes like below?
> > > 
> > > +/* default GFP bits for our allocations */
> > > +static gfp_t default_gfp(size_t size)
> > > +{
> > > +	/* don't allocate intensively for high-order pages */
> > > +	if (size > PAGE_SIZE)
> > > +		return GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY;
> > > +	else
> > > +		return GFP_KERNEL | __GFP_NOWARN | __GFP_RETRY_MAYFAIL;
> > > +}
> > 
> > Looks like an overdose because both __GFP_NORETRY and __GFP_RETRY_MAYFAIL
> > are checked in __alloc_pages_slowpath().
> 
> If the check there worked as expected, this shouldn't have been a
> problem, no?
> 
> The fact that we have to drop __GFP_RETRY_MAYFAIL indicates that the
> handling there doesn't suffice -- at least for the audio operation.

Dropping the retry gfp flag makes no sense without checking fallback in
sound allocation context in this thread, nor checking it without checking
the costly order in the page allocator.
OTOH page allocator will never work in every corner case particularly
where perfermance/response is expected given insanely heavy fragmentation
background.
Fragmentation [1] is not anything new, nor is it a cure to fiddle with a
couple flags.

[1] https://lore.kernel.org/lkml/20230418191313.268131-1-hannes@xxxxxxxxxxx/