Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1516672 > unrolled thread

[PATCH v2 00/12] mm: page migration enhancement for thp

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/12] mm: page migration enhancement for thp Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2016-11-08 00:40 +0100
    [PATCH v2 02/12] mm: mempolicy: add queue_pages_node_check() Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2016-11-08 00:50 +0100
    [PATCH v2 07/12] mm: thp: check pmd migration entry in common path Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2016-11-08 01:00 +0100
      Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common  path kbuild test robot <lkp@intel.com> - 2016-11-08 01:30 +0100
        Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common  path Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2016-11-08 02:30 +0100
      Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common  path Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-10 09:40 +0100
        Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common  path Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2016-11-10 10:20 +0100
      Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common  path Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2016-11-10 10:30 +0100
        Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common  path Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2016-11-10 10:40 +0100
        Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-11-10 10:40 +0100
      Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-11-10 10:30 +0100
    [PATCH v2 06/12] mm: thp: enable thp migration in generic path Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2016-11-08 01:40 +0100
    Re: [PATCH v2 00/12] mm: page migration enhancement for thp Balbir Singh <bsingharora@gmail.com> - 2016-11-09 03:40 +0100
      Re: [PATCH v2 00/12] mm: page migration enhancement for thp Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2016-11-09 06:10 +0100
        Re: [PATCH v2 00/12] mm: page migration enhancement for thp Balbir Singh <bsingharora@gmail.com> - 2016-11-09 22:30 +0100
    Re: [PATCH v2 00/12] mm: page migration enhancement for thp Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-09 11:40 +0100
      Re: [PATCH v2 00/12] mm: page migration enhancement for thp Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2016-11-10 01:00 +0100
        Re: [PATCH v2 00/12] mm: page migration enhancement for thp Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-11 04:50 +0100

#1516672 — [PATCH v2 00/12] mm: page migration enhancement for thp

Hi everyone,

I've updated thp migration patches for v4.9-rc2-mmotm-2016-10-27-18-27
with feedbacks for ver.1.

General description (no change since ver.1)
===========================================

This patchset enhances page migration functionality to handle thp migration
for various page migration's callers:
 - mbind(2)
 - move_pages(2)
 - migrate_pages(2)
 - cgroup/cpuset migration
 - memory hotremove
 - soft offline

The main benefit is that we can avoid unnecessary thp splits, which helps us
avoid performance decrease when your applications handles NUMA optimization on
their own.

The implementation is similar to that of normal page migration, the key point
is that we modify a pmd to a pmd migration entry in swap-entry like format.

Changes / Notes
===============

- pmd_present() in x86 checks _PAGE_PRESENT, _PAGE_PROTNONE and _PAGE_PSE
  bits together, which makes implementing thp migration a bit hard because
  _PAGE_PSE bit is currently used by soft-dirty in swap-entry format.
  I was advised to dropping _PAGE_PSE in pmd_present(), but I don't think
  of the justification, so I keep it in this version. Instead, my approach
  is to move _PAGE_SWP_SOFT_DIRTY to bit 6 (unused) and reserve bit 7 for
  pmd non-present cases.

- this patchset still covers only x86_64. Zi Yan posted a patch for ppc64
  and I think it's favorably received so that's fine. But there's unsolved
  minor suggestion by Aneesh, so I don't include it in this set, expecting
  that it will be updated/reposted.

- pte-mapped thp and doubly-mapped thp were not supported in ver.1, but
  this version should work for such kinds of thp.

- thp page cache is not tested yet, and it's at the head of my todo list
  for future version.

Any comments or advices are welcomed.

Thanks,
Naoya Horiguchi

[toc] | [next] | [standalone]

#1516680 — [PATCH v2 02/12] mm: mempolicy: add queue_pages_node_check()

Introduce a separate check routine related to MPOL_MF_INVERT flag. This patch
just does cleanup, no behavioral change.

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 mm/mempolicy.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git v4.9-rc2-mmotm-2016-10-27-18-27/mm/mempolicy.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/mempolicy.c
index 6d3639e..77d0668 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/mm/mempolicy.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/mempolicy.c
@@ -477,6 +477,15 @@ struct queue_pages {
 	struct vm_area_struct *prev;
 };
 
+static inline bool queue_pages_node_check(struct page *page,
+					struct queue_pages *qp)
+{
+	int nid = page_to_nid(page);
+	unsigned long flags = qp->flags;
+
+	return node_isset(nid, *qp->nmask) == !!(flags & MPOL_MF_INVERT);
+}
+
 /*
  * Scan through pages checking if pages follow certain conditions,
  * and move them to the pagelist if they do.
@@ -530,8 +539,7 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned long addr,
 		 */
 		if (PageReserved(page))
 			continue;
-		nid = page_to_nid(page);
-		if (node_isset(nid, *qp->nmask) == !!(flags & MPOL_MF_INVERT))
+		if (queue_pages_node_check(page, qp))
 			continue;
 		if (PageTransCompound(page)) {
 			get_page(page);
@@ -563,7 +571,6 @@ static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask,
 #ifdef CONFIG_HUGETLB_PAGE
 	struct queue_pages *qp = walk->private;
 	unsigned long flags = qp->flags;
-	int nid;
 	struct page *page;
 	spinlock_t *ptl;
 	pte_t entry;
@@ -573,8 +580,7 @@ static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask,
 	if (!pte_present(entry))
 		goto unlock;
 	page = pte_page(entry);
-	nid = page_to_nid(page);
-	if (node_isset(nid, *qp->nmask) == !!(flags & MPOL_MF_INVERT))
+	if (queue_pages_node_check(page, qp))
 		goto unlock;
 	/* With MPOL_MF_MOVE, we migrate only unshared hugepage. */
 	if (flags & (MPOL_MF_MOVE_ALL) ||
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1516691 — [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

If one of callers of page migration starts to handle thp, memory management code
start to see pmd migration entry, so we need to prepare for it before enabling.
This patch changes various code point which checks the status of given pmds in
order to prevent race between thp migration and the pmd-related works.

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
ChangeLog v1 -> v2:
- introduce pmd_related() (I know the naming is not good, but can't think up
  no better name. Any suggesntion is welcomed.)
---
 arch/x86/mm/gup.c       |  4 +--
 fs/proc/task_mmu.c      | 23 +++++++------
 include/linux/huge_mm.h |  9 ++++-
 mm/gup.c                | 10 ++++--
 mm/huge_memory.c        | 88 ++++++++++++++++++++++++++++++++++++++++---------
 mm/madvise.c            |  2 +-
 mm/memcontrol.c         |  2 ++
 mm/memory.c             |  6 +++-
 mm/mprotect.c           |  2 ++
 mm/mremap.c             |  2 +-
 10 files changed, 114 insertions(+), 34 deletions(-)

diff --git v4.9-rc2-mmotm-2016-10-27-18-27/arch/x86/mm/gup.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/arch/x86/mm/gup.c
index 0d4fb3e..78a153d 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/arch/x86/mm/gup.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/arch/x86/mm/gup.c
@@ -222,9 +222,9 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
 		pmd_t pmd = *pmdp;
 
 		next = pmd_addr_end(addr, end);
-		if (pmd_none(pmd))
+		if (!pmd_present(pmd))
 			return 0;
-		if (unlikely(pmd_large(pmd) || !pmd_present(pmd))) {
+		if (unlikely(pmd_large(pmd))) {
 			/*
 			 * NUMA hinting faults need to be handled in the GUP
 			 * slowpath for accounting purposes and so that they
diff --git v4.9-rc2-mmotm-2016-10-27-18-27/fs/proc/task_mmu.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/fs/proc/task_mmu.c
index 35b92d8..c1f9cf4 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/fs/proc/task_mmu.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/fs/proc/task_mmu.c
@@ -596,7 +596,8 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 
 	ptl = pmd_trans_huge_lock(pmd, vma);
 	if (ptl) {
-		smaps_pmd_entry(pmd, addr, walk);
+		if (pmd_present(*pmd))
+			smaps_pmd_entry(pmd, addr, walk);
 		spin_unlock(ptl);
 		return 0;
 	}
@@ -929,6 +930,9 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned long addr,
 			goto out;
 		}
 
+		if (!pmd_present(*pmd))
+			goto out;
+
 		page = pmd_page(*pmd);
 
 		/* Clear accessed and referenced bits. */
@@ -1208,19 +1212,18 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned long addr, unsigned long end,
 	if (ptl) {
 		u64 flags = 0, frame = 0;
 		pmd_t pmd = *pmdp;
+		struct page *page;
 
 		if ((vma->vm_flags & VM_SOFTDIRTY) || pmd_soft_dirty(pmd))
 			flags |= PM_SOFT_DIRTY;
 
-		/*
-		 * Currently pmd for thp is always present because thp
-		 * can not be swapped-out, migrated, or HWPOISONed
-		 * (split in such cases instead.)
-		 * This if-check is just to prepare for future implementation.
-		 */
-		if (pmd_present(pmd)) {
-			struct page *page = pmd_page(pmd);
-
+		if (is_pmd_migration_entry(pmd)) {
+			swp_entry_t entry = pmd_to_swp_entry(pmd);
+			frame = swp_type(entry) |
+				(swp_offset(entry) << MAX_SWAPFILES_SHIFT);
+			page = migration_entry_to_page(entry);
+		} else if (pmd_present(pmd)) {
+			page = pmd_page(pmd);
 			if (page_mapcount(page) == 1)
 				flags |= PM_MMAP_EXCLUSIVE;
 
diff --git v4.9-rc2-mmotm-2016-10-27-18-27/include/linux/huge_mm.h v4.9-rc2-mmotm-2016-10-27-18-27_patched/include/linux/huge_mm.h
index fcbca51..3c252cd 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/include/linux/huge_mm.h
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/include/linux/huge_mm.h
@@ -125,12 +125,19 @@ extern void vma_adjust_trans_huge(struct vm_area_struct *vma,
 				    long adjust_next);
 extern spinlock_t *__pmd_trans_huge_lock(pmd_t *pmd,
 		struct vm_area_struct *vma);
+
+static inline int pmd_related(pmd_t pmd)
+{
+	return !pmd_none(pmd) &&
+		(!pmd_present(pmd) || pmd_trans_huge(pmd) || pmd_devmap(pmd));
+}
+
 /* mmap_sem must be held on entry */
 static inline spinlock_t *pmd_trans_huge_lock(pmd_t *pmd,
 		struct vm_area_struct *vma)
 {
 	VM_BUG_ON_VMA(!rwsem_is_locked(&vma->vm_mm->mmap_sem), vma);
-	if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd))
+	if (pmd_related(*pmd))
 		return __pmd_trans_huge_lock(pmd, vma);
 	else
 		return NULL;
diff --git v4.9-rc2-mmotm-2016-10-27-18-27/mm/gup.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/gup.c
index e50178c..2dc4978 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/mm/gup.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/gup.c
@@ -267,6 +267,8 @@ struct page *follow_page_mask(struct vm_area_struct *vma,
 	}
 	if ((flags & FOLL_NUMA) && pmd_protnone(*pmd))
 		return no_page_table(vma, flags);
+	if (!pmd_present(*pmd))
+		return no_page_table(vma, flags);
 	if (pmd_devmap(*pmd)) {
 		ptl = pmd_lock(mm, pmd);
 		page = follow_devmap_pmd(vma, address, pmd, flags);
@@ -278,6 +280,10 @@ struct page *follow_page_mask(struct vm_area_struct *vma,
 		return follow_page_pte(vma, address, pmd, flags);
 
 	ptl = pmd_lock(mm, pmd);
+	if (unlikely(!pmd_present(*pmd))) {
+		spin_unlock(ptl);
+		return no_page_table(vma, flags);
+	}
 	if (unlikely(!pmd_trans_huge(*pmd))) {
 		spin_unlock(ptl);
 		return follow_page_pte(vma, address, pmd, flags);
@@ -333,7 +339,7 @@ static int get_gate_page(struct mm_struct *mm, unsigned long address,
 	pud = pud_offset(pgd, address);
 	BUG_ON(pud_none(*pud));
 	pmd = pmd_offset(pud, address);
-	if (pmd_none(*pmd))
+	if (!pmd_present(*pmd))
 		return -EFAULT;
 	VM_BUG_ON(pmd_trans_huge(*pmd));
 	pte = pte_offset_map(pmd, address);
@@ -1357,7 +1363,7 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
 		pmd_t pmd = READ_ONCE(*pmdp);
 
 		next = pmd_addr_end(addr, end);
-		if (pmd_none(pmd))
+		if (!pmd_present(pmd))
 			return 0;
 
 		if (unlikely(pmd_trans_huge(pmd) || pmd_huge(pmd))) {
diff --git v4.9-rc2-mmotm-2016-10-27-18-27/mm/huge_memory.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/huge_memory.c
index b3022b3..4e9090c 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/mm/huge_memory.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/huge_memory.c
@@ -825,6 +825,20 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 
 	ret = -EAGAIN;
 	pmd = *src_pmd;
+
+	if (unlikely(is_pmd_migration_entry(pmd))) {
+		swp_entry_t entry = pmd_to_swp_entry(pmd);
+
+		if (is_write_migration_entry(entry)) {
+			make_migration_entry_read(&entry);
+			pmd = swp_entry_to_pmd(entry);
+			set_pmd_at(src_mm, addr, src_pmd, pmd);
+		}
+		set_pmd_at(dst_mm, addr, dst_pmd, pmd);
+		ret = 0;
+		goto out_unlock;
+	}
+
 	if (unlikely(!pmd_trans_huge(pmd))) {
 		pte_free(dst_mm, pgtable);
 		goto out_unlock;
@@ -1013,6 +1027,9 @@ int do_huge_pmd_wp_page(struct fault_env *fe, pmd_t orig_pmd)
 	if (unlikely(!pmd_same(*fe->pmd, orig_pmd)))
 		goto out_unlock;
 
+	if (unlikely(!pmd_present(orig_pmd)))
+		goto out_unlock;
+
 	page = pmd_page(orig_pmd);
 	VM_BUG_ON_PAGE(!PageCompound(page) || !PageHead(page), page);
 	/*
@@ -1137,7 +1154,14 @@ struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
 	if ((flags & FOLL_NUMA) && pmd_protnone(*pmd))
 		goto out;
 
-	page = pmd_page(*pmd);
+	if (is_pmd_migration_entry(*pmd)) {
+		swp_entry_t entry;
+		entry = pmd_to_swp_entry(*pmd);
+		page = pfn_to_page(swp_offset(entry));
+		if (!is_migration_entry(entry))
+			goto out;
+	} else
+		page = pmd_page(*pmd);
 	VM_BUG_ON_PAGE(!PageHead(page) && !is_zone_device_page(page), page);
 	if (flags & FOLL_TOUCH)
 		touch_pmd(vma, addr, pmd);
@@ -1332,6 +1356,9 @@ bool madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 	if (is_huge_zero_pmd(orig_pmd))
 		goto out;
 
+	if (unlikely(!pmd_present(orig_pmd)))
+		goto out;
+
 	page = pmd_page(orig_pmd);
 	/*
 	 * If other processes are mapping this page, we couldn't discard
@@ -1410,20 +1437,35 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 		tlb_remove_page_size(tlb, pmd_page(orig_pmd), HPAGE_PMD_SIZE);
 	} else {
 		struct page *page = pmd_page(orig_pmd);
-		page_remove_rmap(page, true);
-		VM_BUG_ON_PAGE(page_mapcount(page) < 0, page);
-		VM_BUG_ON_PAGE(!PageHead(page), page);
-		if (PageAnon(page)) {
-			pgtable_t pgtable;
-			pgtable = pgtable_trans_huge_withdraw(tlb->mm, pmd);
-			pte_free(tlb->mm, pgtable);
-			atomic_long_dec(&tlb->mm->nr_ptes);
-			add_mm_counter(tlb->mm, MM_ANONPAGES, -HPAGE_PMD_NR);
+		int migration = 0;
+
+		if (!is_pmd_migration_entry(orig_pmd)) {
+			page_remove_rmap(page, true);
+			VM_BUG_ON_PAGE(page_mapcount(page) < 0, page);
+			VM_BUG_ON_PAGE(!PageHead(page), page);
+			if (PageAnon(page)) {
+				pgtable_t pgtable;
+				pgtable = pgtable_trans_huge_withdraw(tlb->mm,
+								      pmd);
+				pte_free(tlb->mm, pgtable);
+				atomic_long_dec(&tlb->mm->nr_ptes);
+				add_mm_counter(tlb->mm, MM_ANONPAGES,
+					       -HPAGE_PMD_NR);
+			} else {
+				add_mm_counter(tlb->mm, MM_FILEPAGES,
+					       -HPAGE_PMD_NR);
+			}
 		} else {
-			add_mm_counter(tlb->mm, MM_FILEPAGES, -HPAGE_PMD_NR);
+			swp_entry_t entry;
+
+			entry = pmd_to_swp_entry(orig_pmd);
+			free_swap_and_cache(entry); /* waring in failure? */
+			add_mm_counter(tlb->mm, MM_ANONPAGES, -HPAGE_PMD_NR);
+			migration = 1;
 		}
 		spin_unlock(ptl);
-		tlb_remove_page_size(tlb, page, HPAGE_PMD_SIZE);
+		if (!migration)
+			tlb_remove_page_size(tlb, page, HPAGE_PMD_SIZE);
 	}
 	return 1;
 }
@@ -1496,14 +1538,27 @@ int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
 		bool preserve_write = prot_numa && pmd_write(*pmd);
 		ret = 1;
 
+		if (!pmd_present(*pmd))
+			goto unlock;
 		/*
 		 * Avoid trapping faults against the zero page. The read-only
 		 * data is likely to be read-cached on the local CPU and
 		 * local/remote hits to the zero page are not interesting.
 		 */
-		if (prot_numa && is_huge_zero_pmd(*pmd)) {
-			spin_unlock(ptl);
-			return ret;
+		if (prot_numa && is_huge_zero_pmd(*pmd))
+			goto unlock;
+
+		if (is_pmd_migration_entry(*pmd)) {
+			swp_entry_t entry = pmd_to_swp_entry(*pmd);
+
+			if (is_write_migration_entry(entry)) {
+				pmd_t newpmd;
+
+				make_migration_entry_read(&entry);
+				newpmd = swp_entry_to_pmd(entry);
+				set_pmd_at(mm, addr, pmd, newpmd);
+			}
+			goto unlock;
 		}
 
 		if (!prot_numa || !pmd_protnone(*pmd)) {
@@ -1516,6 +1571,7 @@ int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
 			BUG_ON(vma_is_anonymous(vma) && !preserve_write &&
 					pmd_write(entry));
 		}
+unlock:
 		spin_unlock(ptl);
 	}
 
@@ -1532,7 +1588,7 @@ spinlock_t *__pmd_trans_huge_lock(pmd_t *pmd, struct vm_area_struct *vma)
 {
 	spinlock_t *ptl;
 	ptl = pmd_lock(vma->vm_mm, pmd);
-	if (likely(pmd_trans_huge(*pmd) || pmd_devmap(*pmd)))
+	if (likely(pmd_related(*pmd)))
 		return ptl;
 	spin_unlock(ptl);
 	return NULL;
diff --git v4.9-rc2-mmotm-2016-10-27-18-27/mm/madvise.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/madvise.c
index 0e3828e..eaa2b02 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/mm/madvise.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/madvise.c
@@ -274,7 +274,7 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
 	unsigned long next;
 
 	next = pmd_addr_end(addr, end);
-	if (pmd_trans_huge(*pmd))
+	if (pmd_related(*pmd))
 		if (madvise_free_huge_pmd(tlb, vma, pmd, addr, next))
 			goto next;
 
diff --git v4.9-rc2-mmotm-2016-10-27-18-27/mm/memcontrol.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/memcontrol.c
index 91dfc7c..ebc2c42 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/mm/memcontrol.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/memcontrol.c
@@ -4635,6 +4635,8 @@ static enum mc_target_type get_mctgt_type_thp(struct vm_area_struct *vma,
 	struct page *page = NULL;
 	enum mc_target_type ret = MC_TARGET_NONE;
 
+	if (unlikely(!pmd_present(pmd)))
+		return ret;
 	page = pmd_page(pmd);
 	VM_BUG_ON_PAGE(!page || !PageHead(page), page);
 	if (!(mc.flags & MOVE_ANON))
diff --git v4.9-rc2-mmotm-2016-10-27-18-27/mm/memory.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/memory.c
index 94b5e2c..33fa439 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/mm/memory.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/memory.c
@@ -999,7 +999,7 @@ static inline int copy_pmd_range(struct mm_struct *dst_mm, struct mm_struct *src
 	src_pmd = pmd_offset(src_pud, addr);
 	do {
 		next = pmd_addr_end(addr, end);
-		if (pmd_trans_huge(*src_pmd) || pmd_devmap(*src_pmd)) {
+		if (pmd_related(*src_pmd)) {
 			int err;
 			VM_BUG_ON(next-addr != HPAGE_PMD_SIZE);
 			err = copy_huge_pmd(dst_mm, src_mm,
@@ -3591,6 +3591,10 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
 		int ret;
 
 		barrier();
+		if (unlikely(is_pmd_migration_entry(orig_pmd))) {
+			pmd_migration_entry_wait(mm, fe.pmd);
+			return 0;
+		}
 		if (pmd_trans_huge(orig_pmd) || pmd_devmap(orig_pmd)) {
 			if (pmd_protnone(orig_pmd) && vma_is_accessible(vma))
 				return do_huge_pmd_numa_page(&fe, orig_pmd);
diff --git v4.9-rc2-mmotm-2016-10-27-18-27/mm/mprotect.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/mprotect.c
index c5ba2aa..81186e3 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/mm/mprotect.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/mprotect.c
@@ -164,6 +164,8 @@ static inline unsigned long change_pmd_range(struct vm_area_struct *vma,
 		unsigned long this_pages;
 
 		next = pmd_addr_end(addr, end);
+		if (!pmd_present(*pmd))
+			continue;
 		if (!pmd_trans_huge(*pmd) && !pmd_devmap(*pmd)
 				&& pmd_none_or_clear_bad(pmd))
 			continue;
diff --git v4.9-rc2-mmotm-2016-10-27-18-27/mm/mremap.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/mremap.c
index da22ad2..a94a698 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/mm/mremap.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/mremap.c
@@ -194,7 +194,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 		new_pmd = alloc_new_pmd(vma->vm_mm, vma, new_addr);
 		if (!new_pmd)
 			break;
-		if (pmd_trans_huge(*old_pmd)) {
+		if (pmd_related(*old_pmd)) {
 			if (extent == HPAGE_PMD_SIZE) {
 				bool moved;
 				/* See comment in move_ptes() */
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1516702 — Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

[Multipart message — attachments visible in raw view] — view raw

Hi Naoya,

[auto build test ERROR on mmotm/master]
[also build test ERROR on v4.9-rc4 next-20161028]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Naoya-Horiguchi/mm-x86-move-_PAGE_SWP_SOFT_DIRTY-from-bit-7-to-bit-6/20161108-080615
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: i386-randconfig-x007-201645 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   mm/memory.c: In function 'copy_pmd_range':
>> mm/memory.c:1002:7: error: implicit declaration of function 'pmd_related' [-Werror=implicit-function-declaration]
      if (pmd_related(*src_pmd)) {
          ^~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   mm/mremap.c: In function 'move_page_tables':
>> mm/mremap.c:197:7: error: implicit declaration of function 'pmd_related' [-Werror=implicit-function-declaration]
      if (pmd_related(*old_pmd)) {
          ^~~~~~~~~~~
   In file included from include/asm-generic/bug.h:4:0,
                    from arch/x86/include/asm/bug.h:35,
                    from include/linux/bug.h:4,
                    from include/linux/mmdebug.h:4,
                    from include/linux/mm.h:8,
                    from mm/mremap.c:10:
>> include/linux/compiler.h:518:38: error: call to '__compiletime_assert_198' declared with attribute error: BUILD_BUG failed
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
                                         ^
   include/linux/compiler.h:501:4: note: in definition of macro '__compiletime_assert'
       prefix ## suffix();    \
       ^~~~~~
   include/linux/compiler.h:518:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/bug.h:54:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^~~~~~~~~~~~~~~~~~
   include/linux/bug.h:88:21: note: in expansion of macro 'BUILD_BUG_ON_MSG'
    #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
                        ^~~~~~~~~~~~~~~~
>> include/linux/huge_mm.h:183:27: note: in expansion of macro 'BUILD_BUG'
    #define HPAGE_PMD_SIZE ({ BUILD_BUG(); 0; })
                              ^~~~~~~~~
>> mm/mremap.c:198:18: note: in expansion of macro 'HPAGE_PMD_SIZE'
       if (extent == HPAGE_PMD_SIZE) {
                     ^~~~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   mm/madvise.c: In function 'madvise_free_pte_range':
>> mm/madvise.c:277:6: error: implicit declaration of function 'pmd_related' [-Werror=implicit-function-declaration]
     if (pmd_related(*pmd))
         ^~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/pmd_related +1002 mm/memory.c

   996		dst_pmd = pmd_alloc(dst_mm, dst_pud, addr);
   997		if (!dst_pmd)
   998			return -ENOMEM;
   999		src_pmd = pmd_offset(src_pud, addr);
  1000		do {
  1001			next = pmd_addr_end(addr, end);
> 1002			if (pmd_related(*src_pmd)) {
  1003				int err;
  1004				VM_BUG_ON(next-addr != HPAGE_PMD_SIZE);
  1005				err = copy_huge_pmd(dst_mm, src_mm,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1516714 — Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

On Tue, Nov 08, 2016 at 08:23:50AM +0800, kbuild test robot wrote:
> Hi Naoya,
> 
> [auto build test ERROR on mmotm/master]
> [also build test ERROR on v4.9-rc4 next-20161028]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Naoya-Horiguchi/mm-x86-move-_PAGE_SWP_SOFT_DIRTY-from-bit-7-to-bit-6/20161108-080615
> base:   git://git.cmpxchg.org/linux-mmotm.git master
> config: i386-randconfig-x007-201645 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All error/warnings (new ones prefixed by >>):
> 
>    mm/memory.c: In function 'copy_pmd_range':
> >> mm/memory.c:1002:7: error: implicit declaration of function 'pmd_related' [-Werror=implicit-function-declaration]
>       if (pmd_related(*src_pmd)) {
>           ^~~~~~~~~~~
>    cc1: some warnings being treated as errors
> --

I forgot to declare a noop routine for CONFIG_TRANSPARENT_HUGEPAGE=n.

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -222,6 +229,10 @@ static inline void vma_adjust_trans_huge(struct vm_area_struct *vma,
 					 long adjust_next)
 {
 }
+static inline int pmd_related(pmd_t pmd)
+{
+	return 0;
+}
 static inline spinlock_t *pmd_trans_huge_lock(pmd_t *pmd,
 		struct vm_area_struct *vma)
 {


>    mm/mremap.c: In function 'move_page_tables':
> >> mm/mremap.c:197:7: error: implicit declaration of function 'pmd_related' [-Werror=implicit-function-declaration]
>       if (pmd_related(*old_pmd)) {
>           ^~~~~~~~~~~
>    In file included from include/asm-generic/bug.h:4:0,
>                     from arch/x86/include/asm/bug.h:35,
>                     from include/linux/bug.h:4,
>                     from include/linux/mmdebug.h:4,
>                     from include/linux/mm.h:8,
>                     from mm/mremap.c:10:
> >> include/linux/compiler.h:518:38: error: call to '__compiletime_assert_198' declared with attribute error: BUILD_BUG failed
>      _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>                                          ^
>    include/linux/compiler.h:501:4: note: in definition of macro '__compiletime_assert'
>        prefix ## suffix();    \
>        ^~~~~~
>    include/linux/compiler.h:518:2: note: in expansion of macro '_compiletime_assert'
>      _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>      ^~~~~~~~~~~~~~~~~~~
>    include/linux/bug.h:54:37: note: in expansion of macro 'compiletime_assert'
>     #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>                                         ^~~~~~~~~~~~~~~~~~
>    include/linux/bug.h:88:21: note: in expansion of macro 'BUILD_BUG_ON_MSG'
>     #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
>                         ^~~~~~~~~~~~~~~~
> >> include/linux/huge_mm.h:183:27: note: in expansion of macro 'BUILD_BUG'
>     #define HPAGE_PMD_SIZE ({ BUILD_BUG(); 0; })
>                               ^~~~~~~~~
> >> mm/mremap.c:198:18: note: in expansion of macro 'HPAGE_PMD_SIZE'
>        if (extent == HPAGE_PMD_SIZE) {
>                      ^~~~~~~~~~~~~~
>    cc1: some warnings being treated as errors

HPAGE_PMD_SIZE is available only in CONFIG_TRANSPARENT_HUGEPAGE=y, and
this code looks to violate the rule, but it is in if (pmd_related()) block
which is compiled out in CONFIG_TRANSPARENT_HUGEPAGE=n, so this is OK
only with the above change.

Thanks,
Naoya Horiguchi

> --
>    mm/madvise.c: In function 'madvise_free_pte_range':
> >> mm/madvise.c:277:6: error: implicit declaration of function 'pmd_related' [-Werror=implicit-function-declaration]
>      if (pmd_related(*pmd))
>          ^~~~~~~~~~~
>    cc1: some warnings being treated as errors
> 
> vim +/pmd_related +1002 mm/memory.c
> 
>    996		dst_pmd = pmd_alloc(dst_mm, dst_pud, addr);
>    997		if (!dst_pmd)
>    998			return -ENOMEM;
>    999		src_pmd = pmd_offset(src_pud, addr);
>   1000		do {
>   1001			next = pmd_addr_end(addr, end);
> > 1002			if (pmd_related(*src_pmd)) {
>   1003				int err;
>   1004				VM_BUG_ON(next-addr != HPAGE_PMD_SIZE);
>   1005				err = copy_huge_pmd(dst_mm, src_mm,
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1518738 — Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

On 11/08/2016 05:01 AM, Naoya Horiguchi wrote:
> If one of callers of page migration starts to handle thp, memory management code
> start to see pmd migration entry, so we need to prepare for it before enabling.
> This patch changes various code point which checks the status of given pmds in
> order to prevent race between thp migration and the pmd-related works.

There are lot of changes in this one patch. Should not we split
this up into multiple patches and explain them in a bit detail
through their commit messages ?

[toc] | [prev] | [next] | [standalone]

#1518773 — Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

On Thu, Nov 10, 2016 at 02:06:14PM +0530, Anshuman Khandual wrote:
> On 11/08/2016 05:01 AM, Naoya Horiguchi wrote:
> > If one of callers of page migration starts to handle thp, memory management code
> > start to see pmd migration entry, so we need to prepare for it before enabling.
> > This patch changes various code point which checks the status of given pmds in
> > order to prevent race between thp migration and the pmd-related works.
> 
> There are lot of changes in this one patch. Should not we split
> this up into multiple patches and explain them in a bit detail
> through their commit messages ?

Yes, and I admit that I might change more than necessary, if the context
never encounters migration entry for any reason, no change is needed.
I'll dig more detail.

- Naoya Horiguchi

[toc] | [prev] | [next] | [standalone]

#1518782 — Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

Hi Hillf,

On Thu, Nov 10, 2016 at 05:08:07PM +0800, Hillf Danton wrote:
> On Tuesday, November 08, 2016 7:32 AM Naoya Horiguchi wrote:
> > 
> > @@ -1013,6 +1027,9 @@ int do_huge_pmd_wp_page(struct fault_env *fe, pmd_t orig_pmd)
> >  	if (unlikely(!pmd_same(*fe->pmd, orig_pmd)))
> >  		goto out_unlock;
> > 
> > +	if (unlikely(!pmd_present(orig_pmd)))
> > +		goto out_unlock;
> > +
> 
> Can we encounter a migration entry after acquiring ptl ?

I think we can. thp migration code releases ptl after converting pmd into
migration entry, so other code can see it even within ptl.

Thanks,
Naoya Horiguchi

[toc] | [prev] | [next] | [standalone]

#1518794 — Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

On Thu, Nov 10, 2016 at 05:28:20PM +0800, Hillf Danton wrote:
> On Thursday, November 10, 2016 5:22 PM Naoya Horiguchi wrote:
> > On Thu, Nov 10, 2016 at 05:08:07PM +0800, Hillf Danton wrote:
> > > On Tuesday, November 08, 2016 7:32 AM Naoya Horiguchi wrote:
> > > >
> > > > @@ -1013,6 +1027,9 @@ int do_huge_pmd_wp_page(struct fault_env *fe, pmd_t orig_pmd)
> > > >  	if (unlikely(!pmd_same(*fe->pmd, orig_pmd)))
> > > >  		goto out_unlock;
> > > >
> > > > +	if (unlikely(!pmd_present(orig_pmd)))
> > > > +		goto out_unlock;
> > > > +
> > >
> > > Can we encounter a migration entry after acquiring ptl ?
> > 
> > I think we can. thp migration code releases ptl after converting pmd into
> > migration entry, so other code can see it even within ptl.
> > 
> But we have a pmd_same check there, you see. 

You're right. So we can omit this pmd_present check.

Thanks,
Naoya Horiguchi

[toc] | [prev] | [next] | [standalone]

#1518801 — Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

On Thursday, November 10, 2016 5:22 PM Naoya Horiguchi wrote:
> On Thu, Nov 10, 2016 at 05:08:07PM +0800, Hillf Danton wrote:
> > On Tuesday, November 08, 2016 7:32 AM Naoya Horiguchi wrote:
> > >
> > > @@ -1013,6 +1027,9 @@ int do_huge_pmd_wp_page(struct fault_env *fe, pmd_t orig_pmd)
> > >  	if (unlikely(!pmd_same(*fe->pmd, orig_pmd)))
> > >  		goto out_unlock;
> > >
> > > +	if (unlikely(!pmd_present(orig_pmd)))
> > > +		goto out_unlock;
> > > +
> >
> > Can we encounter a migration entry after acquiring ptl ?
> 
> I think we can. thp migration code releases ptl after converting pmd into
> migration entry, so other code can see it even within ptl.
> 
But we have a pmd_same check there, you see. 

[toc] | [prev] | [next] | [standalone]

#1518784 — Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

On Tuesday, November 08, 2016 7:32 AM Naoya Horiguchi wrote:
> 
> @@ -1013,6 +1027,9 @@ int do_huge_pmd_wp_page(struct fault_env *fe, pmd_t orig_pmd)
>  	if (unlikely(!pmd_same(*fe->pmd, orig_pmd)))
>  		goto out_unlock;
> 
> +	if (unlikely(!pmd_present(orig_pmd)))
> +		goto out_unlock;
> +

Can we encounter a migration entry after acquiring ptl ?

>  	page = pmd_page(orig_pmd);
>  	VM_BUG_ON_PAGE(!PageCompound(page) || !PageHead(page), page);
>  	/*
[...]
> @@ -3591,6 +3591,10 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
>  		int ret;
> 
>  		barrier();
> +		if (unlikely(is_pmd_migration_entry(orig_pmd))) {
> +			pmd_migration_entry_wait(mm, fe.pmd);
> +			return 0;
> +		}
>  		if (pmd_trans_huge(orig_pmd) || pmd_devmap(orig_pmd)) {
>  			if (pmd_protnone(orig_pmd) && vma_is_accessible(vma))
>  				return do_huge_pmd_numa_page(&fe, orig_pmd);

[toc] | [prev] | [next] | [standalone]

#1516703 — [PATCH v2 06/12] mm: thp: enable thp migration in generic path

This patch makes it possible to support thp migration gradually. If you fail
to allocate a destination page as a thp, you just split the source thp as we
do now, and then enter the normal page migration. If you succeed to allocate
destination thp, you enter thp migration. Subsequent patches actually enable
thp migration for each caller of page migration by allowing its get_new_page()
callback to allocate thps.

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 mm/migrate.c | 2 +-
 mm/rmap.c    | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git v4.9-rc2-mmotm-2016-10-27-18-27/mm/migrate.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/migrate.c
index 54f2eb6..97ab8d9 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/mm/migrate.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/migrate.c
@@ -1142,7 +1142,7 @@ static ICE_noinline int unmap_and_move(new_page_t get_new_page,
 		goto out;
 	}
 
-	if (unlikely(PageTransHuge(page))) {
+	if (unlikely(PageTransHuge(page) && !PageTransHuge(newpage))) {
 		lock_page(page);
 		rc = split_huge_page(page);
 		unlock_page(page);
diff --git v4.9-rc2-mmotm-2016-10-27-18-27/mm/rmap.c v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/rmap.c
index a4be307..a0b665c 100644
--- v4.9-rc2-mmotm-2016-10-27-18-27/mm/rmap.c
+++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/mm/rmap.c
@@ -1443,6 +1443,13 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 	struct rmap_private *rp = arg;
 	enum ttu_flags flags = rp->flags;
 
+	if (flags & TTU_MIGRATION) {
+		if (!PageHuge(page) && PageTransCompound(page)) {
+			set_pmd_migration_entry(page, vma, address);
+			goto out;
+		}
+	}
+
 	/* munlock has nothing to gain from examining un-locked vmas */
 	if ((flags & TTU_MUNLOCK) && !(vma->vm_flags & VM_LOCKED))
 		goto out;
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1517736

On 08/11/16 10:31, Naoya Horiguchi wrote:
> Hi everyone,
> 
> I've updated thp migration patches for v4.9-rc2-mmotm-2016-10-27-18-27
> with feedbacks for ver.1.
> 
> General description (no change since ver.1)
> ===========================================
> 
> This patchset enhances page migration functionality to handle thp migration
> for various page migration's callers:
>  - mbind(2)
>  - move_pages(2)
>  - migrate_pages(2)
>  - cgroup/cpuset migration
>  - memory hotremove
>  - soft offline
> 
> The main benefit is that we can avoid unnecessary thp splits, which helps us
> avoid performance decrease when your applications handles NUMA optimization on
> their own.
> 
> The implementation is similar to that of normal page migration, the key point
> is that we modify a pmd to a pmd migration entry in swap-entry like format.
> 
> Changes / Notes
> ===============
> 
> - pmd_present() in x86 checks _PAGE_PRESENT, _PAGE_PROTNONE and _PAGE_PSE
>   bits together, which makes implementing thp migration a bit hard because
>   _PAGE_PSE bit is currently used by soft-dirty in swap-entry format.
>   I was advised to dropping _PAGE_PSE in pmd_present(), but I don't think
>   of the justification, so I keep it in this version. Instead, my approach
>   is to move _PAGE_SWP_SOFT_DIRTY to bit 6 (unused) and reserve bit 7 for
>   pmd non-present cases.

Thanks, IIRC

pmd_present = _PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PSE

AutoNUMA balancing would change it to

pmd_present = _PAGE_PROTNONE | _PAGE_PSE

and PMD_SWP_SOFT_DIRTY would make it

pmd_present = _PAGE_PSE

What you seem to be suggesting in your comment is that

pmd_present should be _PAGE_PRESENT | _PAGE_PROTNONE

Isn't that good enough?

For THP migration I guess we use

_PAGE_PRESENT | _PAGE_PROTNONE | is_migration_entry(pmd)


> 
> - this patchset still covers only x86_64. Zi Yan posted a patch for ppc64
>   and I think it's favorably received so that's fine. But there's unsolved
>   minor suggestion by Aneesh, so I don't include it in this set, expecting
>   that it will be updated/reposted.
> 
> - pte-mapped thp and doubly-mapped thp were not supported in ver.1, but
>   this version should work for such kinds of thp.
> 
> - thp page cache is not tested yet, and it's at the head of my todo list
>   for future version.
> 
> Any comments or advices are welcomed.

Balbir Singh

[toc] | [prev] | [next] | [standalone]

#1517778

On Wed, Nov 09, 2016 at 01:32:04PM +1100, Balbir Singh wrote:
> On 08/11/16 10:31, Naoya Horiguchi wrote:
> > Hi everyone,
> > 
> > I've updated thp migration patches for v4.9-rc2-mmotm-2016-10-27-18-27
> > with feedbacks for ver.1.
> > 
> > General description (no change since ver.1)
> > ===========================================
> > 
> > This patchset enhances page migration functionality to handle thp migration
> > for various page migration's callers:
> >  - mbind(2)
> >  - move_pages(2)
> >  - migrate_pages(2)
> >  - cgroup/cpuset migration
> >  - memory hotremove
> >  - soft offline
> > 
> > The main benefit is that we can avoid unnecessary thp splits, which helps us
> > avoid performance decrease when your applications handles NUMA optimization on
> > their own.
> > 
> > The implementation is similar to that of normal page migration, the key point
> > is that we modify a pmd to a pmd migration entry in swap-entry like format.
> > 
> > Changes / Notes
> > ===============
> > 
> > - pmd_present() in x86 checks _PAGE_PRESENT, _PAGE_PROTNONE and _PAGE_PSE
> >   bits together, which makes implementing thp migration a bit hard because
> >   _PAGE_PSE bit is currently used by soft-dirty in swap-entry format.
> >   I was advised to dropping _PAGE_PSE in pmd_present(), but I don't think
> >   of the justification, so I keep it in this version. Instead, my approach
> >   is to move _PAGE_SWP_SOFT_DIRTY to bit 6 (unused) and reserve bit 7 for
> >   pmd non-present cases.
> 
> Thanks, IIRC
> 
> pmd_present = _PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PSE
> 
> AutoNUMA balancing would change it to
> 
> pmd_present = _PAGE_PROTNONE | _PAGE_PSE
> 
> and PMD_SWP_SOFT_DIRTY would make it
> 
> pmd_present = _PAGE_PSE
> 
> What you seem to be suggesting in your comment is that
> 
> pmd_present should be _PAGE_PRESENT | _PAGE_PROTNONE

This (no _PAGE_PSE) was a possibile solution, and as I described I gave up
this solution, because I noticed that what I actually wanted was that
pmd_present() certainly returns false during thp migration and that's done
by moving _PAGE_SWP_SOFT_DIRTY. So

  pmd_present = _PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PSE

is still correct in this patchset.

> 
> Isn't that good enough?
> 
> For THP migration I guess we use
> 
> _PAGE_PRESENT | _PAGE_PROTNONE | is_migration_entry(pmd)

Though I might misread your notations, I hope that the following code
seems describe itself well.

  static inline int is_pmd_migration_entry(pmd_t pmd)                            
  {                                                                              
          return !pmd_present(pmd) && is_migration_entry(pmd_to_swp_entry(pmd)); 
  }                                                                              

Thanks,
Naoya Horiguchi

> 
> 
> > 
> > - this patchset still covers only x86_64. Zi Yan posted a patch for ppc64
> >   and I think it's favorably received so that's fine. But there's unsolved
> >   minor suggestion by Aneesh, so I don't include it in this set, expecting
> >   that it will be updated/reposted.
> > 
> > - pte-mapped thp and doubly-mapped thp were not supported in ver.1, but
> >   this version should work for such kinds of thp.
> > 
> > - thp page cache is not tested yet, and it's at the head of my todo list
> >   for future version.
> > 
> > Any comments or advices are welcomed.
> 
> Balbir Singh
> 

[toc] | [prev] | [next] | [standalone]

#1518497

On 09/11/16 15:59, Naoya Horiguchi wrote:
> On Wed, Nov 09, 2016 at 01:32:04PM +1100, Balbir Singh wrote:
>> On 08/11/16 10:31, Naoya Horiguchi wrote:
>>> Hi everyone,
>>>
>>> I've updated thp migration patches for v4.9-rc2-mmotm-2016-10-27-18-27
>>> with feedbacks for ver.1.
>>>
>>> General description (no change since ver.1)
>>> ===========================================
>>>
>>> This patchset enhances page migration functionality to handle thp migration
>>> for various page migration's callers:
>>>  - mbind(2)
>>>  - move_pages(2)
>>>  - migrate_pages(2)
>>>  - cgroup/cpuset migration
>>>  - memory hotremove
>>>  - soft offline
>>>
>>> The main benefit is that we can avoid unnecessary thp splits, which helps us
>>> avoid performance decrease when your applications handles NUMA optimization on
>>> their own.
>>>
>>> The implementation is similar to that of normal page migration, the key point
>>> is that we modify a pmd to a pmd migration entry in swap-entry like format.
>>>
>>> Changes / Notes
>>> ===============
>>>
>>> - pmd_present() in x86 checks _PAGE_PRESENT, _PAGE_PROTNONE and _PAGE_PSE
>>>   bits together, which makes implementing thp migration a bit hard because
>>>   _PAGE_PSE bit is currently used by soft-dirty in swap-entry format.
>>>   I was advised to dropping _PAGE_PSE in pmd_present(), but I don't think
>>>   of the justification, so I keep it in this version. Instead, my approach
>>>   is to move _PAGE_SWP_SOFT_DIRTY to bit 6 (unused) and reserve bit 7 for
>>>   pmd non-present cases.
>>
>> Thanks, IIRC
>>
>> pmd_present = _PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PSE
>>
>> AutoNUMA balancing would change it to
>>
>> pmd_present = _PAGE_PROTNONE | _PAGE_PSE
>>
>> and PMD_SWP_SOFT_DIRTY would make it
>>
>> pmd_present = _PAGE_PSE
>>
>> What you seem to be suggesting in your comment is that
>>
>> pmd_present should be _PAGE_PRESENT | _PAGE_PROTNONE
> 
> This (no _PAGE_PSE) was a possibile solution, and as I described I gave up
> this solution, because I noticed that what I actually wanted was that
> pmd_present() certainly returns false during thp migration and that's done
> by moving _PAGE_SWP_SOFT_DIRTY. So
> 
>   pmd_present = _PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PSE
> 
> is still correct in this patchset.
> 

Thanks, I was wondering if there is any advantage or you felt its
safer not to change pmd_present().

>>
>> Isn't that good enough?
>>
>> For THP migration I guess we use
>>
>> _PAGE_PRESENT | _PAGE_PROTNONE | is_migration_entry(pmd)
> 
> Though I might misread your notations, I hope that the following code
> seems describe itself well.
> 
>   static inline int is_pmd_migration_entry(pmd_t pmd)                            
>   {                                                                              
>           return !pmd_present(pmd) && is_migration_entry(pmd_to_swp_entry(pmd)); 
>   }                                                                              
> 

Thanks, yes my notation is not the best.



Balbir Singh.

[toc] | [prev] | [next] | [standalone]

#1517886

On 11/08/2016 05:01 AM, Naoya Horiguchi wrote:
> Hi everyone,
> 
> I've updated thp migration patches for v4.9-rc2-mmotm-2016-10-27-18-27
> with feedbacks for ver.1.
> 
> General description (no change since ver.1)
> ===========================================
> 
> This patchset enhances page migration functionality to handle thp migration
> for various page migration's callers:
>  - mbind(2)
>  - move_pages(2)
>  - migrate_pages(2)
>  - cgroup/cpuset migration
>  - memory hotremove
>  - soft offline
> 
> The main benefit is that we can avoid unnecessary thp splits, which helps us
> avoid performance decrease when your applications handles NUMA optimization on
> their own.
> 
> The implementation is similar to that of normal page migration, the key point
> is that we modify a pmd to a pmd migration entry in swap-entry like format.

Will it be better to have new THP_MIGRATE_SUCCESS and THP_MIGRATE_FAIL
VM events to capture how many times the migration worked without first
splitting the huge page and how many time it did not work ? Also do you
have a test case which demonstrates this THP migration and kind of shows
its better than the present split and move method ?

[toc] | [prev] | [next] | [standalone]

#1518583

Hi Anshuman,

On Wed, Nov 09, 2016 at 04:03:04PM +0530, Anshuman Khandual wrote:
> On 11/08/2016 05:01 AM, Naoya Horiguchi wrote:
> > Hi everyone,
> > 
> > I've updated thp migration patches for v4.9-rc2-mmotm-2016-10-27-18-27
> > with feedbacks for ver.1.
> > 
> > General description (no change since ver.1)
> > ===========================================
> > 
> > This patchset enhances page migration functionality to handle thp migration
> > for various page migration's callers:
> >  - mbind(2)
> >  - move_pages(2)
> >  - migrate_pages(2)
> >  - cgroup/cpuset migration
> >  - memory hotremove
> >  - soft offline
> > 
> > The main benefit is that we can avoid unnecessary thp splits, which helps us
> > avoid performance decrease when your applications handles NUMA optimization on
> > their own.
> > 
> > The implementation is similar to that of normal page migration, the key point
> > is that we modify a pmd to a pmd migration entry in swap-entry like format.
> 
> Will it be better to have new THP_MIGRATE_SUCCESS and THP_MIGRATE_FAIL
> VM events to capture how many times the migration worked without first
> splitting the huge page and how many time it did not work ?

Thank you for the suggestion.
I think that's helpful, so will try it in next version.

> Also do you
> have a test case which demonstrates this THP migration and kind of shows
> its better than the present split and move method ?

I don't have test cases which compare thp migration and split-then-migration
with some numbers. Maybe measuring/comparing the overhead of migration is
a good start point, although I think the real benefit of thp migration comes
from workload "after migration" by avoiding thp split.

Thanks,
Naoya Horiguchi

[toc] | [prev] | [next] | [standalone]

#1519548

On 11/10/2016 07:31 PM, Zi Yan wrote:
> On 9 Nov 2016, at 18:52, Naoya Horiguchi wrote:
> 
>> Hi Anshuman,
>>
>> On Wed, Nov 09, 2016 at 04:03:04PM +0530, Anshuman Khandual wrote:
>>> On 11/08/2016 05:01 AM, Naoya Horiguchi wrote:
>>>> Hi everyone,
>>>>
>>>> I've updated thp migration patches for v4.9-rc2-mmotm-2016-10-27-18-27
>>>> with feedbacks for ver.1.
>>>>
>>>> General description (no change since ver.1)
>>>> ===========================================
>>>>
>>>> This patchset enhances page migration functionality to handle thp migration
>>>> for various page migration's callers:
>>>>  - mbind(2)
>>>>  - move_pages(2)
>>>>  - migrate_pages(2)
>>>>  - cgroup/cpuset migration
>>>>  - memory hotremove
>>>>  - soft offline
>>>>
>>>> The main benefit is that we can avoid unnecessary thp splits, which helps us
>>>> avoid performance decrease when your applications handles NUMA optimization on
>>>> their own.
>>>>
>>>> The implementation is similar to that of normal page migration, the key point
>>>> is that we modify a pmd to a pmd migration entry in swap-entry like format.
>>>
>>> Will it be better to have new THP_MIGRATE_SUCCESS and THP_MIGRATE_FAIL
>>> VM events to capture how many times the migration worked without first
>>> splitting the huge page and how many time it did not work ?
>>
>> Thank you for the suggestion.
>> I think that's helpful, so will try it in next version.
>>
>>> Also do you
>>> have a test case which demonstrates this THP migration and kind of shows
>>> its better than the present split and move method ?
>>
>> I don't have test cases which compare thp migration and split-then-migration
>> with some numbers. Maybe measuring/comparing the overhead of migration is
>> a good start point, although I think the real benefit of thp migration comes
>> from workload "after migration" by avoiding thp split.
> 
> Migrating 4KB pages has much lower (~1/3) throughput than 2MB pages.

I assume the 2MB throughput you mentioned is with this THP migration
feature enabled.

> 
> What I get is that on average it takes 1987.38 us to migrate 512 4KB pages and
>                                        658.54  us to migrate 1   2MB page.
> 
> I did the test in a two-socket Intel Xeon E5-2640v4 box. I used migrate_pages()
> system call to migrate pages. MADV_NOHUGEPAGE and MADV_HUGEPAGE are used to
> make 4KB and 2MB pages and each page’s flags are checked to make sure the page
> size is 4KB or 2MB THP.
> 
> There is no split page. But the page migration time already tells the story.

Right. Just wondering if we can add a test case which measures just
this migration time improvement by avoiding the split not the TLB
based improvement which the workload will receive as an addition.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web