On 1/14/25 13:24, Ge Yang wrote: >> Hopefully also when done from the pin_user_pages_remote(..., FOLL_LONGTERM, >> ...) context the allocation gfp_mask correctly lacks __GFP_MOVABLE? > yes. > I guess >> it has to, otherwise it would allocate from the CMA pageblocks. >> >> Then I wonder if we could use the real allocation context to determine >> watermarks, as __compaction_suitable() is passing ALLOC_CMA instead because >> it's checking only for migration targets, which have to be CMA compatible by >> definition. But we could use the real unmovable allocation context to have >> __zone_watermark_unusable_free() subtract CMA pages, and thus we won't pass >> the order-0 check anymore once the non-CMA part is exhausted. >> >> There's some risk that in some different scenario the compaction could in >> fact migrate pages from the exhausted non-CMA part of the zone to the CMA >> part and succeed, and we'll skip it instead. But that should be rare? >> > Below is the previous discussion： > https://lore.kernel.org/lkml/1734436004-1212-1-git-send-email-yangge1116@xxxxxxx/ Right so Johannes had the same concern. >> Anyway given that concern I'm not sure about changing >> __compaction_suitable() for every caller like this. We could (at least >> initially) target this heuristic only for COMPACT_PRIO_ASYNC which is being >> used for this THP opportunistic attempt. >> >> So for example: >> - add a new bool flag to compact_control that is true for COMPACT_PRIO_ASYNC >> - pass cc pointer to compaction_suit_allocation_order() >> - in that function, add another check if the the new cc flag is true, >> between the current zone_watermark_ok() and compaction_suitable() checks, >> which works like __compaction_suitable() but uses alloc_flags (which should >> not be ALLOC_CMA in our pinned allocation case) instead of ALLOC_CMA, return >> COMPACT_SKIPPED if it fails. >> > I will send a new version of the patch based on the suggestions here. > Thank you. Yeah that way should hopefully limit the concern sufficiently. Maybe we could also add costly_order condition in addition to COMPACT_PRIO_ASYNC condition to set the new compact_control flag. But only __GFP_NORETRY allocations should be affected in the immediate "goto nopage" when compaction is skipped, others will attempt with DEF_COMPACT_PRIORITY anyway and won't fail without trying to compact-migrate the non-CMA pageblocks into CMA pageblocks first, so it should be fine.
