[PATCH v4 1/3] memcg: clean up existing move charge code

[PATCH v4 1/3] memcg: clean up existing move charge code

[Naoya Horiguchi]

We'll introduce the thp variant of move charge code in later patches,
but before doing that let's start with refactoring existing code.
Here we replace lengthy function name is_target_pte_for_mc() with
shorter one in order to avoid ugly line breaks.
And for better readability, we explicitly use MC_TARGET_* instead of
simply using integers.

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 mm/memcontrol.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git linux-next-20120307.orig/mm/memcontrol.c linux-next-20120307/mm/memcontrol.c
index a288855..3d16618 100644
--- linux-next-20120307.orig/mm/memcontrol.c
+++ linux-next-20120307/mm/memcontrol.c
@@ -5069,7 +5069,7 @@ one_by_one:
 }
 
 /**
- * is_target_pte_for_mc - check a pte whether it is valid for move charge
+ * get_mctgt_type - get target type of moving charge
  * @vma: the vma the pte to be checked belongs
  * @addr: the address corresponding to the pte to be checked
  * @ptent: the pte to be checked
@@ -5092,7 +5092,7 @@ union mc_target {
 };
 
 enum mc_target_type {
-	MC_TARGET_NONE,	/* not used */
+	MC_TARGET_NONE,
 	MC_TARGET_PAGE,
 	MC_TARGET_SWAP,
 };
@@ -5173,12 +5173,12 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
 	return page;
 }
 
-static int is_target_pte_for_mc(struct vm_area_struct *vma,
+static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
 		unsigned long addr, pte_t ptent, union mc_target *target)
 {
 	struct page *page = NULL;
 	struct page_cgroup *pc;
-	int ret = 0;
+	enum mc_target_type ret = MC_TARGET_NONE;
 	swp_entry_t ent = { .val = 0 };
 
 	if (pte_present(ptent))
@@ -5189,7 +5189,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 		page = mc_handle_file_pte(vma, addr, ptent, &ent);
 
 	if (!page && !ent.val)
-		return 0;
+		return ret;
 	if (page) {
 		pc = lookup_page_cgroup(page);
 		/*
@@ -5206,7 +5206,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 			put_page(page);
 	}
 	/* There is a swap entry and a page doesn't exist or isn't charged */
-	if (ent.val && !ret &&
+	if (ent.val && ret != MC_TARGET_NONE &&
 			css_id(&mc.from->css) == lookup_swap_cgroup_id(ent)) {
 		ret = MC_TARGET_SWAP;
 		if (target)
@@ -5227,7 +5227,7 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
 
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
 	for (; addr != end; pte++, addr += PAGE_SIZE)
-		if (is_target_pte_for_mc(vma, addr, *pte, NULL))
+		if (get_mctgt_type(vma, addr, *pte, NULL))
 			mc.precharge++;	/* increment precharge temporarily */
 	pte_unmap_unlock(pte - 1, ptl);
 	cond_resched();
@@ -5397,8 +5397,8 @@ retry:
 		if (!mc.precharge)
 			break;
 
-		type = is_target_pte_for_mc(vma, addr, ptent, &target);
-		switch (type) {
+		target_type = get_mctgt_type(vma, addr, ptent, &target);
+		switch (target_type) {
 		case MC_TARGET_PAGE:
 			page = target.page;
 			if (isolate_lru_page(page))
@@ -5411,7 +5411,7 @@ retry:
 				mc.moved_charge++;
 			}
 			putback_lru_page(page);
-put:			/* is_target_pte_for_mc() gets the page */
+put:			/* get_mctgt_type() gets the page */
 			put_page(page);
 			break;
 		case MC_TARGET_SWAP:
-- 
1.7.7.6

[PATCH v4 1/3] memcg: clean up existing move charge code

[Naoya Horiguchi]

We'll introduce the thp variant of move charge code in later patches,
but before doing that let's start with refactoring existing code.
Here we replace lengthy function name is_target_pte_for_mc() with
shorter one in order to avoid ugly line breaks.
And for better readability, we explicitly use MC_TARGET_* instead of
simply using integers.

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 mm/memcontrol.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git linux-next-20120307.orig/mm/memcontrol.c linux-next-20120307/mm/memcontrol.c
index a288855..3d16618 100644
--- linux-next-20120307.orig/mm/memcontrol.c
+++ linux-next-20120307/mm/memcontrol.c
@@ -5069,7 +5069,7 @@ one_by_one:
 }
 
 /**
- * is_target_pte_for_mc - check a pte whether it is valid for move charge
+ * get_mctgt_type - get target type of moving charge
  * @vma: the vma the pte to be checked belongs
  * @addr: the address corresponding to the pte to be checked
  * @ptent: the pte to be checked
@@ -5092,7 +5092,7 @@ union mc_target {
 };
 
 enum mc_target_type {
-	MC_TARGET_NONE,	/* not used */
+	MC_TARGET_NONE,
 	MC_TARGET_PAGE,
 	MC_TARGET_SWAP,
 };
@@ -5173,12 +5173,12 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
 	return page;
 }
 
-static int is_target_pte_for_mc(struct vm_area_struct *vma,
+static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
 		unsigned long addr, pte_t ptent, union mc_target *target)
 {
 	struct page *page = NULL;
 	struct page_cgroup *pc;
-	int ret = 0;
+	enum mc_target_type ret = MC_TARGET_NONE;
 	swp_entry_t ent = { .val = 0 };
 
 	if (pte_present(ptent))
@@ -5189,7 +5189,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 		page = mc_handle_file_pte(vma, addr, ptent, &ent);
 
 	if (!page && !ent.val)
-		return 0;
+		return ret;
 	if (page) {
 		pc = lookup_page_cgroup(page);
 		/*
@@ -5206,7 +5206,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 			put_page(page);
 	}
 	/* There is a swap entry and a page doesn't exist or isn't charged */
-	if (ent.val && !ret &&
+	if (ent.val && ret != MC_TARGET_NONE &&
 			css_id(&mc.from->css) == lookup_swap_cgroup_id(ent)) {
 		ret = MC_TARGET_SWAP;
 		if (target)
@@ -5227,7 +5227,7 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
 
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
 	for (; addr != end; pte++, addr += PAGE_SIZE)
-		if (is_target_pte_for_mc(vma, addr, *pte, NULL))
+		if (get_mctgt_type(vma, addr, *pte, NULL))
 			mc.precharge++;	/* increment precharge temporarily */
 	pte_unmap_unlock(pte - 1, ptl);
 	cond_resched();
@@ -5397,8 +5397,8 @@ retry:
 		if (!mc.precharge)
 			break;
 
-		type = is_target_pte_for_mc(vma, addr, ptent, &target);
-		switch (type) {
+		target_type = get_mctgt_type(vma, addr, ptent, &target);
+		switch (target_type) {
 		case MC_TARGET_PAGE:
 			page = target.page;
 			if (isolate_lru_page(page))
@@ -5411,7 +5411,7 @@ retry:
 				mc.moved_charge++;
 			}
 			putback_lru_page(page);
-put:			/* is_target_pte_for_mc() gets the page */
+put:			/* get_mctgt_type() gets the page */
 			put_page(page);
 			break;
 		case MC_TARGET_SWAP:
-- 
1.7.7.6

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Naoya Horiguchi]

These macros will be used in later patch, where all usage are expected
to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Acked-by: Hillf Danton <dhillf@gmail.com>
Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: David Rientjes <rientjes@google.com>
---
 include/linux/huge_mm.h |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git linux-next-20120307.orig/include/linux/huge_mm.h linux-next-20120307/include/linux/huge_mm.h
index f56cacb..c8af7a2 100644
--- linux-next-20120307.orig/include/linux/huge_mm.h
+++ linux-next-20120307/include/linux/huge_mm.h
@@ -51,6 +51,9 @@ extern pmd_t *page_check_address_pmd(struct page *page,
 				     unsigned long address,
 				     enum page_check_address_pmd_flag flag);
 
+#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
+#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 #define HPAGE_PMD_SHIFT HPAGE_SHIFT
 #define HPAGE_PMD_MASK HPAGE_MASK
@@ -102,8 +105,6 @@ extern void __split_huge_page_pmd(struct mm_struct *mm, pmd_t *pmd);
 		BUG_ON(pmd_trans_splitting(*____pmd) ||			\
 		       pmd_trans_huge(*____pmd));			\
 	} while (0)
-#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
-#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
 #if HPAGE_PMD_ORDER > MAX_ORDER
 #error "hugepages can't be allocated by the buddy allocator"
 #endif
@@ -158,9 +159,9 @@ static inline struct page *compound_trans_head(struct page *page)
 	return page;
 }
 #else /* CONFIG_TRANSPARENT_HUGEPAGE */
-#define HPAGE_PMD_SHIFT ({ BUG(); 0; })
-#define HPAGE_PMD_MASK ({ BUG(); 0; })
-#define HPAGE_PMD_SIZE ({ BUG(); 0; })
+#define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
+#define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
+#define HPAGE_PMD_SIZE ({ BUILD_BUG(); 0; })
 
 #define hpage_nr_pages(x) 1
 
-- 
1.7.7.6

[PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Naoya Horiguchi]

These macros will be used in later patch, where all usage are expected
to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Acked-by: Hillf Danton <dhillf@gmail.com>
Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: David Rientjes <rientjes@google.com>
---
 include/linux/huge_mm.h |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git linux-next-20120307.orig/include/linux/huge_mm.h linux-next-20120307/include/linux/huge_mm.h
index f56cacb..c8af7a2 100644
--- linux-next-20120307.orig/include/linux/huge_mm.h
+++ linux-next-20120307/include/linux/huge_mm.h
@@ -51,6 +51,9 @@ extern pmd_t *page_check_address_pmd(struct page *page,
 				     unsigned long address,
 				     enum page_check_address_pmd_flag flag);
 
+#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
+#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 #define HPAGE_PMD_SHIFT HPAGE_SHIFT
 #define HPAGE_PMD_MASK HPAGE_MASK
@@ -102,8 +105,6 @@ extern void __split_huge_page_pmd(struct mm_struct *mm, pmd_t *pmd);
 		BUG_ON(pmd_trans_splitting(*____pmd) ||			\
 		       pmd_trans_huge(*____pmd));			\
 	} while (0)
-#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
-#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
 #if HPAGE_PMD_ORDER > MAX_ORDER
 #error "hugepages can't be allocated by the buddy allocator"
 #endif
@@ -158,9 +159,9 @@ static inline struct page *compound_trans_head(struct page *page)
 	return page;
 }
 #else /* CONFIG_TRANSPARENT_HUGEPAGE */
-#define HPAGE_PMD_SHIFT ({ BUG(); 0; })
-#define HPAGE_PMD_MASK ({ BUG(); 0; })
-#define HPAGE_PMD_SIZE ({ BUG(); 0; })
+#define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
+#define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
+#define HPAGE_PMD_SIZE ({ BUILD_BUG(); 0; })
 
 #define hpage_nr_pages(x) 1
 
-- 
1.7.7.6

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH v4 3/3] memcg: avoid THP split in task migration

[Naoya Horiguchi]

Currently we can't do task migration among memory cgroups without THP split,
which means processes heavily using THP experience large overhead in task
migration. This patch introduces the code for moving charge of THP and makes
THP more valuable.

Changes from v3:
- use enum mc_target_type and MC_TARGET_* explicitly
- replace lengthy name is_target_thp_for_mc() with get_mctgt_type_thp()
- drop cond_resched()
- drop mapcount check of page sharing (Hugh and KAMEZAWA-san are preparing
  patches to change the behavior of moving charge of shared pages, so this
  patch keeps up with the change to avoid potential conflict.)

Changes from v2:
- add move_anon() and mapcount check

Changes from v1:
- rename is_target_huge_pmd_for_mc() to is_target_thp_for_mc()
- remove pmd_present() check (it's buggy when pmd_trans_huge(pmd) is true)
- is_target_thp_for_mc() calls get_page() only when checks are passed
- unlock page table lock if !mc.precharge
- compare return value of is_target_thp_for_mc() explicitly to MC_TARGET_TYPE
- clean up &walk->mm->page_table_lock to &vma->vm_mm->page_table_lock
- add comment about why race with split_huge_page() does not happen

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Acked-by: Hillf Danton <dhillf@gmail.com>
---
 mm/memcontrol.c |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 79 insertions(+), 6 deletions(-)

diff --git linux-next-20120307.orig/mm/memcontrol.c linux-next-20120307/mm/memcontrol.c
index 3d16618..7b345a3 100644
--- linux-next-20120307.orig/mm/memcontrol.c
+++ linux-next-20120307/mm/memcontrol.c
@@ -5215,6 +5215,41 @@ static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
 	return ret;
 }
 
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+/*
+ * We don't consider swapping or file mapped pages because THP does not
+ * support them for now.
+ * Caller should make sure that pmd_trans_huge(pmd) is true.
+ */
+static enum mc_target_type get_mctgt_type_thp(struct vm_area_struct *vma,
+		unsigned long addr, pmd_t pmd, union mc_target *target)
+{
+	struct page *page = NULL;
+	struct page_cgroup *pc;
+	enum mc_target_type ret = MC_TARGET_NONE;
+
+	page = pmd_page(pmd);
+	VM_BUG_ON(!page || !PageHead(page));
+	if (!move_anon())
+		return ret;
+	pc = lookup_page_cgroup(page);
+	if (PageCgroupUsed(pc) && pc->mem_cgroup == mc.from) {
+		ret = MC_TARGET_PAGE;
+		if (target) {
+			get_page(page);
+			target->page = page;
+		}
+	}
+	return ret;
+}
+#else
+static inline enum mc_target_type get_mctgt_type_thp(struct vm_area_struct *vma,
+		unsigned long addr, pmd_t pmd, union mc_target *target)
+{
+	return MC_TARGET_NONE;
+}
+#endif
+
 static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
 					unsigned long addr, unsigned long end,
 					struct mm_walk *walk)
@@ -5223,7 +5258,12 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
 	pte_t *pte;
 	spinlock_t *ptl;
 
-	split_huge_page_pmd(walk->mm, pmd);
+	if (pmd_trans_huge_lock(pmd, vma) == 1) {
+		if (get_mctgt_type_thp(vma, addr, *pmd, NULL) == MC_TARGET_PAGE)
+			mc.precharge += HPAGE_PMD_NR;
+		spin_unlock(&vma->vm_mm->page_table_lock);
+		return 0;
+	}
 
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
 	for (; addr != end; pte++, addr += PAGE_SIZE)
@@ -5382,16 +5422,49 @@ static int mem_cgroup_move_charge_pte_range(pmd_t *pmd,
 	struct vm_area_struct *vma = walk->private;
 	pte_t *pte;
 	spinlock_t *ptl;
+	enum mc_target_type target_type;
+	union mc_target target;
+	struct page *page;
+	struct page_cgroup *pc;
+
+	/*
+	 * We don't take compound_lock() here but no race with splitting thp
+	 * happens because:
+	 *  - if pmd_trans_huge_lock() returns 1, the relevant thp is not
+	 *    under splitting, which means there's no concurrent thp split,
+	 *  - if another thread runs into split_huge_page() just after we
+	 *    entered this if-block, the thread must wait for page table lock
+	 *    to be unlocked in __split_huge_page_splitting(), where the main
+	 *    part of thp split is not executed yet.
+	 */
+	if (pmd_trans_huge_lock(pmd, vma) == 1) {
+		if (!mc.precharge) {
+			spin_unlock(&vma->vm_mm->page_table_lock);
+			return 0;
+		}
+		target_type = get_mctgt_type_thp(vma, addr, *pmd, &target);
+		if (target_type == MC_TARGET_PAGE) {
+			page = target.page;
+			if (!isolate_lru_page(page)) {
+				pc = lookup_page_cgroup(page);
+				if (!mem_cgroup_move_account(page, HPAGE_PMD_NR,
+							     pc, mc.from, mc.to,
+							     false)) {
+					mc.precharge -= HPAGE_PMD_NR;
+					mc.moved_charge += HPAGE_PMD_NR;
+				}
+				putback_lru_page(page);
+			}
+			put_page(page);
+		}
+		spin_unlock(&vma->vm_mm->page_table_lock);
+		return 0;
+	}
 
-	split_huge_page_pmd(walk->mm, pmd);
 retry:
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
 	for (; addr != end; addr += PAGE_SIZE) {
 		pte_t ptent = *(pte++);
-		union mc_target target;
-		int type;
-		struct page *page;
-		struct page_cgroup *pc;
 		swp_entry_t ent;
 
 		if (!mc.precharge)
-- 
1.7.7.6

[PATCH v4 3/3] memcg: avoid THP split in task migration

[Naoya Horiguchi]

Currently we can't do task migration among memory cgroups without THP split,
which means processes heavily using THP experience large overhead in task
migration. This patch introduces the code for moving charge of THP and makes
THP more valuable.

Changes from v3:
- use enum mc_target_type and MC_TARGET_* explicitly
- replace lengthy name is_target_thp_for_mc() with get_mctgt_type_thp()
- drop cond_resched()
- drop mapcount check of page sharing (Hugh and KAMEZAWA-san are preparing
  patches to change the behavior of moving charge of shared pages, so this
  patch keeps up with the change to avoid potential conflict.)

Changes from v2:
- add move_anon() and mapcount check

Changes from v1:
- rename is_target_huge_pmd_for_mc() to is_target_thp_for_mc()
- remove pmd_present() check (it's buggy when pmd_trans_huge(pmd) is true)
- is_target_thp_for_mc() calls get_page() only when checks are passed
- unlock page table lock if !mc.precharge
- compare return value of is_target_thp_for_mc() explicitly to MC_TARGET_TYPE
- clean up &walk->mm->page_table_lock to &vma->vm_mm->page_table_lock
- add comment about why race with split_huge_page() does not happen

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Acked-by: Hillf Danton <dhillf@gmail.com>
---
 mm/memcontrol.c |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 79 insertions(+), 6 deletions(-)

diff --git linux-next-20120307.orig/mm/memcontrol.c linux-next-20120307/mm/memcontrol.c
index 3d16618..7b345a3 100644
--- linux-next-20120307.orig/mm/memcontrol.c
+++ linux-next-20120307/mm/memcontrol.c
@@ -5215,6 +5215,41 @@ static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
 	return ret;
 }
 
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+/*
+ * We don't consider swapping or file mapped pages because THP does not
+ * support them for now.
+ * Caller should make sure that pmd_trans_huge(pmd) is true.
+ */
+static enum mc_target_type get_mctgt_type_thp(struct vm_area_struct *vma,
+		unsigned long addr, pmd_t pmd, union mc_target *target)
+{
+	struct page *page = NULL;
+	struct page_cgroup *pc;
+	enum mc_target_type ret = MC_TARGET_NONE;
+
+	page = pmd_page(pmd);
+	VM_BUG_ON(!page || !PageHead(page));
+	if (!move_anon())
+		return ret;
+	pc = lookup_page_cgroup(page);
+	if (PageCgroupUsed(pc) && pc->mem_cgroup == mc.from) {
+		ret = MC_TARGET_PAGE;
+		if (target) {
+			get_page(page);
+			target->page = page;
+		}
+	}
+	return ret;
+}
+#else
+static inline enum mc_target_type get_mctgt_type_thp(struct vm_area_struct *vma,
+		unsigned long addr, pmd_t pmd, union mc_target *target)
+{
+	return MC_TARGET_NONE;
+}
+#endif
+
 static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
 					unsigned long addr, unsigned long end,
 					struct mm_walk *walk)
@@ -5223,7 +5258,12 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
 	pte_t *pte;
 	spinlock_t *ptl;
 
-	split_huge_page_pmd(walk->mm, pmd);
+	if (pmd_trans_huge_lock(pmd, vma) == 1) {
+		if (get_mctgt_type_thp(vma, addr, *pmd, NULL) == MC_TARGET_PAGE)
+			mc.precharge += HPAGE_PMD_NR;
+		spin_unlock(&vma->vm_mm->page_table_lock);
+		return 0;
+	}
 
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
 	for (; addr != end; pte++, addr += PAGE_SIZE)
@@ -5382,16 +5422,49 @@ static int mem_cgroup_move_charge_pte_range(pmd_t *pmd,
 	struct vm_area_struct *vma = walk->private;
 	pte_t *pte;
 	spinlock_t *ptl;
+	enum mc_target_type target_type;
+	union mc_target target;
+	struct page *page;
+	struct page_cgroup *pc;
+
+	/*
+	 * We don't take compound_lock() here but no race with splitting thp
+	 * happens because:
+	 *  - if pmd_trans_huge_lock() returns 1, the relevant thp is not
+	 *    under splitting, which means there's no concurrent thp split,
+	 *  - if another thread runs into split_huge_page() just after we
+	 *    entered this if-block, the thread must wait for page table lock
+	 *    to be unlocked in __split_huge_page_splitting(), where the main
+	 *    part of thp split is not executed yet.
+	 */
+	if (pmd_trans_huge_lock(pmd, vma) == 1) {
+		if (!mc.precharge) {
+			spin_unlock(&vma->vm_mm->page_table_lock);
+			return 0;
+		}
+		target_type = get_mctgt_type_thp(vma, addr, *pmd, &target);
+		if (target_type == MC_TARGET_PAGE) {
+			page = target.page;
+			if (!isolate_lru_page(page)) {
+				pc = lookup_page_cgroup(page);
+				if (!mem_cgroup_move_account(page, HPAGE_PMD_NR,
+							     pc, mc.from, mc.to,
+							     false)) {
+					mc.precharge -= HPAGE_PMD_NR;
+					mc.moved_charge += HPAGE_PMD_NR;
+				}
+				putback_lru_page(page);
+			}
+			put_page(page);
+		}
+		spin_unlock(&vma->vm_mm->page_table_lock);
+		return 0;
+	}
 
-	split_huge_page_pmd(walk->mm, pmd);
 retry:
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
 	for (; addr != end; addr += PAGE_SIZE) {
 		pte_t ptent = *(pte++);
-		union mc_target target;
-		int type;
-		struct page *page;
-		struct page_cgroup *pc;
 		swp_entry_t ent;
 
 		if (!mc.precharge)
-- 
1.7.7.6

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v4 1/3] memcg: clean up existing move charge code

[KAMEZAWA Hiroyuki]

On Mon, 12 Mar 2012 18:30:54 -0400
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:

> We'll introduce the thp variant of move charge code in later patches,
> but before doing that let's start with refactoring existing code.
> Here we replace lengthy function name is_target_pte_for_mc() with
> shorter one in order to avoid ugly line breaks.
> And for better readability, we explicitly use MC_TARGET_* instead of
> simply using integers.
> 
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

Thanks.

Seems ok to me.
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.futjisu.com>

Hmm. some nitpicks.

> ---
>  mm/memcontrol.c |   20 ++++++++++----------
>  1 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git linux-next-20120307.orig/mm/memcontrol.c linux-next-20120307/mm/memcontrol.c
> index a288855..3d16618 100644
> --- linux-next-20120307.orig/mm/memcontrol.c
> +++ linux-next-20120307/mm/memcontrol.c
> @@ -5069,7 +5069,7 @@ one_by_one:
>  }
>  
>  /**
> - * is_target_pte_for_mc - check a pte whether it is valid for move charge
> + * get_mctgt_type - get target type of moving charge
>   * @vma: the vma the pte to be checked belongs
>   * @addr: the address corresponding to the pte to be checked
>   * @ptent: the pte to be checked
> @@ -5092,7 +5092,7 @@ union mc_target {
>  };
>  
>  enum mc_target_type {
> -	MC_TARGET_NONE,	/* not used */
> +	MC_TARGET_NONE, 

How about

	MC_TARGET_NONE = 0,

Becasue you use 
	if (get_mctgt_type()) 
later.



>  	MC_TARGET_PAGE,
>  	MC_TARGET_SWAP,
>  };
> @@ -5173,12 +5173,12 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
>  	return page;
>  }
>  
> -static int is_target_pte_for_mc(struct vm_area_struct *vma,
> +static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
>  		unsigned long addr, pte_t ptent, union mc_target *target)

I admit old name is too long. But...Hm...get_mctgt_type()...how about

	move_charge_type() or
	mctgt_type() or
	mc_type() ?

I don't have good sense of naming ;(

>  {
>  	struct page *page = NULL;
>  	struct page_cgroup *pc;
> -	int ret = 0;
> +	enum mc_target_type ret = MC_TARGET_NONE;
>  	swp_entry_t ent = { .val = 0 };
>  
>  	if (pte_present(ptent))
> @@ -5189,7 +5189,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  		page = mc_handle_file_pte(vma, addr, ptent, &ent);
>  
>  	if (!page && !ent.val)
> -		return 0;
> +		return ret;
>  	if (page) {
>  		pc = lookup_page_cgroup(page);
>  		/*
> @@ -5206,7 +5206,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  			put_page(page);
>  	}
>  	/* There is a swap entry and a page doesn't exist or isn't charged */
> -	if (ent.val && !ret &&
> +	if (ent.val && ret != MC_TARGET_NONE &&

If you do MC_TARGET_NONE = 0 in above, using !ret seems ok to me.

>  			css_id(&mc.from->css) == lookup_swap_cgroup_id(ent)) {
>  		ret = MC_TARGET_SWAP;
>  		if (target)
> @@ -5227,7 +5227,7 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
>  
>  	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
>  	for (; addr != end; pte++, addr += PAGE_SIZE)
> -		if (is_target_pte_for_mc(vma, addr, *pte, NULL))
> +		if (get_mctgt_type(vma, addr, *pte, NULL))
>  			mc.precharge++;	/* increment precharge temporarily */
>  	pte_unmap_unlock(pte - 1, ptl);
>  	cond_resched();
> @@ -5397,8 +5397,8 @@ retry:
>  		if (!mc.precharge)
>  			break;
>  
> -		type = is_target_pte_for_mc(vma, addr, ptent, &target);
> -		switch (type) {
> +		target_type = get_mctgt_type(vma, addr, ptent, &target);
> +		switch (target_type) {

It 'target_type' is unused later
 
	switch(get_mctgt_type(vma, addr, ptent, &target))

is ok ?

Thanks,
-Kame

>  		case MC_TARGET_PAGE:
>  			page = target.page;
>  			if (isolate_lru_page(page))
> @@ -5411,7 +5411,7 @@ retry:
>  				mc.moved_charge++;
>  			}
>  			putback_lru_page(page);
> -put:			/* is_target_pte_for_mc() gets the page */
> +put:			/* get_mctgt_type() gets the page */
>  			put_page(page);
>  			break;
>  		case MC_TARGET_SWAP:
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] memcg: clean up existing move charge code

[KAMEZAWA Hiroyuki]

On Mon, 12 Mar 2012 18:30:54 -0400
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:

> We'll introduce the thp variant of move charge code in later patches,
> but before doing that let's start with refactoring existing code.
> Here we replace lengthy function name is_target_pte_for_mc() with
> shorter one in order to avoid ugly line breaks.
> And for better readability, we explicitly use MC_TARGET_* instead of
> simply using integers.
> 
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

Thanks.

Seems ok to me.
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.futjisu.com>

Hmm. some nitpicks.

> ---
>  mm/memcontrol.c |   20 ++++++++++----------
>  1 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git linux-next-20120307.orig/mm/memcontrol.c linux-next-20120307/mm/memcontrol.c
> index a288855..3d16618 100644
> --- linux-next-20120307.orig/mm/memcontrol.c
> +++ linux-next-20120307/mm/memcontrol.c
> @@ -5069,7 +5069,7 @@ one_by_one:
>  }
>  
>  /**
> - * is_target_pte_for_mc - check a pte whether it is valid for move charge
> + * get_mctgt_type - get target type of moving charge
>   * @vma: the vma the pte to be checked belongs
>   * @addr: the address corresponding to the pte to be checked
>   * @ptent: the pte to be checked
> @@ -5092,7 +5092,7 @@ union mc_target {
>  };
>  
>  enum mc_target_type {
> -	MC_TARGET_NONE,	/* not used */
> +	MC_TARGET_NONE, 

How about

	MC_TARGET_NONE = 0,

Becasue you use 
	if (get_mctgt_type()) 
later.



>  	MC_TARGET_PAGE,
>  	MC_TARGET_SWAP,
>  };
> @@ -5173,12 +5173,12 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
>  	return page;
>  }
>  
> -static int is_target_pte_for_mc(struct vm_area_struct *vma,
> +static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
>  		unsigned long addr, pte_t ptent, union mc_target *target)

I admit old name is too long. But...Hm...get_mctgt_type()...how about

	move_charge_type() or
	mctgt_type() or
	mc_type() ?

I don't have good sense of naming ;(

>  {
>  	struct page *page = NULL;
>  	struct page_cgroup *pc;
> -	int ret = 0;
> +	enum mc_target_type ret = MC_TARGET_NONE;
>  	swp_entry_t ent = { .val = 0 };
>  
>  	if (pte_present(ptent))
> @@ -5189,7 +5189,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  		page = mc_handle_file_pte(vma, addr, ptent, &ent);
>  
>  	if (!page && !ent.val)
> -		return 0;
> +		return ret;
>  	if (page) {
>  		pc = lookup_page_cgroup(page);
>  		/*
> @@ -5206,7 +5206,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  			put_page(page);
>  	}
>  	/* There is a swap entry and a page doesn't exist or isn't charged */
> -	if (ent.val && !ret &&
> +	if (ent.val && ret != MC_TARGET_NONE &&

If you do MC_TARGET_NONE = 0 in above, using !ret seems ok to me.

>  			css_id(&mc.from->css) == lookup_swap_cgroup_id(ent)) {
>  		ret = MC_TARGET_SWAP;
>  		if (target)
> @@ -5227,7 +5227,7 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
>  
>  	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
>  	for (; addr != end; pte++, addr += PAGE_SIZE)
> -		if (is_target_pte_for_mc(vma, addr, *pte, NULL))
> +		if (get_mctgt_type(vma, addr, *pte, NULL))
>  			mc.precharge++;	/* increment precharge temporarily */
>  	pte_unmap_unlock(pte - 1, ptl);
>  	cond_resched();
> @@ -5397,8 +5397,8 @@ retry:
>  		if (!mc.precharge)
>  			break;
>  
> -		type = is_target_pte_for_mc(vma, addr, ptent, &target);
> -		switch (type) {
> +		target_type = get_mctgt_type(vma, addr, ptent, &target);
> +		switch (target_type) {

It 'target_type' is unused later
 
	switch(get_mctgt_type(vma, addr, ptent, &target))

is ok ?

Thanks,
-Kame

>  		case MC_TARGET_PAGE:
>  			page = target.page;
>  			if (isolate_lru_page(page))
> @@ -5411,7 +5411,7 @@ retry:
>  				mc.moved_charge++;
>  			}
>  			putback_lru_page(page);
> -put:			/* is_target_pte_for_mc() gets the page */
> +put:			/* get_mctgt_type() gets the page */
>  			put_page(page);
>  			break;
>  		case MC_TARGET_SWAP:
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v4 3/3] memcg: avoid THP split in task migration

[KAMEZAWA Hiroyuki]

On Mon, 12 Mar 2012 18:30:56 -0400
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:

> Currently we can't do task migration among memory cgroups without THP split,
> which means processes heavily using THP experience large overhead in task
> migration. This patch introduces the code for moving charge of THP and makes
> THP more valuable.
> 
> Changes from v3:
> - use enum mc_target_type and MC_TARGET_* explicitly
> - replace lengthy name is_target_thp_for_mc() with get_mctgt_type_thp()
> - drop cond_resched()
> - drop mapcount check of page sharing (Hugh and KAMEZAWA-san are preparing
>   patches to change the behavior of moving charge of shared pages, so this
>   patch keeps up with the change to avoid potential conflict.)
> 
> Changes from v2:
> - add move_anon() and mapcount check
> 
> Changes from v1:
> - rename is_target_huge_pmd_for_mc() to is_target_thp_for_mc()
> - remove pmd_present() check (it's buggy when pmd_trans_huge(pmd) is true)
> - is_target_thp_for_mc() calls get_page() only when checks are passed
> - unlock page table lock if !mc.precharge
> - compare return value of is_target_thp_for_mc() explicitly to MC_TARGET_TYPE
> - clean up &walk->mm->page_table_lock to &vma->vm_mm->page_table_lock
> - add comment about why race with split_huge_page() does not happen
> 
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Acked-by: Hillf Danton <dhillf@gmail.com>

Thank you.

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Re: [PATCH v4 3/3] memcg: avoid THP split in task migration

[KAMEZAWA Hiroyuki]

On Mon, 12 Mar 2012 18:30:56 -0400
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:

> Currently we can't do task migration among memory cgroups without THP split,
> which means processes heavily using THP experience large overhead in task
> migration. This patch introduces the code for moving charge of THP and makes
> THP more valuable.
> 
> Changes from v3:
> - use enum mc_target_type and MC_TARGET_* explicitly
> - replace lengthy name is_target_thp_for_mc() with get_mctgt_type_thp()
> - drop cond_resched()
> - drop mapcount check of page sharing (Hugh and KAMEZAWA-san are preparing
>   patches to change the behavior of moving charge of shared pages, so this
>   patch keeps up with the change to avoid potential conflict.)
> 
> Changes from v2:
> - add move_anon() and mapcount check
> 
> Changes from v1:
> - rename is_target_huge_pmd_for_mc() to is_target_thp_for_mc()
> - remove pmd_present() check (it's buggy when pmd_trans_huge(pmd) is true)
> - is_target_thp_for_mc() calls get_page() only when checks are passed
> - unlock page table lock if !mc.precharge
> - compare return value of is_target_thp_for_mc() explicitly to MC_TARGET_TYPE
> - clean up &walk->mm->page_table_lock to &vma->vm_mm->page_table_lock
> - add comment about why race with split_huge_page() does not happen
> 
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Acked-by: Hillf Danton <dhillf@gmail.com>

Thank you.

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v4 1/3] memcg: clean up existing move charge code

[Naoya Horiguchi]

On Tue, Mar 13, 2012 at 02:45:01PM +0900, KAMEZAWA Hiroyuki wrote:
> On Mon, 12 Mar 2012 18:30:54 -0400
> Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:
> 
> > We'll introduce the thp variant of move charge code in later patches,
> > but before doing that let's start with refactoring existing code.
> > Here we replace lengthy function name is_target_pte_for_mc() with
> > shorter one in order to avoid ugly line breaks.
> > And for better readability, we explicitly use MC_TARGET_* instead of
> > simply using integers.
> > 
> > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> 
> Thanks.
> 
> Seems ok to me.
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.futjisu.com>

Thank you.

> Hmm. some nitpicks.
> 
> > ---
> >  mm/memcontrol.c |   20 ++++++++++----------
> >  1 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git linux-next-20120307.orig/mm/memcontrol.c linux-next-20120307/mm/memcontrol.c
> > index a288855..3d16618 100644
> > --- linux-next-20120307.orig/mm/memcontrol.c
> > +++ linux-next-20120307/mm/memcontrol.c
> > @@ -5069,7 +5069,7 @@ one_by_one:
> >  }
> >  
> >  /**
> > - * is_target_pte_for_mc - check a pte whether it is valid for move charge
> > + * get_mctgt_type - get target type of moving charge
> >   * @vma: the vma the pte to be checked belongs
> >   * @addr: the address corresponding to the pte to be checked
> >   * @ptent: the pte to be checked
> > @@ -5092,7 +5092,7 @@ union mc_target {
> >  };
> >  
> >  enum mc_target_type {
> > -	MC_TARGET_NONE,	/* not used */
> > +	MC_TARGET_NONE, 
> 
> How about
> 
> 	MC_TARGET_NONE = 0,

Sounds good.

> Becasue you use 
> 	if (get_mctgt_type()) 
> later.

In previous discussion, I got the feedback to compare the returned value
of get_mctgt_type() with MC_TARGET_*.
If we obey the advice, if (get_mctgt_type() == MC_TARGET_NONE)" is better,
but here we define MC_TARGET_NONE as 0 explicitly, so I think it's OK to
use the short form you suggest.

> 
> >  	MC_TARGET_PAGE,
> >  	MC_TARGET_SWAP,
> >  };
> > @@ -5173,12 +5173,12 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
> >  	return page;
> >  }
> >  
> > -static int is_target_pte_for_mc(struct vm_area_struct *vma,
> > +static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
> >  		unsigned long addr, pte_t ptent, union mc_target *target)
> 
> I admit old name is too long. But...Hm...get_mctgt_type()...how about
> 
> 	move_charge_type() or
> 	mctgt_type() or
> 	mc_type() ?
> 
> I don't have good sense of naming ;(

Shorter one is generally good, but too short one doesn't show what it does.
The first one appears to say it moves something. Latter two looks unclear to
me because they don't contain any verbs. So I'm afraid I like the current one.

> >  {
> >  	struct page *page = NULL;
> >  	struct page_cgroup *pc;
> > -	int ret = 0;
> > +	enum mc_target_type ret = MC_TARGET_NONE;
> >  	swp_entry_t ent = { .val = 0 };
> >  
> >  	if (pte_present(ptent))
> > @@ -5189,7 +5189,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
> >  		page = mc_handle_file_pte(vma, addr, ptent, &ent);
> >  
> >  	if (!page && !ent.val)
> > -		return 0;
> > +		return ret;
> >  	if (page) {
> >  		pc = lookup_page_cgroup(page);
> >  		/*
> > @@ -5206,7 +5206,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
> >  			put_page(page);
> >  	}
> >  	/* There is a swap entry and a page doesn't exist or isn't charged */
> > -	if (ent.val && !ret &&
> > +	if (ent.val && ret != MC_TARGET_NONE &&
> 
> If you do MC_TARGET_NONE = 0 in above, using !ret seems ok to me.

OK. It's clearer.

> >  			css_id(&mc.from->css) == lookup_swap_cgroup_id(ent)) {
> >  		ret = MC_TARGET_SWAP;
> >  		if (target)
> > @@ -5227,7 +5227,7 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
> >  
> >  	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> >  	for (; addr != end; pte++, addr += PAGE_SIZE)
> > -		if (is_target_pte_for_mc(vma, addr, *pte, NULL))
> > +		if (get_mctgt_type(vma, addr, *pte, NULL))
> >  			mc.precharge++;	/* increment precharge temporarily */
> >  	pte_unmap_unlock(pte - 1, ptl);
> >  	cond_resched();
> > @@ -5397,8 +5397,8 @@ retry:
> >  		if (!mc.precharge)
> >  			break;
> >  
> > -		type = is_target_pte_for_mc(vma, addr, ptent, &target);
> > -		switch (type) {
> > +		target_type = get_mctgt_type(vma, addr, ptent, &target);
> > +		switch (target_type) {
> 
> It 'target_type' is unused later
>  
> 	switch(get_mctgt_type(vma, addr, ptent, &target))
> 
> is ok ?

Yes. That's OK.

So I attach the revised patch below.
The applied changes do not affect later patches.

Thanks,
Naoya Horiguchi
---
>From c9bdd8f19040f3cea1c7d36e98b03ee13c1b8505 Mon Sep 17 00:00:00 2001
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Date: Tue, 13 Mar 2012 15:21:47 -0400
Subject: [PATCH 1/3] memcg: clean up existing move charge code

We'll introduce the thp variant of move charge code in later patches,
but before doing that let's start with refactoring existing code.
Here we replace lengthy function name is_target_pte_for_mc() with
shorter one in order to avoid ugly line breaks.
And for better readability, we explicitly use MC_TARGET_* instead of
simply using integers.

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.futjisu.com>
---
 mm/memcontrol.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a288855..508a7ed 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5069,7 +5069,7 @@ one_by_one:
 }
 
 /**
- * is_target_pte_for_mc - check a pte whether it is valid for move charge
+ * get_mctgt_type - get target type of moving charge
  * @vma: the vma the pte to be checked belongs
  * @addr: the address corresponding to the pte to be checked
  * @ptent: the pte to be checked
@@ -5092,7 +5092,7 @@ union mc_target {
 };
 
 enum mc_target_type {
-	MC_TARGET_NONE,	/* not used */
+	MC_TARGET_NONE = 0,
 	MC_TARGET_PAGE,
 	MC_TARGET_SWAP,
 };
@@ -5173,12 +5173,12 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
 	return page;
 }
 
-static int is_target_pte_for_mc(struct vm_area_struct *vma,
+static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
 		unsigned long addr, pte_t ptent, union mc_target *target)
 {
 	struct page *page = NULL;
 	struct page_cgroup *pc;
-	int ret = 0;
+	enum mc_target_type ret = MC_TARGET_NONE;
 	swp_entry_t ent = { .val = 0 };
 
 	if (pte_present(ptent))
@@ -5189,7 +5189,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 		page = mc_handle_file_pte(vma, addr, ptent, &ent);
 
 	if (!page && !ent.val)
-		return 0;
+		return ret;
 	if (page) {
 		pc = lookup_page_cgroup(page);
 		/*
@@ -5227,7 +5227,7 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
 
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
 	for (; addr != end; pte++, addr += PAGE_SIZE)
-		if (is_target_pte_for_mc(vma, addr, *pte, NULL))
+		if (get_mctgt_type(vma, addr, *pte, NULL))
 			mc.precharge++;	/* increment precharge temporarily */
 	pte_unmap_unlock(pte - 1, ptl);
 	cond_resched();
@@ -5397,8 +5397,7 @@ retry:
 		if (!mc.precharge)
 			break;
 
-		type = is_target_pte_for_mc(vma, addr, ptent, &target);
-		switch (type) {
+		switch (get_mctgt_type(vma, addr, ptent, &target)) {
 		case MC_TARGET_PAGE:
 			page = target.page;
 			if (isolate_lru_page(page))
@@ -5411,7 +5410,7 @@ retry:
 				mc.moved_charge++;
 			}
 			putback_lru_page(page);
-put:			/* is_target_pte_for_mc() gets the page */
+put:			/* get_mctgt_type() gets the page */
 			put_page(page);
 			break;
 		case MC_TARGET_SWAP:
-- 
1.7.7.6

Re: [PATCH v4 1/3] memcg: clean up existing move charge code

[Naoya Horiguchi]

On Tue, Mar 13, 2012 at 02:45:01PM +0900, KAMEZAWA Hiroyuki wrote:
> On Mon, 12 Mar 2012 18:30:54 -0400
> Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:
> 
> > We'll introduce the thp variant of move charge code in later patches,
> > but before doing that let's start with refactoring existing code.
> > Here we replace lengthy function name is_target_pte_for_mc() with
> > shorter one in order to avoid ugly line breaks.
> > And for better readability, we explicitly use MC_TARGET_* instead of
> > simply using integers.
> > 
> > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> 
> Thanks.
> 
> Seems ok to me.
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.futjisu.com>

Thank you.

> Hmm. some nitpicks.
> 
> > ---
> >  mm/memcontrol.c |   20 ++++++++++----------
> >  1 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git linux-next-20120307.orig/mm/memcontrol.c linux-next-20120307/mm/memcontrol.c
> > index a288855..3d16618 100644
> > --- linux-next-20120307.orig/mm/memcontrol.c
> > +++ linux-next-20120307/mm/memcontrol.c
> > @@ -5069,7 +5069,7 @@ one_by_one:
> >  }
> >  
> >  /**
> > - * is_target_pte_for_mc - check a pte whether it is valid for move charge
> > + * get_mctgt_type - get target type of moving charge
> >   * @vma: the vma the pte to be checked belongs
> >   * @addr: the address corresponding to the pte to be checked
> >   * @ptent: the pte to be checked
> > @@ -5092,7 +5092,7 @@ union mc_target {
> >  };
> >  
> >  enum mc_target_type {
> > -	MC_TARGET_NONE,	/* not used */
> > +	MC_TARGET_NONE, 
> 
> How about
> 
> 	MC_TARGET_NONE = 0,

Sounds good.

> Becasue you use 
> 	if (get_mctgt_type()) 
> later.

In previous discussion, I got the feedback to compare the returned value
of get_mctgt_type() with MC_TARGET_*.
If we obey the advice, if (get_mctgt_type() == MC_TARGET_NONE)" is better,
but here we define MC_TARGET_NONE as 0 explicitly, so I think it's OK to
use the short form you suggest.

> 
> >  	MC_TARGET_PAGE,
> >  	MC_TARGET_SWAP,
> >  };
> > @@ -5173,12 +5173,12 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
> >  	return page;
> >  }
> >  
> > -static int is_target_pte_for_mc(struct vm_area_struct *vma,
> > +static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
> >  		unsigned long addr, pte_t ptent, union mc_target *target)
> 
> I admit old name is too long. But...Hm...get_mctgt_type()...how about
> 
> 	move_charge_type() or
> 	mctgt_type() or
> 	mc_type() ?
> 
> I don't have good sense of naming ;(

Shorter one is generally good, but too short one doesn't show what it does.
The first one appears to say it moves something. Latter two looks unclear to
me because they don't contain any verbs. So I'm afraid I like the current one.

> >  {
> >  	struct page *page = NULL;
> >  	struct page_cgroup *pc;
> > -	int ret = 0;
> > +	enum mc_target_type ret = MC_TARGET_NONE;
> >  	swp_entry_t ent = { .val = 0 };
> >  
> >  	if (pte_present(ptent))
> > @@ -5189,7 +5189,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
> >  		page = mc_handle_file_pte(vma, addr, ptent, &ent);
> >  
> >  	if (!page && !ent.val)
> > -		return 0;
> > +		return ret;
> >  	if (page) {
> >  		pc = lookup_page_cgroup(page);
> >  		/*
> > @@ -5206,7 +5206,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
> >  			put_page(page);
> >  	}
> >  	/* There is a swap entry and a page doesn't exist or isn't charged */
> > -	if (ent.val && !ret &&
> > +	if (ent.val && ret != MC_TARGET_NONE &&
> 
> If you do MC_TARGET_NONE = 0 in above, using !ret seems ok to me.

OK. It's clearer.

> >  			css_id(&mc.from->css) == lookup_swap_cgroup_id(ent)) {
> >  		ret = MC_TARGET_SWAP;
> >  		if (target)
> > @@ -5227,7 +5227,7 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
> >  
> >  	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> >  	for (; addr != end; pte++, addr += PAGE_SIZE)
> > -		if (is_target_pte_for_mc(vma, addr, *pte, NULL))
> > +		if (get_mctgt_type(vma, addr, *pte, NULL))
> >  			mc.precharge++;	/* increment precharge temporarily */
> >  	pte_unmap_unlock(pte - 1, ptl);
> >  	cond_resched();
> > @@ -5397,8 +5397,8 @@ retry:
> >  		if (!mc.precharge)
> >  			break;
> >  
> > -		type = is_target_pte_for_mc(vma, addr, ptent, &target);
> > -		switch (type) {
> > +		target_type = get_mctgt_type(vma, addr, ptent, &target);
> > +		switch (target_type) {
> 
> It 'target_type' is unused later
>  
> 	switch(get_mctgt_type(vma, addr, ptent, &target))
> 
> is ok ?

Yes. That's OK.

So I attach the revised patch below.
The applied changes do not affect later patches.

Thanks,
Naoya Horiguchi
---

Re: [PATCH v4 1/3] memcg: clean up existing move charge code

[Hillf Danton]

On Wed, Mar 14, 2012 at 6:15 AM, Naoya Horiguchi
<n-horiguchi@ah.jp.nec.com> wrote:
> From c9bdd8f19040f3cea1c7d36e98b03ee13c1b8505 Mon Sep 17 00:00:00 2001
> From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Date: Tue, 13 Mar 2012 15:21:47 -0400
> Subject: [PATCH 1/3] memcg: clean up existing move charge code
>
> We'll introduce the thp variant of move charge code in later patches,
> but before doing that let's start with refactoring existing code.
> Here we replace lengthy function name is_target_pte_for_mc() with
> shorter one in order to avoid ugly line breaks.
> And for better readability, we explicitly use MC_TARGET_* instead of
> simply using integers.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.futjisu.com>

Acked-by: Hillf Danton <dhillf@gmail.com>

> ---
>  mm/memcontrol.c |   17 ++++++++---------
>  1 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a288855..508a7ed 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5069,7 +5069,7 @@ one_by_one:
>  }
>
>  /**
> - * is_target_pte_for_mc - check a pte whether it is valid for move charge
> + * get_mctgt_type - get target type of moving charge
>  * @vma: the vma the pte to be checked belongs
>  * @addr: the address corresponding to the pte to be checked
>  * @ptent: the pte to be checked
> @@ -5092,7 +5092,7 @@ union mc_target {
>  };
>
>  enum mc_target_type {
> -       MC_TARGET_NONE, /* not used */
> +       MC_TARGET_NONE = 0,
>        MC_TARGET_PAGE,
>        MC_TARGET_SWAP,
>  };
> @@ -5173,12 +5173,12 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
>        return page;
>  }
>
> -static int is_target_pte_for_mc(struct vm_area_struct *vma,
> +static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
>                unsigned long addr, pte_t ptent, union mc_target *target)
>  {
>        struct page *page = NULL;
>        struct page_cgroup *pc;
> -       int ret = 0;
> +       enum mc_target_type ret = MC_TARGET_NONE;
>        swp_entry_t ent = { .val = 0 };
>
>        if (pte_present(ptent))
> @@ -5189,7 +5189,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>                page = mc_handle_file_pte(vma, addr, ptent, &ent);
>
>        if (!page && !ent.val)
> -               return 0;
> +               return ret;
>        if (page) {
>                pc = lookup_page_cgroup(page);
>                /*
> @@ -5227,7 +5227,7 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
>
>        pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
>        for (; addr != end; pte++, addr += PAGE_SIZE)
> -               if (is_target_pte_for_mc(vma, addr, *pte, NULL))
> +               if (get_mctgt_type(vma, addr, *pte, NULL))
>                        mc.precharge++; /* increment precharge temporarily */
>        pte_unmap_unlock(pte - 1, ptl);
>        cond_resched();
> @@ -5397,8 +5397,7 @@ retry:
>                if (!mc.precharge)
>                        break;
>
> -               type = is_target_pte_for_mc(vma, addr, ptent, &target);
> -               switch (type) {
> +               switch (get_mctgt_type(vma, addr, ptent, &target)) {
>                case MC_TARGET_PAGE:
>                        page = target.page;
>                        if (isolate_lru_page(page))
> @@ -5411,7 +5410,7 @@ retry:
>                                mc.moved_charge++;
>                        }
>                        putback_lru_page(page);
> -put:                   /* is_target_pte_for_mc() gets the page */
> +put:                   /* get_mctgt_type() gets the page */
>                        put_page(page);
>                        break;
>                case MC_TARGET_SWAP:
> --
> 1.7.7.6
>

Re: [PATCH v4 1/3] memcg: clean up existing move charge code

[Hillf Danton]

On Wed, Mar 14, 2012 at 6:15 AM, Naoya Horiguchi
<n-horiguchi@ah.jp.nec.com> wrote:
> From c9bdd8f19040f3cea1c7d36e98b03ee13c1b8505 Mon Sep 17 00:00:00 2001
> From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Date: Tue, 13 Mar 2012 15:21:47 -0400
> Subject: [PATCH 1/3] memcg: clean up existing move charge code
>
> We'll introduce the thp variant of move charge code in later patches,
> but before doing that let's start with refactoring existing code.
> Here we replace lengthy function name is_target_pte_for_mc() with
> shorter one in order to avoid ugly line breaks.
> And for better readability, we explicitly use MC_TARGET_* instead of
> simply using integers.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.futjisu.com>

Acked-by: Hillf Danton <dhillf@gmail.com>

> ---
>  mm/memcontrol.c |   17 ++++++++---------
>  1 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a288855..508a7ed 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5069,7 +5069,7 @@ one_by_one:
>  }
>
>  /**
> - * is_target_pte_for_mc - check a pte whether it is valid for move charge
> + * get_mctgt_type - get target type of moving charge
>  * @vma: the vma the pte to be checked belongs
>  * @addr: the address corresponding to the pte to be checked
>  * @ptent: the pte to be checked
> @@ -5092,7 +5092,7 @@ union mc_target {
>  };
>
>  enum mc_target_type {
> -       MC_TARGET_NONE, /* not used */
> +       MC_TARGET_NONE = 0,
>        MC_TARGET_PAGE,
>        MC_TARGET_SWAP,
>  };
> @@ -5173,12 +5173,12 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
>        return page;
>  }
>
> -static int is_target_pte_for_mc(struct vm_area_struct *vma,
> +static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma,
>                unsigned long addr, pte_t ptent, union mc_target *target)
>  {
>        struct page *page = NULL;
>        struct page_cgroup *pc;
> -       int ret = 0;
> +       enum mc_target_type ret = MC_TARGET_NONE;
>        swp_entry_t ent = { .val = 0 };
>
>        if (pte_present(ptent))
> @@ -5189,7 +5189,7 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>                page = mc_handle_file_pte(vma, addr, ptent, &ent);
>
>        if (!page && !ent.val)
> -               return 0;
> +               return ret;
>        if (page) {
>                pc = lookup_page_cgroup(page);
>                /*
> @@ -5227,7 +5227,7 @@ static int mem_cgroup_count_precharge_pte_range(pmd_t *pmd,
>
>        pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
>        for (; addr != end; pte++, addr += PAGE_SIZE)
> -               if (is_target_pte_for_mc(vma, addr, *pte, NULL))
> +               if (get_mctgt_type(vma, addr, *pte, NULL))
>                        mc.precharge++; /* increment precharge temporarily */
>        pte_unmap_unlock(pte - 1, ptl);
>        cond_resched();
> @@ -5397,8 +5397,7 @@ retry:
>                if (!mc.precharge)
>                        break;
>
> -               type = is_target_pte_for_mc(vma, addr, ptent, &target);
> -               switch (type) {
> +               switch (get_mctgt_type(vma, addr, ptent, &target)) {
>                case MC_TARGET_PAGE:
>                        page = target.page;
>                        if (isolate_lru_page(page))
> @@ -5411,7 +5410,7 @@ retry:
>                                mc.moved_charge++;
>                        }
>                        putback_lru_page(page);
> -put:                   /* is_target_pte_for_mc() gets the page */
> +put:                   /* get_mctgt_type() gets the page */
>                        put_page(page);
>                        break;
>                case MC_TARGET_SWAP:
> --
> 1.7.7.6
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Paul Gortmaker]

On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
<n-horiguchi@ah.jp.nec.com> wrote:
> These macros will be used in later patch, where all usage are expected
> to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
> But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().

Just a heads up that this showed up in linux-next today as the
cause of a new build failure for an ARM board:

http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/

Paul.
--

git bisect start
# good: [fde7d9049e55ab85a390be7f415d74c9f62dd0f9] Linux 3.3-rc7
git bisect good fde7d9049e55ab85a390be7f415d74c9f62dd0f9
# bad: [9166d6581d1ca6795bcd38506c7d29bf39402a7d] Add linux-next
specific files for 20120321
git bisect bad 9166d6581d1ca6795bcd38506c7d29bf39402a7d
# good: [e863be5cac0a857302ccc0129326ca3634c1136a] Merge
remote-tracking branch 'net-next/master'
git bisect good e863be5cac0a857302ccc0129326ca3634c1136a
# good: [2ac4846c531fc33783999e8819a66d0f3e36058f] Merge
remote-tracking branch 'spi/spi/next'
git bisect good 2ac4846c531fc33783999e8819a66d0f3e36058f
# good: [a3ebef77540d27811aab7514b9aeaab13c573b46] Merge
remote-tracking branch 'oprofile/for-next'
git bisect good a3ebef77540d27811aab7514b9aeaab13c573b46
# good: [3ee174051be0246173ba8a386bb7a2d20d9e27c3] [arm-soc internal]
add back contents file
git bisect good 3ee174051be0246173ba8a386bb7a2d20d9e27c3
# good: [cc61a2762110efb0868bc326be52f3ecd22c4e99] Merge
remote-tracking branch 'dma-buf/for-next'
git bisect good cc61a2762110efb0868bc326be52f3ecd22c4e99
# bad: [dd09ea75ddb4d712abe5ec8a9b37619a99d4554c] kernel/watchdog.c:
add comment to watchdog() exit path
git bisect bad dd09ea75ddb4d712abe5ec8a9b37619a99d4554c
# good: [a80d01212507a57d48912f08ca22de039f4470b8] rmap: remove
__anon_vma_link() declaration
git bisect good a80d01212507a57d48912f08ca22de039f4470b8
# good: [dfac39f6a2daa212de0eb80bf14c76a2bee23dc4] memcg: remove
PCG_CACHE page_cgroup flag
git bisect good dfac39f6a2daa212de0eb80bf14c76a2bee23dc4
# bad: [8887892fd3354617e63be2399e286a86d0f9279c] alpha: use
set_current_blocked() and block_sigmask()
git bisect bad 8887892fd3354617e63be2399e286a86d0f9279c
# good: [0d7a67d6525414f8541f4a61084d783b4b53b8ec] memcg: remove
PCG_FILE_MAPPED fix cosmetic fix
git bisect good 0d7a67d6525414f8541f4a61084d783b4b53b8ec
# good: [ea20bf604adaab3a4ffb887083e62e7d76eb5d53] memcg: clean up
existing move charge code
git bisect good ea20bf604adaab3a4ffb887083e62e7d76eb5d53
# bad: [0709378dc1d716112e10de7f687af4993e69df7b] frv: use
set_current_blocked() and block_sigmask()
git bisect bad 0709378dc1d716112e10de7f687af4993e69df7b
# bad: [3135be0275c89f28c352554a0ec1874ea7cd3c3a] memcg: avoid THP
split in task migration
git bisect bad 3135be0275c89f28c352554a0ec1874ea7cd3c3a
# bad: [92c36300cf69f6ea1267d0bba7af708560c116d7] thp: add HPAGE_PMD_*
definitions for !CONFIG_TRANSPARENT_HUGEPAGE
git bisect bad 92c36300cf69f6ea1267d0bba7af708560c116d7
paul@yow-lpgnfs-02:~/git/linux-head$



>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Acked-by: Hillf Danton <dhillf@gmail.com>
> Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Acked-by: David Rientjes <rientjes@google.com>
> ---
>  include/linux/huge_mm.h |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git linux-next-20120307.orig/include/linux/huge_mm.h linux-next-20120307/include/linux/huge_mm.h
> index f56cacb..c8af7a2 100644
> --- linux-next-20120307.orig/include/linux/huge_mm.h
> +++ linux-next-20120307/include/linux/huge_mm.h
> @@ -51,6 +51,9 @@ extern pmd_t *page_check_address_pmd(struct page *page,
>                                     unsigned long address,
>                                     enum page_check_address_pmd_flag flag);
>
> +#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
> +#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
> +
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  #define HPAGE_PMD_SHIFT HPAGE_SHIFT
>  #define HPAGE_PMD_MASK HPAGE_MASK
> @@ -102,8 +105,6 @@ extern void __split_huge_page_pmd(struct mm_struct *mm, pmd_t *pmd);
>                BUG_ON(pmd_trans_splitting(*____pmd) ||                 \
>                       pmd_trans_huge(*____pmd));                       \
>        } while (0)
> -#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
> -#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
>  #if HPAGE_PMD_ORDER > MAX_ORDER
>  #error "hugepages can't be allocated by the buddy allocator"
>  #endif
> @@ -158,9 +159,9 @@ static inline struct page *compound_trans_head(struct page *page)
>        return page;
>  }
>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
> -#define HPAGE_PMD_SHIFT ({ BUG(); 0; })
> -#define HPAGE_PMD_MASK ({ BUG(); 0; })
> -#define HPAGE_PMD_SIZE ({ BUG(); 0; })
> +#define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
> +#define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
> +#define HPAGE_PMD_SIZE ({ BUILD_BUG(); 0; })
>
>  #define hpage_nr_pages(x) 1
>
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Paul Gortmaker]

On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
<n-horiguchi@ah.jp.nec.com> wrote:
> These macros will be used in later patch, where all usage are expected
> to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
> But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().

Just a heads up that this showed up in linux-next today as the
cause of a new build failure for an ARM board:

http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/

Paul.
--

git bisect start
# good: [fde7d9049e55ab85a390be7f415d74c9f62dd0f9] Linux 3.3-rc7
git bisect good fde7d9049e55ab85a390be7f415d74c9f62dd0f9
# bad: [9166d6581d1ca6795bcd38506c7d29bf39402a7d] Add linux-next
specific files for 20120321
git bisect bad 9166d6581d1ca6795bcd38506c7d29bf39402a7d
# good: [e863be5cac0a857302ccc0129326ca3634c1136a] Merge
remote-tracking branch 'net-next/master'
git bisect good e863be5cac0a857302ccc0129326ca3634c1136a
# good: [2ac4846c531fc33783999e8819a66d0f3e36058f] Merge
remote-tracking branch 'spi/spi/next'
git bisect good 2ac4846c531fc33783999e8819a66d0f3e36058f
# good: [a3ebef77540d27811aab7514b9aeaab13c573b46] Merge
remote-tracking branch 'oprofile/for-next'
git bisect good a3ebef77540d27811aab7514b9aeaab13c573b46
# good: [3ee174051be0246173ba8a386bb7a2d20d9e27c3] [arm-soc internal]
add back contents file
git bisect good 3ee174051be0246173ba8a386bb7a2d20d9e27c3
# good: [cc61a2762110efb0868bc326be52f3ecd22c4e99] Merge
remote-tracking branch 'dma-buf/for-next'
git bisect good cc61a2762110efb0868bc326be52f3ecd22c4e99
# bad: [dd09ea75ddb4d712abe5ec8a9b37619a99d4554c] kernel/watchdog.c:
add comment to watchdog() exit path
git bisect bad dd09ea75ddb4d712abe5ec8a9b37619a99d4554c
# good: [a80d01212507a57d48912f08ca22de039f4470b8] rmap: remove
__anon_vma_link() declaration
git bisect good a80d01212507a57d48912f08ca22de039f4470b8
# good: [dfac39f6a2daa212de0eb80bf14c76a2bee23dc4] memcg: remove
PCG_CACHE page_cgroup flag
git bisect good dfac39f6a2daa212de0eb80bf14c76a2bee23dc4
# bad: [8887892fd3354617e63be2399e286a86d0f9279c] alpha: use
set_current_blocked() and block_sigmask()
git bisect bad 8887892fd3354617e63be2399e286a86d0f9279c
# good: [0d7a67d6525414f8541f4a61084d783b4b53b8ec] memcg: remove
PCG_FILE_MAPPED fix cosmetic fix
git bisect good 0d7a67d6525414f8541f4a61084d783b4b53b8ec
# good: [ea20bf604adaab3a4ffb887083e62e7d76eb5d53] memcg: clean up
existing move charge code
git bisect good ea20bf604adaab3a4ffb887083e62e7d76eb5d53
# bad: [0709378dc1d716112e10de7f687af4993e69df7b] frv: use
set_current_blocked() and block_sigmask()
git bisect bad 0709378dc1d716112e10de7f687af4993e69df7b
# bad: [3135be0275c89f28c352554a0ec1874ea7cd3c3a] memcg: avoid THP
split in task migration
git bisect bad 3135be0275c89f28c352554a0ec1874ea7cd3c3a
# bad: [92c36300cf69f6ea1267d0bba7af708560c116d7] thp: add HPAGE_PMD_*
definitions for !CONFIG_TRANSPARENT_HUGEPAGE
git bisect bad 92c36300cf69f6ea1267d0bba7af708560c116d7
paul@yow-lpgnfs-02:~/git/linux-head$



>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Acked-by: Hillf Danton <dhillf@gmail.com>
> Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Acked-by: David Rientjes <rientjes@google.com>
> ---
>  include/linux/huge_mm.h |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git linux-next-20120307.orig/include/linux/huge_mm.h linux-next-20120307/include/linux/huge_mm.h
> index f56cacb..c8af7a2 100644
> --- linux-next-20120307.orig/include/linux/huge_mm.h
> +++ linux-next-20120307/include/linux/huge_mm.h
> @@ -51,6 +51,9 @@ extern pmd_t *page_check_address_pmd(struct page *page,
>                                     unsigned long address,
>                                     enum page_check_address_pmd_flag flag);
>
> +#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
> +#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
> +
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  #define HPAGE_PMD_SHIFT HPAGE_SHIFT
>  #define HPAGE_PMD_MASK HPAGE_MASK
> @@ -102,8 +105,6 @@ extern void __split_huge_page_pmd(struct mm_struct *mm, pmd_t *pmd);
>                BUG_ON(pmd_trans_splitting(*____pmd) ||                 \
>                       pmd_trans_huge(*____pmd));                       \
>        } while (0)
> -#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
> -#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
>  #if HPAGE_PMD_ORDER > MAX_ORDER
>  #error "hugepages can't be allocated by the buddy allocator"
>  #endif
> @@ -158,9 +159,9 @@ static inline struct page *compound_trans_head(struct page *page)
>        return page;
>  }
>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
> -#define HPAGE_PMD_SHIFT ({ BUG(); 0; })
> -#define HPAGE_PMD_MASK ({ BUG(); 0; })
> -#define HPAGE_PMD_SIZE ({ BUG(); 0; })
> +#define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
> +#define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
> +#define HPAGE_PMD_SIZE ({ BUILD_BUG(); 0; })
>
>  #define hpage_nr_pages(x) 1
>
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Andrew Morton]

On Wed, 21 Mar 2012 18:07:41 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
> <n-horiguchi@ah.jp.nec.com> wrote:
> > These macros will be used in later patch, where all usage are expected
> > to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
> > But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().
> 
> Just a heads up that this showed up in linux-next today as the
> cause of a new build failure for an ARM board:

Dammit.

> http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/

Site is dead.  What was failure, please?

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Andrew Morton]

On Wed, 21 Mar 2012 18:07:41 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
> <n-horiguchi@ah.jp.nec.com> wrote:
> > These macros will be used in later patch, where all usage are expected
> > to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
> > But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().
> 
> Just a heads up that this showed up in linux-next today as the
> cause of a new build failure for an ARM board:

Dammit.

> http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/

Site is dead.  What was failure, please?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Paul Gortmaker]

On 12-03-21 06:19 PM, Andrew Morton wrote:
> On Wed, 21 Mar 2012 18:07:41 -0400
> Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
>> On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
>> <n-horiguchi@ah.jp.nec.com> wrote:
>>> These macros will be used in later patch, where all usage are expected
>>> to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
>>> But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().
>>
>> Just a heads up that this showed up in linux-next today as the
>> cause of a new build failure for an ARM board:
> 
> Dammit.
> 
>> http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/
> 
> Site is dead.  What was failure, please?

Odd, I just reloaded the above link and it seems alive?
Anyway here is where it goes off the rails.

mm/pgtable-generic.c: In function 'pmdp_clear_flush_young':
mm/pgtable-generic.c:76:136: error: call to '__build_bug_failed' declared with attribute error: BUILD_BUG failed
make[2]: *** [mm/pgtable-generic.o] Error 1

Build was for ARM, tct_hammer_defconfig

Paul.

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Paul Gortmaker]

On 12-03-21 06:19 PM, Andrew Morton wrote:
> On Wed, 21 Mar 2012 18:07:41 -0400
> Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
>> On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
>> <n-horiguchi@ah.jp.nec.com> wrote:
>>> These macros will be used in later patch, where all usage are expected
>>> to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
>>> But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().
>>
>> Just a heads up that this showed up in linux-next today as the
>> cause of a new build failure for an ARM board:
> 
> Dammit.
> 
>> http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/
> 
> Site is dead.  What was failure, please?

Odd, I just reloaded the above link and it seems alive?
Anyway here is where it goes off the rails.

mm/pgtable-generic.c: In function 'pmdp_clear_flush_young':
mm/pgtable-generic.c:76:136: error: call to '__build_bug_failed' declared with attribute error: BUILD_BUG failed
make[2]: *** [mm/pgtable-generic.o] Error 1

Build was for ARM, tct_hammer_defconfig

Paul.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Andrew Morton]

On Wed, 21 Mar 2012 18:07:41 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
> <n-horiguchi@ah.jp.nec.com> wrote:
> > These macros will be used in later patch, where all usage are expected
> > to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
> > But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().
> 
> Just a heads up that this showed up in linux-next today as the
> cause of a new build failure for an ARM board:
> 
> http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/

The internet started working again.

mm/pgtable-generic.c: In function 'pmdp_clear_flush_young':
mm/pgtable-generic.c:76: error: call to '__build_bug_failed' declared with attribute error: BUILD_BUG failed

I guess we shouldn't be evaluating HPAGE_PMD_MASK at all if
!CONFIG_TRANSPARENT_HUGEPAGE, so...

--- a/mm/pgtable-generic.c~thp-add-hpage_pmd_-definitions-for-config_transparent_hugepage-fix
+++ a/mm/pgtable-generic.c
@@ -70,10 +70,11 @@ int pmdp_clear_flush_young(struct vm_are
 			   unsigned long address, pmd_t *pmdp)
 {
 	int young;
-#ifndef CONFIG_TRANSPARENT_HUGEPAGE
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
+#else
 	BUG();
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
-	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
 	young = pmdp_test_and_clear_young(vma, address, pmdp);
 	if (young)
 		flush_tlb_range(vma, address, address + HPAGE_PMD_SIZE);
_

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Andrew Morton]

On Wed, 21 Mar 2012 18:07:41 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
> <n-horiguchi@ah.jp.nec.com> wrote:
> > These macros will be used in later patch, where all usage are expected
> > to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
> > But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().
> 
> Just a heads up that this showed up in linux-next today as the
> cause of a new build failure for an ARM board:
> 
> http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/

The internet started working again.

mm/pgtable-generic.c: In function 'pmdp_clear_flush_young':
mm/pgtable-generic.c:76: error: call to '__build_bug_failed' declared with attribute error: BUILD_BUG failed

I guess we shouldn't be evaluating HPAGE_PMD_MASK at all if
!CONFIG_TRANSPARENT_HUGEPAGE, so...

--- a/mm/pgtable-generic.c~thp-add-hpage_pmd_-definitions-for-config_transparent_hugepage-fix
+++ a/mm/pgtable-generic.c
@@ -70,10 +70,11 @@ int pmdp_clear_flush_young(struct vm_are
 			   unsigned long address, pmd_t *pmdp)
 {
 	int young;
-#ifndef CONFIG_TRANSPARENT_HUGEPAGE
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
+#else
 	BUG();
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
-	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
 	young = pmdp_test_and_clear_young(vma, address, pmdp);
 	if (young)
 		flush_tlb_range(vma, address, address + HPAGE_PMD_SIZE);
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Andrea Arcangeli]

Hi,

On Wed, Mar 21, 2012 at 03:47:21PM -0700, Andrew Morton wrote:
> On Wed, 21 Mar 2012 18:07:41 -0400
> Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
> > On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
> > <n-horiguchi@ah.jp.nec.com> wrote:
> > > These macros will be used in later patch, where all usage are expected
> > > to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
> > > But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().
> > 
> > Just a heads up that this showed up in linux-next today as the
> > cause of a new build failure for an ARM board:
> > 
> > http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/
> 
> The internet started working again.
> 
> mm/pgtable-generic.c: In function 'pmdp_clear_flush_young':
> mm/pgtable-generic.c:76: error: call to '__build_bug_failed' declared with attribute error: BUILD_BUG failed
> 
> I guess we shouldn't be evaluating HPAGE_PMD_MASK at all if
> !CONFIG_TRANSPARENT_HUGEPAGE, so...

Yes. Either that or define __HAVE_ARCH_PMDP_CLEAR_YOUNG_FLUSH without
actually implementing the function to flush it away of the .text (is
it perhaps flushed away at vmlinux link time?). That
function never could be called by ARM. The BUG() is actually correct
even in the original position, just now it triggers at build time
because it doesn't know it can't be called.

> 
> --- a/mm/pgtable-generic.c~thp-add-hpage_pmd_-definitions-for-config_transparent_hugepage-fix
> +++ a/mm/pgtable-generic.c
> @@ -70,10 +70,11 @@ int pmdp_clear_flush_young(struct vm_are
>  			   unsigned long address, pmd_t *pmdp)
>  {
>  	int young;
> -#ifndef CONFIG_TRANSPARENT_HUGEPAGE
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
> +#else
>  	BUG();
>  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> -	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
>  	young = pmdp_test_and_clear_young(vma, address, pmdp);
>  	if (young)
>  		flush_tlb_range(vma, address, address + HPAGE_PMD_SIZE);
> _
> 

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[Andrea Arcangeli]

Hi,

On Wed, Mar 21, 2012 at 03:47:21PM -0700, Andrew Morton wrote:
> On Wed, 21 Mar 2012 18:07:41 -0400
> Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
> > On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
> > <n-horiguchi@ah.jp.nec.com> wrote:
> > > These macros will be used in later patch, where all usage are expected
> > > to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
> > > But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().
> > 
> > Just a heads up that this showed up in linux-next today as the
> > cause of a new build failure for an ARM board:
> > 
> > http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/
> 
> The internet started working again.
> 
> mm/pgtable-generic.c: In function 'pmdp_clear_flush_young':
> mm/pgtable-generic.c:76: error: call to '__build_bug_failed' declared with attribute error: BUILD_BUG failed
> 
> I guess we shouldn't be evaluating HPAGE_PMD_MASK at all if
> !CONFIG_TRANSPARENT_HUGEPAGE, so...

Yes. Either that or define __HAVE_ARCH_PMDP_CLEAR_YOUNG_FLUSH without
actually implementing the function to flush it away of the .text (is
it perhaps flushed away at vmlinux link time?). That
function never could be called by ARM. The BUG() is actually correct
even in the original position, just now it triggers at build time
because it doesn't know it can't be called.

> 
> --- a/mm/pgtable-generic.c~thp-add-hpage_pmd_-definitions-for-config_transparent_hugepage-fix
> +++ a/mm/pgtable-generic.c
> @@ -70,10 +70,11 @@ int pmdp_clear_flush_young(struct vm_are
>  			   unsigned long address, pmd_t *pmdp)
>  {
>  	int young;
> -#ifndef CONFIG_TRANSPARENT_HUGEPAGE
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
> +#else
>  	BUG();
>  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> -	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
>  	young = pmdp_test_and_clear_young(vma, address, pmdp);
>  	if (young)
>  		flush_tlb_range(vma, address, address + HPAGE_PMD_SIZE);
> _
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[David Daney]

On 03/21/2012 03:36 PM, Paul Gortmaker wrote:
> On 12-03-21 06:19 PM, Andrew Morton wrote:
>> On Wed, 21 Mar 2012 18:07:41 -0400
>> Paul Gortmaker<paul.gortmaker@windriver.com>  wrote:
>>
>>> On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
>>> <n-horiguchi@ah.jp.nec.com>  wrote:
>>>> These macros will be used in later patch, where all usage are expected
>>>> to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
>>>> But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().
>>>
>>> Just a heads up that this showed up in linux-next today as the
>>> cause of a new build failure for an ARM board:
>>
>> Dammit.
>>
>>> http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/
>>
>> Site is dead.  What was failure, please?
>
> Odd, I just reloaded the above link and it seems alive?
> Anyway here is where it goes off the rails.
>
> mm/pgtable-generic.c: In function 'pmdp_clear_flush_young':
> mm/pgtable-generic.c:76:136: error: call to '__build_bug_failed' declared with attribute error: BUILD_BUG failed
> make[2]: *** [mm/pgtable-generic.o] Error 1
>

This is just another instance of:

https://lkml.org/lkml/2011/12/16/507

There was some discussion in that thread of how it might be fixed.

David Daney


> Build was for ARM, tct_hammer_defconfig
>
> Paul.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH v4 2/3] thp: add HPAGE_PMD_* definitions for !CONFIG_TRANSPARENT_HUGEPAGE

[David Daney]

On 03/21/2012 03:36 PM, Paul Gortmaker wrote:
> On 12-03-21 06:19 PM, Andrew Morton wrote:
>> On Wed, 21 Mar 2012 18:07:41 -0400
>> Paul Gortmaker<paul.gortmaker@windriver.com>  wrote:
>>
>>> On Mon, Mar 12, 2012 at 6:30 PM, Naoya Horiguchi
>>> <n-horiguchi@ah.jp.nec.com>  wrote:
>>>> These macros will be used in later patch, where all usage are expected
>>>> to be optimized away without #ifdef CONFIG_TRANSPARENT_HUGEPAGE.
>>>> But to detect unexpected usages, we convert existing BUG() to BUILD_BUG().
>>>
>>> Just a heads up that this showed up in linux-next today as the
>>> cause of a new build failure for an ARM board:
>>
>> Dammit.
>>
>>> http://kisskb.ellerman.id.au/kisskb/buildresult/5930053/
>>
>> Site is dead.  What was failure, please?
>
> Odd, I just reloaded the above link and it seems alive?
> Anyway here is where it goes off the rails.
>
> mm/pgtable-generic.c: In function 'pmdp_clear_flush_young':
> mm/pgtable-generic.c:76:136: error: call to '__build_bug_failed' declared with attribute error: BUILD_BUG failed
> make[2]: *** [mm/pgtable-generic.o] Error 1
>

This is just another instance of:

https://lkml.org/lkml/2011/12/16/507

There was some discussion in that thread of how it might be fixed.

David Daney


> Build was for ARM, tct_hammer_defconfig
>
> Paul.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>