]> Pileus Git - ~andy/linux/commitdiff
Fix TLB gather virtual address range invalidation corner cases
authorLinus Torvalds <torvalds@linux-foundation.org>
Thu, 15 Aug 2013 18:42:25 +0000 (11:42 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Fri, 16 Aug 2013 15:52:46 +0000 (08:52 -0700)
Ben Tebulin reported:

 "Since v3.7.2 on two independent machines a very specific Git
  repository fails in 9/10 cases on git-fsck due to an SHA1/memory
  failures.  This only occurs on a very specific repository and can be
  reproduced stably on two independent laptops.  Git mailing list ran
  out of ideas and for me this looks like some very exotic kernel issue"

and bisected the failure to the backport of commit 53a59fc67f97 ("mm:
limit mmu_gather batching to fix soft lockups on !CONFIG_PREEMPT").

That commit itself is not actually buggy, but what it does is to make it
much more likely to hit the partial TLB invalidation case, since it
introduces a new case in tlb_next_batch() that previously only ever
happened when running out of memory.

The real bug is that the TLB gather virtual memory range setup is subtly
buggered.  It was introduced in commit 597e1c3580b7 ("mm/mmu_gather:
enable tlb flush range in generic mmu_gather"), and the range handling
was already fixed at least once in commit e6c495a96ce0 ("mm: fix the TLB
range flushed when __tlb_remove_page() runs out of slots"), but that fix
was not complete.

The problem with the TLB gather virtual address range is that it isn't
set up by the initial tlb_gather_mmu() initialization (which didn't get
the TLB range information), but it is set up ad-hoc later by the
functions that actually flush the TLB.  And so any such case that forgot
to update the TLB range entries would potentially miss TLB invalidates.

Rather than try to figure out exactly which particular ad-hoc range
setup was missing (I personally suspect it's the hugetlb case in
zap_huge_pmd(), which didn't have the same logic as zap_pte_range()
did), this patch just gets rid of the problem at the source: make the
TLB range information available to tlb_gather_mmu(), and initialize it
when initializing all the other tlb gather fields.

This makes the patch larger, but conceptually much simpler.  And the end
result is much more understandable; even if you want to play games with
partial ranges when invalidating the TLB contents in chunks, now the
range information is always there, and anybody who doesn't want to
bother with it won't introduce subtle bugs.

Ben verified that this fixes his problem.

Reported-bisected-and-tested-by: Ben Tebulin <tebulin@googlemail.com>
Build-testing-by: Stephen Rothwell <sfr@canb.auug.org.au>
Build-testing-by: Richard Weinberger <richard.weinberger@gmail.com>
Reviewed-by: Michal Hocko <mhocko@suse.cz>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Cc: stable@vger.kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
arch/arm/include/asm/tlb.h
arch/arm64/include/asm/tlb.h
arch/ia64/include/asm/tlb.h
arch/s390/include/asm/tlb.h
arch/sh/include/asm/tlb.h
arch/um/include/asm/tlb.h
fs/exec.c
include/asm-generic/tlb.h
mm/hugetlb.c
mm/memory.c
mm/mmap.c

index 46e7cfb3e7219d2ac4db9013d5dc673c155167ed..0baf7f0d939484264b089c772112657cb9f15c75 100644 (file)
@@ -43,6 +43,7 @@ struct mmu_gather {
        struct mm_struct        *mm;
        unsigned int            fullmm;
        struct vm_area_struct   *vma;
+       unsigned long           start, end;
        unsigned long           range_start;
        unsigned long           range_end;
        unsigned int            nr;
@@ -107,10 +108,12 @@ static inline void tlb_flush_mmu(struct mmu_gather *tlb)
 }
 
 static inline void
-tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned int fullmm)
+tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned long start, unsigned long end)
 {
        tlb->mm = mm;
-       tlb->fullmm = fullmm;
+       tlb->fullmm = !(start | (end+1));
+       tlb->start = start;
+       tlb->end = end;
        tlb->vma = NULL;
        tlb->max = ARRAY_SIZE(tlb->local);
        tlb->pages = tlb->local;
index 46b3beb4b773c85cd381f72301b8f2e9dfdc7a97..717031a762c27966aabc7786f6d2a900034b0b08 100644 (file)
@@ -35,6 +35,7 @@ struct mmu_gather {
        struct mm_struct        *mm;
        unsigned int            fullmm;
        struct vm_area_struct   *vma;
+       unsigned long           start, end;
        unsigned long           range_start;
        unsigned long           range_end;
        unsigned int            nr;
@@ -97,10 +98,12 @@ static inline void tlb_flush_mmu(struct mmu_gather *tlb)
 }
 
 static inline void
-tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned int fullmm)
+tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned long start, unsigned long end)
 {
        tlb->mm = mm;
-       tlb->fullmm = fullmm;
+       tlb->fullmm = !(start | (end+1));
+       tlb->start = start;
+       tlb->end = end;
        tlb->vma = NULL;
        tlb->max = ARRAY_SIZE(tlb->local);
        tlb->pages = tlb->local;
index ef3a9de01954511a352fa5b24285136789425e21..bc5efc7c3f3f8ead3608780ba5e2f5b9e212e20c 100644 (file)
@@ -22,7 +22,7 @@
  * unmapping a portion of the virtual address space, these hooks are called according to
  * the following template:
  *
- *     tlb <- tlb_gather_mmu(mm, full_mm_flush);       // start unmap for address space MM
+ *     tlb <- tlb_gather_mmu(mm, start, end);          // start unmap for address space MM
  *     {
  *       for each vma that needs a shootdown do {
  *         tlb_start_vma(tlb, vma);
@@ -58,6 +58,7 @@ struct mmu_gather {
        unsigned int            max;
        unsigned char           fullmm;         /* non-zero means full mm flush */
        unsigned char           need_flush;     /* really unmapped some PTEs? */
+       unsigned long           start, end;
        unsigned long           start_addr;
        unsigned long           end_addr;
        struct page             **pages;
@@ -155,13 +156,15 @@ static inline void __tlb_alloc_page(struct mmu_gather *tlb)
 
 
 static inline void
-tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned int full_mm_flush)
+tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned long start, unsigned long end)
 {
        tlb->mm = mm;
        tlb->max = ARRAY_SIZE(tlb->local);
        tlb->pages = tlb->local;
        tlb->nr = 0;
-       tlb->fullmm = full_mm_flush;
+       tlb->fullmm = !(start | (end+1));
+       tlb->start = start;
+       tlb->end = end;
        tlb->start_addr = ~0UL;
 }
 
index b75d7d686684975278e1881a9aaf74590a6949ed..23a64d25f2b1fc441689ea089d0a13cad3e4dde9 100644 (file)
@@ -32,6 +32,7 @@ struct mmu_gather {
        struct mm_struct *mm;
        struct mmu_table_batch *batch;
        unsigned int fullmm;
+       unsigned long start, unsigned long end;
 };
 
 struct mmu_table_batch {
@@ -48,10 +49,13 @@ extern void tlb_remove_table(struct mmu_gather *tlb, void *table);
 
 static inline void tlb_gather_mmu(struct mmu_gather *tlb,
                                  struct mm_struct *mm,
-                                 unsigned int full_mm_flush)
+                                 unsigned long start,
+                                 unsigned long end)
 {
        tlb->mm = mm;
-       tlb->fullmm = full_mm_flush;
+       tlb->start = start;
+       tlb->end = end;
+       tlb->fullmm = !(start | (end+1));
        tlb->batch = NULL;
        if (tlb->fullmm)
                __tlb_flush_mm(mm);
index e61d43d9f689c0ef61bc0e1e5e43b3826b66a760..362192ed12fef1789d2c23d21c654f19632006a0 100644 (file)
@@ -36,10 +36,12 @@ static inline void init_tlb_gather(struct mmu_gather *tlb)
 }
 
 static inline void
-tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned int full_mm_flush)
+tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned long start, unsigned long end)
 {
        tlb->mm = mm;
-       tlb->fullmm = full_mm_flush;
+       tlb->start = start;
+       tlb->end = end;
+       tlb->fullmm = !(start | (end+1));
 
        init_tlb_gather(tlb);
 }
index 4febacd1a8a1a3dee2c24e3b3447081e7ca94ebb..29b0301c18aab26f2a613d397da9856a8d1eec3b 100644 (file)
@@ -45,10 +45,12 @@ static inline void init_tlb_gather(struct mmu_gather *tlb)
 }
 
 static inline void
-tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned int full_mm_flush)
+tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned long start, unsigned long end)
 {
        tlb->mm = mm;
-       tlb->fullmm = full_mm_flush;
+       tlb->start = start;
+       tlb->end = end;
+       tlb->fullmm = !(start | (end+1));
 
        init_tlb_gather(tlb);
 }
index 9c73def87642fc8f36624b7fa525504541a59a04..fd774c7cb4831be8817799ed6cab355a368fa19f 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -608,7 +608,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, unsigned long shift)
                return -ENOMEM;
 
        lru_add_drain();
-       tlb_gather_mmu(&tlb, mm, 0);
+       tlb_gather_mmu(&tlb, mm, old_start, old_end);
        if (new_end > old_start) {
                /*
                 * when the old and new regions overlap clear from new_end.
@@ -625,7 +625,7 @@ static int shift_arg_pages(struct vm_area_struct *vma, unsigned long shift)
                free_pgd_range(&tlb, old_start, old_end, new_end,
                        vma->vm_next ? vma->vm_next->vm_start : USER_PGTABLES_CEILING);
        }
-       tlb_finish_mmu(&tlb, new_end, old_end);
+       tlb_finish_mmu(&tlb, old_start, old_end);
 
        /*
         * Shrink the vma to just the new range.  Always succeeds.
index 13821c339a4151912b88f2e4de04cbf99653a612..5672d7ea1fa066175b33c8f4c19457fcdc0614ed 100644 (file)
@@ -112,7 +112,7 @@ struct mmu_gather {
 
 #define HAVE_GENERIC_MMU_GATHER
 
-void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, bool fullmm);
+void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned long start, unsigned long end);
 void tlb_flush_mmu(struct mmu_gather *tlb);
 void tlb_finish_mmu(struct mmu_gather *tlb, unsigned long start,
                                                        unsigned long end);
index 83aff0a4d0938e308619a703dfca17265419e628..b60f33080a28f0a078f8186e65ab8d878fced928 100644 (file)
@@ -2490,7 +2490,7 @@ void unmap_hugepage_range(struct vm_area_struct *vma, unsigned long start,
 
        mm = vma->vm_mm;
 
-       tlb_gather_mmu(&tlb, mm, 0);
+       tlb_gather_mmu(&tlb, mm, start, end);
        __unmap_hugepage_range(&tlb, vma, start, end, ref_page);
        tlb_finish_mmu(&tlb, start, end);
 }
index 40268410732a1a954e79310e251d1100a38b18cb..af84bc0ec17c213054b023815ae197fefde7ffb6 100644 (file)
@@ -209,14 +209,15 @@ static int tlb_next_batch(struct mmu_gather *tlb)
  *     tear-down from @mm. The @fullmm argument is used when @mm is without
  *     users and we're going to destroy the full address space (exit/execve).
  */
-void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, bool fullmm)
+void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned long start, unsigned long end)
 {
        tlb->mm = mm;
 
-       tlb->fullmm     = fullmm;
+       /* Is it from 0 to ~0? */
+       tlb->fullmm     = !(start | (end+1));
        tlb->need_flush_all = 0;
-       tlb->start      = -1UL;
-       tlb->end        = 0;
+       tlb->start      = start;
+       tlb->end        = end;
        tlb->need_flush = 0;
        tlb->local.next = NULL;
        tlb->local.nr   = 0;
@@ -256,8 +257,6 @@ void tlb_finish_mmu(struct mmu_gather *tlb, unsigned long start, unsigned long e
 {
        struct mmu_gather_batch *batch, *next;
 
-       tlb->start = start;
-       tlb->end   = end;
        tlb_flush_mmu(tlb);
 
        /* keep the page table cache within bounds */
@@ -1099,7 +1098,6 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
        spinlock_t *ptl;
        pte_t *start_pte;
        pte_t *pte;
-       unsigned long range_start = addr;
 
 again:
        init_rss_vec(rss);
@@ -1205,17 +1203,25 @@ again:
         * and page-free while holding it.
         */
        if (force_flush) {
+               unsigned long old_end;
+
                force_flush = 0;
 
-#ifdef HAVE_GENERIC_MMU_GATHER
-               tlb->start = range_start;
+               /*
+                * Flush the TLB just for the previous segment,
+                * then update the range to be the remaining
+                * TLB range.
+                */
+               old_end = tlb->end;
                tlb->end = addr;
-#endif
+
                tlb_flush_mmu(tlb);
-               if (addr != end) {
-                       range_start = addr;
+
+               tlb->start = addr;
+               tlb->end = old_end;
+
+               if (addr != end)
                        goto again;
-               }
        }
 
        return addr;
@@ -1400,7 +1406,7 @@ void zap_page_range(struct vm_area_struct *vma, unsigned long start,
        unsigned long end = start + size;
 
        lru_add_drain();
-       tlb_gather_mmu(&tlb, mm, 0);
+       tlb_gather_mmu(&tlb, mm, start, end);
        update_hiwater_rss(mm);
        mmu_notifier_invalidate_range_start(mm, start, end);
        for ( ; vma && vma->vm_start < end; vma = vma->vm_next)
@@ -1426,7 +1432,7 @@ static void zap_page_range_single(struct vm_area_struct *vma, unsigned long addr
        unsigned long end = address + size;
 
        lru_add_drain();
-       tlb_gather_mmu(&tlb, mm, 0);
+       tlb_gather_mmu(&tlb, mm, address, end);
        update_hiwater_rss(mm);
        mmu_notifier_invalidate_range_start(mm, address, end);
        unmap_single_vma(&tlb, vma, address, end, details);
index 1edbaa3136c3c464cf648e569f2f98499f00642c..f9c97d10b873ae4a896c3a22266707653d941c90 100644 (file)
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2336,7 +2336,7 @@ static void unmap_region(struct mm_struct *mm,
        struct mmu_gather tlb;
 
        lru_add_drain();
-       tlb_gather_mmu(&tlb, mm, 0);
+       tlb_gather_mmu(&tlb, mm, start, end);
        update_hiwater_rss(mm);
        unmap_vmas(&tlb, vma, start, end);
        free_pgtables(&tlb, vma, prev ? prev->vm_end : FIRST_USER_ADDRESS,
@@ -2709,7 +2709,7 @@ void exit_mmap(struct mm_struct *mm)
 
        lru_add_drain();
        flush_cache_mm(mm);
-       tlb_gather_mmu(&tlb, mm, 1);
+       tlb_gather_mmu(&tlb, mm, 0, -1);
        /* update_hiwater_rss(mm) here? but nobody should be looking */
        /* Use -1 here to ensure all VMAs in the mm are unmapped */
        unmap_vmas(&tlb, vma, 0, -1);