Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] mm: async compaction clears skip hints causing full zone rescans - [Low] mm: missing tracepoint for async compaction deferral reset --
commit 2e6082a2f3b4a8dcf4b7cf16ef03d8f061912e07 Author: Qiliang Yuan <[email protected]> Date: Thu Oct 1 15:33:49 2026 +0000 mm/compaction: defer failed async direct compaction This patch splits the compaction deferral state into sync and async, allowing async compaction to be deferred independently of sync compaction. It aims to prevent unnecessary zone rescans during THP allocations by keeping async deferral state. > diff --git a/mm/compaction.c b/mm/compaction.c > index a049415512c67..25f758bc54f06 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c [ ... ] > @@ -165,24 +165,28 @@ static bool compaction_deferred(struct zone *zone, int > order) > void compaction_defer_reset(struct zone *zone, int order, > bool alloc_success) > { > - if (alloc_success) { > - zone->compact_considered = 0; > - zone->compact_defer_shift = 0; > - } > - if (order >= zone->compact_order_failed) > - zone->compact_order_failed = order + 1; > + int sync; > > - trace_mm_compaction_defer_reset(zone, order); > + for (sync = 0; sync < ASYNC_AND_SYNC; sync++) { > + if (alloc_success) { > + zone->compact_considered[sync] = 0; > + zone->compact_defer_shift[sync] = 0; > + } > + if (order >= zone->compact_order_failed[sync]) > + zone->compact_order_failed[sync] = order + 1; > + } > + > + trace_mm_compaction_defer_reset(zone, order, true); [Severity: Low] Does this tracepoint unintentionally omit the async state? The tracepoint unconditionally reports a reset for sync compaction (true), but entirely omits the corresponding event for the async state that was also modified in the loop. > } > > -/* Returns true if restarting compaction after many failures */ > +/* Returns true if restarting sync compaction after many failures */ > static bool compaction_restarting(struct zone *zone, int order) > { > - if (order < zone->compact_order_failed) > + if (order < zone->compact_order_failed[true]) > return false; > > - return zone->compact_defer_shift == COMPACT_MAX_DEFER_SHIFT && > - zone->compact_considered >= 1UL << zone->compact_defer_shift; > + return zone->compact_defer_shift[true] == COMPACT_MAX_DEFER_SHIFT && > + zone->compact_considered[true] >= 1UL << > zone->compact_defer_shift[true]; > } [ ... ] > @@ -2596,7 +2600,9 @@ compact_zone(struct compact_control *cc, struct > capture_control *capc) > > /* > * Clear pageblock skip if there were failures recently and compaction > - * is about to be retried after being deferred. > + * is about to be retried after being deferred. Only do it when sync > + * compaction restarts: async compaction relies on the skip hints, and > + * clearing them on every async retry would rescan the whole zone. > */ > if (compaction_restarting(cc->zone, cc->order)) > __reset_isolation_suitable(cc->zone); [Severity: High] Does this code clear skip hints during async compaction, causing full zone rescans? The comment explicitly states: "Only do it when sync compaction restarts". However, compaction_restarting() only evaluates the sync deferral state. If sync compaction has been fully deferred (compact_defer_shift[true] == COMPACT_MAX_DEFER_SHIFT), this will return true for all callers. Without checking if the current compaction is actually synchronous (e.g., via cc->mode != MIGRATE_ASYNC), wouldn't this cause async compaction to continuously clear skip hints and rescan the entire zone on every failed THP fault? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20261001-bug-mm-thp-async-compact-defer-v1-0-0174c7923...@gmail.com?part=2
