lkml.org 
[lkml]   [2010]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 7/8] mm: compaction: Use the LRU to get a hint on where compaction should start
    On Wed, Nov 17, 2010 at 04:22:48PM +0000, Mel Gorman wrote:
    > + if (!cc->migrate_pfn)
    > + cc->migrate_pfn = zone->zone_start_pfn;

    wouldn't it remove a branch if the caller always set migrate_pfn?

    > + if (!cc->free_pfn) {
    > + cc->free_pfn = zone->zone_start_pfn + zone->spanned_pages;
    > + cc->free_pfn &= ~(pageblock_nr_pages-1);
    > + }

    Who sets free_pfn to zero? Previously this was always initialized.

    > @@ -523,7 +539,23 @@ unsigned long reclaimcompact_zone_order(struct zone *zone,
    > INIT_LIST_HEAD(&cc.freepages);
    > INIT_LIST_HEAD(&cc.migratepages);
    >
    > - return compact_zone(zone, &cc);
    > + /* Get a hint on where to start compacting from the LRU */
    > + anon_page = lru_to_page(&zone->lru[LRU_BASE + LRU_INACTIVE_ANON].list);
    > + file_page = lru_to_page(&zone->lru[LRU_BASE + LRU_INACTIVE_FILE].list);
    > + cc.migrate_pfn = min(page_to_pfn(anon_page), page_to_pfn(file_page));
    > + cc.migrate_pfn = ALIGN(cc.migrate_pfn, pageblock_nr_pages);
    > + start_migrate_pfn = cc.migrate_pfn;
    > +
    > + ret = compact_zone(zone, &cc);
    > +
    > + /* Restart migration from the start of zone if the hint did not work */
    > + if (!zone_watermark_ok(zone, cc.order, low_wmark_pages(zone), 0, 0)) {
    > + cc.migrate_pfn = 0;
    > + cc.abort_migrate_pfn = start_migrate_pfn;
    > + ret = compact_zone(zone, &cc);
    > + }
    > +

    I doubt it works ok if the list is empty... Maybe it's safer to
    validate the migrate_pfn against the zone pfn start/end before
    setting it in the migrate_pfn.

    Interesting this heuristic slowed down the benchmark, it should lead
    to the exact opposite thanks to saving some cpu. So I guess maybe it's
    not worth it. I see it increases the ratio of compaction of a tiny
    bit, but if a tiny bit of better compaction comes at the expenses of
    an increased runtime I don't like it and I'd drop it... It's not
    making enough difference, further we could extend it to check the
    "second" page in the list and so on... so we can just go blind. All it
    matters is that we use a clock algorithm and I guess this screwes it
    and this is why it leads to increased time.


    \
     
     \ /
      Last update: 2010-11-18 19:49    [W:0.021 / U:30.560 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site