On 06/16/2015 07:44 AM, Joonsoo Kim wrote:
> On Wed, Jun 10, 2015 at 11:32:32AM +0200, Vlastimil Babka wrote:
[...]
>> @@ -723,39 +725,35 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>> * It's possible to migrate LRU pages and balloon pages
>> * Skip any other type of page
>> */
>> - if (!PageLRU(page)) {
>> + is_lru = PageLRU(page);
>> + if (!is_lru) {
>> if (unlikely(balloon_page_movable(page))) {
>> if (balloon_page_isolate(page)) {
>> /* Successfully isolated */
>> goto isolate_success;
>> }
>> }
>> - continue;
>> }
>>
>> /*
>> - * PageLRU is set. lru_lock normally excludes isolation
>> - * splitting and collapsing (collapsing has already happened
>> - * if PageLRU is set) but the lock is not necessarily taken
>> - * here and it is wasteful to take it just to check transhuge.
>> - * Check PageCompound without lock and skip the whole pageblock
>> - * if it's a transhuge page, as calling compound_order()
>> - * without preventing THP from splitting the page underneath us
>> - * may return surprising results.
>> - * If we happen to check a THP tail page, compound_order()
>> - * returns 0. It should be rare enough to not bother with
>> - * using compound_head() in that case.
>> + * Regardless of being on LRU, compound pages such as THP and
>> + * hugetlbfs are not to be compacted. We can potentially save
>> + * a lot of iterations if we skip them at once. The check is
>> + * racy, but we can consider only valid values and the only
>> + * danger is skipping too much.
>> */
>> if (PageCompound(page)) {
>> - int nr;
>> - if (locked)
>> - nr = 1 << compound_order(page);
>> - else
>> - nr = pageblock_nr_pages;
>> - low_pfn += nr - 1;
>> + unsigned int comp_order = compound_order(page);
>> +
>> + if (comp_order > 0 && comp_order < MAX_ORDER)
>> + low_pfn += (1UL << comp_order) - 1;
>> +
>> continue;
>> }
>
> How about moving this PageCompound() check up to the PageLRU check?
> Is there any relationship between balloon page and PageCompound()?
I didn't want to assume if there's a relationship or not, as per the changelog:
>> After this patch, all pages are tested for PageCompound() and we skip them by
>> compound_order(). The test is done after the test for balloon_page_movable()
>> as we don't want to assume if balloon pages (or other pages with own isolation
>> and migration implementation if a generic API gets implemented) are compound
>> or not.
> It will remove is_lru and code would be more understandable.
Right, it just felt safer and more future-proof this way.
> Otherwise,
>
> Acked-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
>
> Thanks.
>
--
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>