Re: [PATCH v3 1/7] mm/compaction: skip useless pfn when updating cached pfn

[Vlastimil Babka]

On 12/03/2015 08:11 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.
> 
> This patch moved update_pageblock_skip() to
> isolate_(freepages|migratepages). Updating pageblock skip information
> isn't relevant to CMA so they are more appropriate place
> to update this information.

That's step in a good direction, yeah. But why not go as far as some variant of
my (not resubmitted) patch "mm, compaction: decouple updating pageblock_skip and
cached pfn" [1]. Now the overloading of update_pageblock_skip() is just too much
- a struct page pointer for the skip bits, and a pfn of different page for the
cached pfn update, that's just more complex than it should be.

(I also suspect the pageblock_flags manipulation functions could be simpler if
they accepted zone pointer and pfn instead of struct page)

Also recently in Aaron's report we found a possible scenario where pageblocks
are being skipped without entering the isolate_*_block() functions, and it would
make sense to update the cached pfn's in that case, independently of updating
pageblock skip bits.

But this might be too out of scope of your series, so if you want I can
separately look at reviving some useful parts of [1] and the simpler
pageblock_flags manipulations.

[1] https://lkml.org/lkml/2015/6/10/237

> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
> ---

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/