On Fri, Apr 06, 2018 at 03:07:11AM +0000, Horiguchi Naoya(堀口 直也) wrote:
...
> -----
> From e31ec037701d1cc76b26226e4b66d8c783d40889 Mon Sep 17 00:00:00 2001
> From: Naoya Horiguchi <n-horigu...@ah.jp.nec.com>
> Date: Fri, 6 Apr 2018 10:58:35 +0900
> Subject: [PATCH] mm: enable thp migration for shmem thp
> 
> My testing for the latest kernel supporting thp migration showed an
> infinite loop in offlining the memory block that is filled with shmem
> thps.  We can get out of the loop with a signal, but kernel should
> return with failure in this case.
> 
> What happens in the loop is that scan_movable_pages() repeats returning
> the same pfn without any progress. That's because page migration always
> fails for shmem thps.
> 
> In memory offline code, memory blocks containing unmovable pages should
> be prevented from being offline targets by has_unmovable_pages() inside
> start_isolate_page_range(). So it's possible to change migratability
> for non-anonymous thps to avoid the issue, but it introduces more complex
> and thp-specific handling in migration code, so it might not good.
> 
> So this patch is suggesting to fix the issue by enabling thp migration
> for shmem thp. Both of anon/shmem thp are migratable so we don't need
> precheck about the type of thps.
> 
> Fixes: commit 72b39cfc4d75 ("mm, memory_hotplug: do not fail offlining too 
> early")
> Signed-off-by: Naoya Horiguchi <n-horigu...@ah.jp.nec.com>
> Cc: sta...@vger.kernel.org # v4.15+

... oh, I don't think this is suitable for stable.
Michal's fix in another email can come first with "CC: stable",
then this one.
Anyway I want to get some feedback on the change of this patch.

Thanks,
Naoya Horiguchi

> ---
>  mm/huge_memory.c |  5 ++++-
>  mm/migrate.c     | 19 ++++++++++++++++---
>  mm/rmap.c        |  3 ---
>  3 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 2aff58624886..933c1bbd3464 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2926,7 +2926,10 @@ void remove_migration_pmd(struct page_vma_mapped_walk 
> *pvmw, struct page *new)
>               pmde = maybe_pmd_mkwrite(pmde, vma);
>  
>       flush_cache_range(vma, mmun_start, mmun_start + HPAGE_PMD_SIZE);
> -     page_add_anon_rmap(new, vma, mmun_start, true);
> +     if (PageAnon(new))
> +             page_add_anon_rmap(new, vma, mmun_start, true);
> +     else
> +             page_add_file_rmap(new, true);
>       set_pmd_at(mm, mmun_start, pvmw->pmd, pmde);
>       if (vma->vm_flags & VM_LOCKED)
>               mlock_vma_page(new);
> diff --git a/mm/migrate.c b/mm/migrate.c
> index bdef905b1737..f92dd9f50981 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -472,7 +472,7 @@ int migrate_page_move_mapping(struct address_space 
> *mapping,
>       pslot = radix_tree_lookup_slot(&mapping->i_pages,
>                                       page_index(page));
>  
> -     expected_count += 1 + page_has_private(page);
> +     expected_count += hpage_nr_pages(page) + page_has_private(page);
>       if (page_count(page) != expected_count ||
>               radix_tree_deref_slot_protected(pslot,
>                                       &mapping->i_pages.xa_lock) != page) {
> @@ -505,7 +505,7 @@ int migrate_page_move_mapping(struct address_space 
> *mapping,
>        */
>       newpage->index = page->index;
>       newpage->mapping = page->mapping;
> -     get_page(newpage);      /* add cache reference */
> +     page_ref_add(newpage, hpage_nr_pages(page)); /* add cache reference */
>       if (PageSwapBacked(page)) {
>               __SetPageSwapBacked(newpage);
>               if (PageSwapCache(page)) {
> @@ -524,13 +524,26 @@ int migrate_page_move_mapping(struct address_space 
> *mapping,
>       }
>  
>       radix_tree_replace_slot(&mapping->i_pages, pslot, newpage);
> +     if (PageTransHuge(page)) {
> +             int i;
> +             int index = page_index(page);
> +
> +             for (i = 0; i < HPAGE_PMD_NR; i++) {
> +                     pslot = radix_tree_lookup_slot(&mapping->i_pages,
> +                                                    index + i);
> +                     radix_tree_replace_slot(&mapping->i_pages, pslot,
> +                                             newpage + i);
> +             }
> +     } else {
> +             radix_tree_replace_slot(&mapping->i_pages, pslot, newpage);
> +     }
>  
>       /*
>        * Drop cache reference from old page by unfreezing
>        * to one less reference.
>        * We know this isn't the last reference.
>        */
> -     page_ref_unfreeze(page, expected_count - 1);
> +     page_ref_unfreeze(page, expected_count - hpage_nr_pages(page));
>  
>       xa_unlock(&mapping->i_pages);
>       /* Leave irq disabled to prevent preemption while updating stats */
> diff --git a/mm/rmap.c b/mm/rmap.c
> index f0dd4e4565bc..8d5337fed37b 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1374,9 +1374,6 @@ static bool try_to_unmap_one(struct page *page, struct 
> vm_area_struct *vma,
>               if (!pvmw.pte && (flags & TTU_MIGRATION)) {
>                       VM_BUG_ON_PAGE(PageHuge(page) || 
> !PageTransCompound(page), page);
>  
> -                     if (!PageAnon(page))
> -                             continue;
> -
>                       set_pmd_migration_entry(&pvmw, page);
>                       continue;
>               }
> -- 
> 2.7.4
> 

Reply via email to