Date: Wed, 21 Feb 2024 10:58:26 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: Sven van Ashbrook <svenva@...omium.org>
Cc: Takashi Iwai <tiwai@...e.de>,
 Karthikeyan Ramasubramanian <kramasub@...omium.org>,
 LKML <linux-kernel@...r.kernel.org>, Brian Geffon <bgeffon@...gle.com>,
 stable@...r.kernel.org, Curtis Malainey <cujomalainey@...omium.org>,
 Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
 linux-sound@...r.kernel.org, linux-mm@...ck.org
Subject: Re: Stall at page allocations with __GFP_RETRY_MAYFAIL (Re: [PATCH
 v1] ALSA: memalloc: Fix indefinite hang in non-iommu case)

On 2/20/24 16:52, Sven van Ashbrook wrote:
> Takaski, Vlastimil: thanks so much for the engagement! See below.
> 
>> On 2/19/24 12:36, Takashi Iwai wrote:
>> >
>> > Karthikeyan, Sven, and co: could you guys show the stack trace at the
>> > stall?  This may give us more clear light.
> Here are our notes of the indefinite stall we saw on v5.10 with iommu SoCs.
> We did not pursue debugging the stall at the time, in favour of a work-around
> with the gfp flags. Therefore we only have partial confidence in the notes
> below. Take them with a block of salt, but they may point in a useful direction.
> 
> 1. try to do a "costly" allocation (order > PAGE_ALLOC_COSTLY_ORDER) with
>     __GFP_RETRY_MAYFAIL set.
> 
> 2. page alloc's __alloc_pages_slowpath [1] tries to get a page from
> the freelist.
>     This fails because there is nothing free of that costly order.
> 
> 3. page alloc tries to reclaim by calling __alloc_pages_direct_reclaim, which
>     bails out [2] because a zone is ready to be compacted; it pretends
> to have made
>     a single page of progress.
> 
> 4. page alloc tries to compact, but this always bails out early [3]
> because __GFP_IO is not set
>     (it's not passed by the snd allocator, and even if it were, we are
> suspending so the
>     __GFP_IO flag would be cleared anyway).
> 
> 5. page alloc believes reclaim progress was made (because of the
> pretense in item 3) and
>     so it checks whether it should retry compaction. The compaction
> retry logic [4] thinks
>     it should try again, because:
>     a) reclaim is needed because of the early bail-out in item 4
>     b) a zonelist is suitable for compaction
> 
> 6. goto 2. indefinite stall.

Thanks a lot, seems this can indeed happen even in 6.8-rc5. We're
mishandling the case where compaction is skipped due to lack of __GFP_IO,
which is indeed cleared in suspend/resume. I'll create a fix. Please don't
hesitate to report such issues the next time, even if not fully debugged :)

>>
>> > Also, Vlastimil suggested that tracepoints would be helpful if that's
>> > really in the page allocator, too.
>> >
> 
> We might be able to generate traces by bailing out of the indefinite
> stall using a timer,
> which should hopefully give us a device that's "alive enough" to read
> the traces.
> 
> Can you advise which tracepoints you'd like to see? Is trace-cmd [5]
> suitable to capture
> this?
> 
> [1] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/v5.10/mm/page_alloc.c;l=4654;drc=a16293af64a1f558dab9a5dd7fb05fdbc2b7c5c0
> [2] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/v5.10/mm/vmscan.c;drc=44452e4236561f6e36ec587805a52b683e2804c9;l=6177
> [3] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/v5.10/mm/compaction.c;l=2479;drc=d7b105aa1559e6c287f3f372044c21c7400b7784
> [4] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/v5.10/mm/page_alloc.c;l=4171;drc=a16293af64a1f558dab9a5dd7fb05fdbc2b7c5c0
> [5] https://chromium.googlesource.com/chromiumos/docs/+/HEAD/kernel_development.md#ftrace-debugging

Powered by blists - more mailing lists