Re: [PATCH v5 06/11] thp: change split_huge_page_pmd() interface

David Rientjes
2012-11-14T23:22:06Z

On Wed, 7 Nov 2012, Kirill A. Shutemov wrote:

> diff --git a/Documentation/vm/transhuge.txt b/Documentation/vm/transhuge.txt
> index f734bb2..677a599 100644
> --- a/Documentation/vm/transhuge.txt
> +++ b/Documentation/vm/transhuge.txt
> @@ -276,7 +276,7 @@ unaffected. libhugetlbfs will also work fine as usual.
> == Graceful fallback ==
>
> Code walking pagetables but unware about huge pmds can simply call
> -split_huge_page_pmd(mm, pmd) where the pmd is the one returned by
> +split_huge_page_pmd(vma, pmd, addr) where the pmd is the one returned by
> pmd_offset. It's trivial to make the code transparent hugepage aware
> by just grepping for "pmd_offset" and adding split_huge_page_pmd where
> missing after pmd_offset returns the pmd. Thanks to the graceful
> @@ -299,7 +299,7 @@ diff --git a/mm/mremap.c b/mm/mremap.c
> return NULL;
>
> pmd = pmd_offset(pud, addr);
> -+ split_huge_page_pmd(mm, pmd);
> ++ split_huge_page_pmd(vma, pmd, addr);
> if (pmd_none_or_clear_bad(pmd))
> return NULL;
>
> diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
> index 5c9687b..1dfe69c 100644
> --- a/arch/x86/kernel/vm86_32.c
> +++ b/arch/x86/kernel/vm86_32.c
> @@ -182,7 +182,7 @@ static void mark_screen_rdonly(struct mm_struct *mm)
> if (pud_none_or_clear_bad(pud))
> goto out;
> pmd = pmd_offset(pud, 0xA0000);
> - split_huge_page_pmd(mm, pmd);
> + split_huge_page_pmd_mm(mm, 0xA0000, pmd);
> if (pmd_none_or_clear_bad(pmd))
> goto out;
> pte = pte_offset_map_lock(mm, pmd, 0xA0000, &ptl);

Why not be consistent and make this split_huge_page_pmd_mm(mm, pmd, addr)?

> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 90c63f9..291a0d1 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -643,7 +643,7 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned long addr,
> spinlock_t *ptl;
> struct page *page;
>
> - split_huge_page_pmd(walk->mm, pmd);
> + split_huge_page_pmd(vma, addr, pmd);

Ah, it's because the change to the documentation is wrong: the format is
actually split_huge_page_pmd(vma, addr, pmd).

> if (pmd_trans_unstable(pmd))
> return 0;
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index b31cb7d..856f080 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -91,12 +91,14 @@ extern int handle_pte_fault(struct mm_struct *mm,
> struct vm_area_struct *vma, unsigned long address,
> pte_t *pte, pmd_t *pmd, unsigned int flags);
> extern int split_huge_page(struct page *page);
> -extern void __split_huge_page_pmd(struct mm_struct *mm, pmd_t *pmd);
> -#define split_huge_page_pmd(__mm, __pmd) \
> +extern void __split_huge_page_pmd(struct vm_area_struct *vma,
> + unsigned long address, pmd_t *pmd);
> +#define split_huge_page_pmd(__vma, __address, __pmd) \
> do { \
> pmd_t *____pmd = (__pmd); \
> if (unlikely(pmd_trans_huge(*____pmd))) \
> - __split_huge_page_pmd(__mm, ____pmd); \
> + __split_huge_page_pmd(__vma, __address, \
> + ____pmd); \
> } while (0)
> #define wait_split_huge_page(__anon_vma, __pmd) \
> do { \
> @@ -106,6 +108,8 @@ 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)
> +extern void split_huge_page_pmd_mm(struct mm_struct *mm, unsigned long address,
> + pmd_t *pmd);
> #if HPAGE_PMD_ORDER > MAX_ORDER
> #error "hugepages can't be allocated by the buddy allocator"
> #endif
> @@ -173,10 +177,12 @@ static inline int split_huge_page(struct page *page)
> {
> return 0;
> }
> -#define split_huge_page_pmd(__mm, __pmd) \
> +#define split_huge_page_pmd(__vma, __address, __pmd) \
> do { } while (0)
> #define wait_split_huge_page(__anon_vma, __pmd) \
> do { } while (0)
> +#define split_huge_page_pmd_mm(__mm, __address, __pmd) \
> + do { } while (0)
> #define compound_trans_head(page) compound_head(page)
> static inline int hugepage_madvise(struct vm_area_struct *vma,
> unsigned long *vm_flags, int advice)
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 05490b3..90e651c 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2509,19 +2509,23 @@ static int khugepaged(void *none)
> return 0;
> }
>
> -void __split_huge_page_pmd(struct mm_struct *mm, pmd_t *pmd)
> +void __split_huge_page_pmd(struct vm_area_struct *vma, unsigned long address,
> + pmd_t *pmd)
> {
> struct page *page;
> + unsigned long haddr = address & HPAGE_PMD_MASK;
>

Just do

struct mm_struct *mm = vma->vm_mm;

here and it makes everything else simpler.

> - spin_lock(&mm->page_table_lock);
> + BUG_ON(vma->vm_start > haddr || vma->vm_end < haddr + HPAGE_PMD_SIZE);
> +
> + spin_lock(&vma->vm_mm->page_table_lock);
> if (unlikely(!pmd_trans_huge(*pmd))) {
> - spin_unlock(&mm->page_table_lock);
> + spin_unlock(&vma->vm_mm->page_table_lock);
> return;
> }
> page = pmd_page(*pmd);
> VM_BUG_ON(!page_count(page));
> get_page(page);
> - spin_unlock(&mm->page_table_lock);
> + spin_unlock(&vma->vm_mm->page_table_lock);
>
> split_huge_page(page);
>