mm: soft-offline: don't free target page in successful page migration

Stress testing showed that soft offline events for a process iterating
"mmap-pagefault-munmap" loop can trigger
VM_BUG_ON(PAGE_FLAGS_CHECK_AT_PREP) in __free_one_page():

  Soft offlining page 0x70fe1 at 0x70100008d000
  Soft offlining page 0x705fb at 0x70300008d000
  page:ffffea0001c3f840 count:0 mapcount:0 mapping:          (null) index:0x2
  flags: 0x1fffff80800000(hwpoison)
  page dumped because: VM_BUG_ON_PAGE(page->flags & ((1 << 25) - 1))
  ------------[ cut here ]------------
  kernel BUG at /src/linux-dev/mm/page_alloc.c:585!
  invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
  Modules linked in: cfg80211 rfkill crc32c_intel microcode ppdev parport_pc pcspkr serio_raw virtio_balloon parport i2c_piix4 virtio_blk virtio_net ata_generic pata_acpi floppy
  CPU: 3 PID: 1779 Comm: test_base_madv_ Not tainted 4.0.0-v4.0-150511-1451-00009-g82360a3730e6 #139
  RIP: free_pcppages_bulk+0x52a/0x6f0
  Call Trace:
    drain_pages_zone+0x3d/0x50
    drain_local_pages+0x1d/0x30
    on_each_cpu_mask+0x46/0x80
    drain_all_pages+0x14b/0x1e0
    soft_offline_page+0x432/0x6e0
    SyS_madvise+0x73c/0x780
    system_call_fastpath+0x12/0x17
  Code: ff 89 45 b4 48 8b 45 c0 48 83 b8 a8 00 00 00 00 0f 85 e3 fb ff ff 0f 1f 00 0f 0b 48 8b 7d 90 48 c7 c6 e8 95 a6 81 e8 e6 32 02 00 <0f> 0b 8b 45 cc 49 89 47 30 41 8b 47 18 83 f8 ff 0f 85 10 ff ff
  RIP  [<ffffffff811a806a>] free_pcppages_bulk+0x52a/0x6f0
   RSP <ffff88007a117d28>
  ---[ end trace 53926436e76d1f35 ]---

When soft offline successfully migrates page, the source page is supposed
to be freed.  But there is a race condition where a source page looks
isolated (i.e.  the refcount is 0 and the PageHWPoison is set) but
somewhat linked to pcplist.  Then another soft offline event calls
drain_all_pages() and tries to free such hwpoisoned page, which is
forbidden.

This odd page state seems to happen due to the race between put_page() in
putback_lru_page() and __pagevec_lru_add_fn().  But I don't want to play
with tweaking drain code as done in commit 9ab3b598d2df "mm: hwpoison:
drop lru_add_drain_all() in __soft_offline_page()", or to change page
freeing code for this soft offline's purpose.

Instead, let's think about the difference between hard offline and soft
offline.  There is an interesting difference in how to isolate the in-use
page between these, that is, hard offline marks PageHWPoison of the target
page at first, and doesn't free it by keeping its refcount 1.  OTOH, soft
offline tries to free the target page then marks PageHWPoison.  This
difference might be the source of complexity and result in bugs like the
above.  So making soft offline isolate with keeping refcount can be a
solution for this problem.

We can pass to page migration code the "reason" which shows the caller, so
let's use this more to avoid calling putback_lru_page() when called from
soft offline, which effectively does the isolation for soft offline.  With
this change, target pages of soft offline never be reused without changing
migratetype, so this patch also removes the related code.

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index a810ab1519f0e147c741a076672addd1eb98f33a..62ebb1b7f4bfed457b54ca788e9fec38107c0925 100644 (file)
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1697,20 +1697,7 @@ static int __soft_offline_page(struct page *page, int flags)
                        if (ret > 0)
                                ret = -EIO;
                } else {
-                       /*
-                        * After page migration succeeds, the source page can
-                        * be trapped in pagevec and actual freeing is delayed.
-                        * Freeing code works differently based on PG_hwpoison,
-                        * so there's a race. We need to make sure that the
-                        * source page should be freed back to buddy before
-                        * setting PG_hwpoison.
-                        */
-                       if (!is_free_buddy_page(page))
-                               drain_all_pages(page_zone(page));
                        SetPageHWPoison(page);
-                       if (!is_free_buddy_page(page))
-                               pr_info("soft offline: %#lx: page leaked\n",
-                                       pfn);
                        atomic_long_inc(&num_poisoned_pages);
                }
        } else {
@@ -1762,14 +1749,6 @@ int soft_offline_page(struct page *page, int flags)
 
        get_online_mems();
 
-       /*
-        * Isolate the page, so that it doesn't get reallocated if it
-        * was free. This flag should be kept set until the source page
-        * is freed and PG_hwpoison on it is set.
-        */
-       if (get_pageblock_migratetype(page) != MIGRATE_ISOLATE)
-               set_migratetype_isolate(page, true);
-
        ret = get_any_page(page, pfn, flags);
        put_online_mems();
        if (ret > 0) { /* for in-use pages */
@@ -1788,6 +1767,5 @@ int soft_offline_page(struct page *page, int flags)
                                atomic_long_inc(&num_poisoned_pages);
                }
        }
-       unset_migratetype_isolate(page, MIGRATE_MOVABLE);
        return ret;
 }

diff --git a/mm/migrate.c b/mm/migrate.c
index f53838fe3dfe6e84868b86eb98fedf7abd384ffb..d4fe1f94120b92e23124651d3d6345fbb65a6289 100644 (file)
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -918,7 +918,8 @@ out:
 static ICE_noinline int unmap_and_move(new_page_t get_new_page,
                                   free_page_t put_new_page,
                                   unsigned long private, struct page *page,
-                                  int force, enum migrate_mode mode)
+                                  int force, enum migrate_mode mode,
+                                  enum migrate_reason reason)
 {
        int rc = 0;
        int *result = NULL;
@@ -949,7 +950,8 @@ out:
                list_del(&page->lru);
                dec_zone_page_state(page, NR_ISOLATED_ANON +
                                page_is_file_cache(page));
-               putback_lru_page(page);
+               if (reason != MR_MEMORY_FAILURE)
+                       putback_lru_page(page);
        }
 
        /*
@@ -1122,7 +1124,8 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page,
                                                pass > 2, mode);
                        else
                                rc = unmap_and_move(get_new_page, put_new_page,
-                                               private, page, pass > 2, mode);
+                                               private, page, pass > 2, mode,
+                                               reason);
 
                        switch(rc) {
                        case -ENOMEM: