Re: [PATCH 4.19.y] mm, vmscan: prevent infinite loop for costly GFP_NOIO | __GFP_RETRY_MAYFAIL allocations

Greg KH
2024-04-05T09:26:39Z

On Thu, Apr 04, 2024 at 05:33:16PM +0200, Vlastimil Babka wrote:
> Sven reports an infinite loop in __alloc_pages_slowpath() for costly order
> __GFP_RETRY_MAYFAIL allocations that are also GFP_NOIO. Such combination
> can happen in a suspend/resume context where a GFP_KERNEL allocation can
> have __GFP_IO masked out via gfp_allowed_mask.
>
> Quoting Sven:
>
> 1. try to do a "costly" allocation (order > PAGE_ALLOC_COSTLY_ORDER)
> with __GFP_RETRY_MAYFAIL set.
>
> 2. page alloc's __alloc_pages_slowpath 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 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 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 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.
>
> (end quote)
>
> The immediate root cause is confusing the COMPACT_SKIPPED returned from
> __alloc_pages_direct_compact() (step 4) due to lack of __GFP_IO to be
> indicating a lack of order-0 pages, and in step 5 evaluating that in
> should_compact_retry() as a reason to retry, before incrementing and
> limiting the number of retries. There are however other places that
> wrongly assume that compaction can happen while we lack __GFP_IO.
>
> To fix this, introduce gfp_compaction_allowed() to abstract the __GFP_IO
> evaluation and switch the open-coded test in try_to_compact_pages() to use
> it.
>
> Also use the new helper in:
> - compaction_ready(), which will make reclaim not bail out in step 3, so
> there's at least one attempt to actually reclaim, even if chances are
> small for a costly order
> - in_reclaim_compaction() which will make should_continue_reclaim()
> return false and we don't over-reclaim unnecessarily
> - in __alloc_pages_slowpath() to set a local variable can_compact,
> which is then used to avoid retrying reclaim/compaction for costly
> allocations (step 5) if we can't compact and also to skip the early
> compaction attempt that we do in some cases
>
> Link: https://lkml.kernel.org/r/20240221114357.13655-2-vbabka@suse.cz
> Fixes: 3250845d0526 ("Revert "mm, oom: prevent premature OOM killer invocation for high order request"")
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> Reported-by: Sven van Ashbrook <svenva@chromium.org>
> Closes: https://lore.kernel.org/all/CAG-rBihs_xMKb3wrMO1%2B-%2Bp4fowP9oy1pa_OTkfxBzPUVOZF%2Bg@mail.gmail.com/
> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org>
> Cc: Brian Geffon <bgeffon@google.com>
> Cc: Curtis Malainey <cujomalainey@chromium.org>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> (cherry picked from commit 803de9000f334b771afacb6ff3e78622916668b0)
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>

All backports now queued up, thanks!

greg k-h