Messages in this thread | | | Subject | Re: [PATCH v2 1/9] mm/compaction: skip useless pfn when updating cached pfn | From | Vlastimil Babka <> | Date | Mon, 24 Aug 2015 11:07:12 +0200 |
| |
On 08/24/2015 04:19 AM, Joonsoo Kim wrote: > Cached pfn is used to determine the start position of scanner > at next compaction run. Current cached pfn points the skipped pageblock > so we uselessly checks whether pageblock is valid for compaction and > skip-bit is set or not. If we set scanner's cached pfn to next pfn of > skipped pageblock, we don't need to do this check. > > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com> > --- > mm/compaction.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index 6ef2fdf..c2d3d6a 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -261,10 +261,9 @@ void reset_isolation_suitable(pg_data_t *pgdat) > */ > static void update_pageblock_skip(struct compact_control *cc, > struct page *page, unsigned long nr_isolated, > - bool migrate_scanner) > + unsigned long pfn, bool migrate_scanner) > { > struct zone *zone = cc->zone; > - unsigned long pfn; > > if (cc->ignore_skip_hint) > return; > @@ -277,8 +276,6 @@ static void update_pageblock_skip(struct compact_control *cc, > > set_pageblock_skip(page); > > - pfn = page_to_pfn(page); > - > /* Update where async and sync compaction should restart */ > if (migrate_scanner) { > if (pfn > zone->compact_cached_migrate_pfn[0]) > @@ -300,7 +297,7 @@ static inline bool isolation_suitable(struct compact_control *cc, > > static void update_pageblock_skip(struct compact_control *cc, > struct page *page, unsigned long nr_isolated, > - bool migrate_scanner) > + unsigned long pfn, bool migrate_scanner) > { > } > #endif /* CONFIG_COMPACTION */ > @@ -509,7 +506,8 @@ isolate_fail: > > /* Update the pageblock-skip if the whole pageblock was scanned */ > if (blockpfn == end_pfn) > - update_pageblock_skip(cc, valid_page, total_isolated, false); > + update_pageblock_skip(cc, valid_page, total_isolated, > + end_pfn, false);
In isolate_freepages_block() this means we actually go logically *back* one pageblock, as the direction is opposite? I know it's not an issue after the redesign patch so you wouldn't notice it when testing the whole series. But there's a non-zero chance that the smaller fixes are merged first and the redesign later...
> > count_compact_events(COMPACTFREE_SCANNED, nr_scanned); > if (total_isolated) > @@ -811,7 +809,8 @@ isolate_success: > * if the whole pageblock was scanned without isolating any page. > */ > if (low_pfn == end_pfn) > - update_pageblock_skip(cc, valid_page, nr_isolated, true); > + update_pageblock_skip(cc, valid_page, nr_isolated, > + end_pfn, true); > > trace_mm_compaction_isolate_migratepages(start_pfn, low_pfn, > nr_scanned, nr_isolated); >
| |