On Tue, Oct 25, 2011 at 10:59:31AM +0800, Shaohua Li wrote:
> Index: linux/arch/x86/include/asm/tlb.h
> ===================================================================
> --- linux.orig/arch/x86/include/asm/tlb.h 2011-10-25 09:00:39.000000000 +0800
> +++ linux/arch/x86/include/asm/tlb.h 2011-10-25 09:02:52.000000000 +0800
> @@ -4,6 +4,7 @@
> #define tlb_start_vma(tlb, vma) do { } while (0)
> #define tlb_end_vma(tlb, vma) do { } while (0)
> #define __tlb_remove_tlb_entry(tlb, ptep, address) do { } while (0)
> +#define __tlb_remove_pmd_tlb_entry(tlb, pmdp, address) do { } while (0)
> #define tlb_flush(tlb) flush_tlb_mm((tlb)->mm)
This is superfluous, it's already define below as noop.
>
> #include <asm-generic/tlb.h>
> Index: linux/include/asm-generic/tlb.h
> ===================================================================
> --- linux.orig/include/asm-generic/tlb.h 2011-10-25 09:00:23.000000000 +0800
> +++ linux/include/asm-generic/tlb.h 2011-10-25 09:18:01.000000000 +0800
> @@ -139,6 +139,16 @@ static inline void tlb_remove_page(struc
> __tlb_remove_tlb_entry(tlb, ptep, address); \
> } while (0)
>
> +#ifndef __tlb_remove_pmd_tlb_entry
> +#define __tlb_remove_pmd_tlb_entry(tlb, pmdp, address) do {} while(0)
> +#endif
> +
> +#define tlb_remove_pmd_tlb_entry(tlb, pmdp, address) \
> + do { \
> + tlb->need_flush = 1; \
> + __tlb_remove_pmd_tlb_entry(tlb, pmdp, address); \
> + } while (0)
this looks weird, why do we set need_flush = 1 again, considering that
we're doing tlb_remove_page() just a few lines later (which also sets
tlb->need_flush = 1).
Ok that other archs may need the __tlb_remove_pmd_tlb_entry to be
called (and I've no idea why), but the need_flush = 1 seems
unnecessary.
Why other archs need the __tlb_remove_pmd_tlb_entry to be called?
One way to go would be to change the tlb->need_flush = 1 in
__tlb_remove_page to a VM_BUG_ON(!tlb->need_flush) and then we keep it
above and we add the __tlb_remove_pmd_tlb_entry call.
Or is there any place where __tlb_remove_page is called without a
tlb_remove_*tlb_entry being called before it?
In any case the VM_BUG_ON will verify this.
> Index: linux/include/linux/huge_mm.h
> ===================================================================
> --- linux.orig/include/linux/huge_mm.h 2011-10-25 09:07:12.000000000 +0800
> +++ linux/include/linux/huge_mm.h 2011-10-25 09:07:44.000000000 +0800
> @@ -18,7 +18,7 @@ extern struct page *follow_trans_huge_pm
> unsigned int flags);
> extern int zap_huge_pmd(struct mmu_gather *tlb,
> struct vm_area_struct *vma,
> - pmd_t *pmd);
> + pmd_t *pmd, unsigned long addr);
> extern int mincore_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
> unsigned long addr, unsigned long end,
> unsigned char *vec);
> Index: linux/mm/huge_memory.c
> ===================================================================
> --- linux.orig/mm/huge_memory.c 2011-10-25 09:00:07.000000000 +0800
> +++ linux/mm/huge_memory.c 2011-10-25 09:06:55.000000000 +0800
> @@ -1005,7 +1005,7 @@ out:
> }
>
> int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> - pmd_t *pmd)
> + pmd_t *pmd, unsigned long addr)
> {
> int ret = 0;
>
> @@ -1021,6 +1021,7 @@ int zap_huge_pmd(struct mmu_gather *tlb,
> pgtable = get_pmd_huge_pte(tlb->mm);
> page = pmd_page(*pmd);
> pmd_clear(pmd);
> + tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> page_remove_rmap(page);
> VM_BUG_ON(page_mapcount(page) < 0);
> add_mm_counter(tlb->mm, MM_ANONPAGES, -HPAGE_PMD_NR);
> Index: linux/mm/memory.c
> ===================================================================
> --- linux.orig/mm/memory.c 2011-10-25 09:07:49.000000000 +0800
> +++ linux/mm/memory.c 2011-10-25 09:08:29.000000000 +0800
> @@ -1231,7 +1231,7 @@ static inline unsigned long zap_pmd_rang
> if (next-addr != HPAGE_PMD_SIZE) {
> VM_BUG_ON(!rwsem_is_locked(&tlb->mm->mmap_sem));
> split_huge_page_pmd(vma->vm_mm, pmd);
> - } else if (zap_huge_pmd(tlb, vma, pmd))
> + } else if (zap_huge_pmd(tlb, vma, pmd, addr))
> continue;
> /* fall through */
> }
Rest looks ok.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. 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@xxxxxxxxx";> email@xxxxxxxxx </a>