On 06/03/2016 02:47 PM, Joonsoo Kim wrote:
[...]

@@ -128,8 +127,12 @@ static void unset_migratetype_isolate(struct page
*page, unsigned migratetype)
        zone->nr_isolate_pageblock--;
 out:
        spin_unlock_irqrestore(&zone->lock, flags);
-       if (isolated_page)
+       if (isolated_page) {
+               kernel_map_pages(page, (1 << order), 1);


So why we don't need the other stuff done by e.g. map_pages()? For example
arch_alloc_page() and kasan_alloc_pages(). Maybe kasan_free_pages() (called
below via __free_pages() I assume) now doesn't check if the allocation part
was done. But maybe it will start doing that?

See how the multiple places doing similar stuff is fragile? :(

I answered it in reply of comment of patch 1.

Right.

Acked-by: Vlastimil Babka <vba...@suse.cz>


+               set_page_refcounted(page);
+               set_page_owner(page, order, __GFP_MOVABLE);
                __free_pages(isolated_page, order);


This mixing of "isolated_page" and "page" is not a bug, but quite ugly.
Can't isolated_page variable just be a bool?


Looks better. I will do it.

Thanks.


Reply via email to