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, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>