• Barry Song's avatar
    mm: compaction: avoid fast_isolate_freepages blindly choose improper pageblock · d19b1a17
    Barry Song authored
    Testing shows fast_isolate_freepages can blindly choose an unsuitable
    pageblock from time to time particularly while the min mark is used from
    XXX path:
    
     if (!page) {
             cc->fast_search_fail++;
             if (scan_start) {
                     /*
                      * Use the highest PFN found above min. If one was
                      * not found, be pessimistic for direct compaction
                      * and use the min mark.
                      */
                     if (highest >= min_pfn) {
                             page = pfn_to_page(highest);
                             cc->free_pfn = highest;
                     } else {
                             if (cc->direct_compaction && pfn_valid(min_pfn)) { /* XXX */
                                     page = pageblock_pfn_to_page(min_pfn,
                                             min(pageblock_end_pfn(min_pfn),
                                                 zone_end_pfn(cc->zone)),
                                             cc->zone);
                                     cc->free_pfn = min_pfn;
                             }
                     }
             }
     }
    
    The reason is that no code is doing any check on the min_pfn
     min_pfn = pageblock_start_pfn(cc->free_pfn - (distance >> 1));
    
    In contrast, slow path of isolate_freepages() is always skipping
    unsuitable pageblocks in a decent way.
    
    This issue doesn't happen quite often.  When running 25 machines with
    16GiB memory for one night, most of them can hit this unexpected code
    path.  However the frequency isn't like many times per second.  It might
    be one time in a couple of hours.  Thus, it is very hard to measure the
    visible performance impact in my machines though the affection of choosing
    the unsuitable migration_target should be negative in theory.
    
    I feel it's still worth fixing this to at least make the code
    theoretically self-explanatory as it is quite odd an unsuitable
    migration_target can be still migration_target.
    
    Link: https://lkml.kernel.org/r/20231206110054.61617-1-v-songbaohua@oppo.comSigned-off-by: default avatarBarry Song <v-songbaohua@oppo.com>
    Reported-by: default avatarZhanyuan Hu <huzhanyuan@oppo.com>
    Reviewed-by: default avatarBaolin Wang <baolin.wang@linux.alibaba.com>
    Cc: David Hildenbrand <david@redhat.com>
    Cc: Johannes Weiner <hannes@cmpxchg.org>
    Cc: Kemeng Shi <shikemeng@huaweicloud.com>
    Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
    Cc: Mel Gorman <mgorman@techsingularity.net>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    d19b1a17
compaction.c 89.2 KB