arch, mm: introduce arch_tlb_gather_mmu_exit
Subsystems (10)
PATCH 1/1 arch, mm: introduce arch_tlb_gather_mmu_exit
From: Michal Hocko <[email protected]>
5a7862e83000 ("arm64: tlbflush: avoid flushing when fullmm == 1") has introduced an optimization to not flush tlb when we are tearing the whole address space down. Will goes on to explain
: Basically, we tag each address space with an ASID (PCID on x86) which : is resident in the TLB. This means we can elide TLB invalidation when : pulling down a full mm because we won't ever assign that ASID to : another mm without doing TLB invalidation elsewhere (which actually : just nukes the whole TLB).
This all is nice but tlb_gather users are not aware of that and this can actually cause some real problems. E.g. the oom_reaper tries to reap the whole address space but it might race with threads accessing the memory [1]. It is possible that soft-dirty handling might suffer from the same problem [2] as soon as it starts supporting the feature.
Introduce an explicit exit variant tlb_gather_mmu_exit which allows the behavior arm64 implements for the fullmm case and replace it by an explicit exit flag in the mmu_gather structure. exit_mmap path is then turned into the explicit exit variant. Other architectures simply ignore the flag.
Changes since RFC
- remove address range from tlb_gather_mmu_exit as it will always operate on the full range - as per Will
[1] http://lkml.kernel.org/r/[email protected]
[2] http://lkml.kernel.org/r/20171110001933.GA12421@bbox
---
Hi,
I have posted this as an RFC [1]. There was minor suggestion about the API by Will which is integrated into this patch. No other fundamental objections so I am asking for inclusion. I suspect that routing this via Andrew's tree is the easiest.
[1] http://lkml.kernel.org/r/[email protected]
arch/arm/include/asm/tlb.h | 3 ++- arch/arm64/include/asm/tlb.h | 2 +- arch/ia64/include/asm/tlb.h | 3 ++- arch/s390/include/asm/tlb.h | 3 ++- arch/sh/include/asm/tlb.h | 2 +- arch/um/include/asm/tlb.h | 2 +- include/asm-generic/tlb.h | 6 ++++-- include/linux/mm_types.h | 1 + mm/memory.c | 16 ++++++++++++++-- mm/mmap.c | 2 +- 10 files changed, 29 insertions(+), 11 deletions(-)
arch/arm/include/asm/tlb.h +2 -1
@@ -149,7 +149,8 @@ static inline void tlb_flush_mmu(struct mmu_gather *tlb)static inline void arch_tlb_gather_mmu (struct mmu_gather *tlb,struct mm_struct *mm, -unsigned long start,unsigned long end) +unsigned long start,unsigned long end, +bool exit) { tlb->mm = mm; tlb->fullmm = !(start | (end+1 ));
arch/arm64/include/asm/tlb.h +1 -1
@@ -43,7 +43,7 @@ static inline void tlb_flush(struct mmu_gather *tlb)* The ASID allocator will either invalidate the ASID or mark * it as used. */ -if (tlb->fullmm) +if (tlb->fullmm && tlb->exit)return ;/*
arch/ia64/include/asm/tlb.h +2 -1
@@ -170,7 +170,8 @@ static inline void __tlb_alloc_page(struct mmu_gather *tlb)static inline void arch_tlb_gather_mmu (struct mmu_gather *tlb,struct mm_struct *mm, -unsigned long start,unsigned long end) +unsigned long start,unsigned long end, +bool exit) { tlb->mm = mm; tlb->max =ARRAY_SIZE (tlb->local);
arch/s390/include/asm/tlb.h +2 -1
@@ -50,7 +50,8 @@ extern void tlb_remove_table(struct mmu_gather *tlb, void *table);static inline void arch_tlb_gather_mmu (struct mmu_gather *tlb,struct mm_struct *mm, -unsigned long start,unsigned long end) +unsigned long start,unsigned long end, +bool exit) { tlb->mm = mm; tlb->start = start;
arch/sh/include/asm/tlb.h +1 -1
@@ -38,7 +38,7 @@ static inline void init_tlb_gather(struct mmu_gather *tlb)static inline void arch_tlb_gather_mmu (struct mmu_gather *tlb,struct mm_struct *mm, -unsigned long start,unsigned long end) +unsigned long start,unsigned long end,bool exit) { tlb->mm = mm; tlb->start = start;
arch/um/include/asm/tlb.h +1 -1
@@ -47,7 +47,7 @@ static inline void init_tlb_gather(struct mmu_gather *tlb)static inline void arch_tlb_gather_mmu (struct mmu_gather *tlb,struct mm_struct *mm, -unsigned long start,unsigned long end) +unsigned long start,unsigned long end,bool exit) { tlb->mm = mm; tlb->start = start;
include/asm-generic/tlb.h +4 -2
@@ -101,7 +101,8 @@ struct mmu_gather { unsigned int fullmm : 1,/* we have performed an operation which * requires a complete flush of the tlb */ - need_flush_all : 1; + need_flush_all : 1, + exit : 1;struct mmu_gather_batch *active;struct mmu_gather_batch local;
@@ -113,7 +114,8 @@ struct mmu_gather {#define HAVE_GENERIC_MMU_GATHER void arch_tlb_gather_mmu (struct mmu_gather *tlb, -struct mm_struct *mm,unsigned long start,unsigned long end); +struct mm_struct *mm,unsigned long start,unsigned long end, +bool exit);void tlb_flush_mmu (struct mmu_gather *tlb);void arch_tlb_finish_mmu (struct mmu_gather *tlb,unsigned long start,unsigned long end,bool force);
include/linux/mm_types.h +1 -0
@@ -527,6 +527,7 @@ static inline cpumask_t *mm_cpumask(struct mm_struct *mm)struct mmu_gather ;extern void tlb_gather_mmu (struct mmu_gather *tlb,struct mm_struct *mm,unsigned long start,unsigned long end); +extern void tlb_gather_mmu_exit (struct mmu_gather *tlb,struct mm_struct *mm);extern void tlb_finish_mmu (struct mmu_gather *tlb,unsigned long start,unsigned long end);
mm/memory.c +14 -2
@@ -218,13 +218,15 @@ static bool tlb_next_batch(struct mmu_gather *tlb) }void arch_tlb_gather_mmu (struct mmu_gather *tlb,struct mm_struct *mm, -unsigned long start,unsigned long end) +unsigned long start,unsigned long end, +bool exit) { tlb->mm = mm;/* Is it from 0 to ~0? */ tlb->fullmm = !(start | (end+1 )); tlb->need_flush_all =0 ; + tlb->exit = exit; tlb->local.next =NULL ; tlb->local.nr =0 ; tlb->local.max =ARRAY_SIZE (tlb->__pages);
@@ -408,7 +410,17 @@ void tlb_remove_table(struct mmu_gather *tlb, void *table)void tlb_gather_mmu (struct mmu_gather *tlb,struct mm_struct *mm,unsigned long start,unsigned long end) { -arch_tlb_gather_mmu (tlb, mm, start, end); +arch_tlb_gather_mmu (tlb, mm, start, end, false); +inc_tlb_flush_pending (tlb->mm); +} + +/* tlb_gather_mmu_exit +* Basically same as tlb_gather_mmu except it allows architectures to +* skip tlb flushing if they can ensure that nobody will reuse tlb entries +*/ +void tlb_gather_mmu_exit (struct mmu_gather *tlb,struct mm_struct *mm) +{ +arch_tlb_gather_mmu (tlb, mm,0 ,-1 , true);inc_tlb_flush_pending (tlb->mm); }
mm/mmap.c +1 -1
@@ -2999,7 +2999,7 @@ void exit_mmap(struct mm_struct *mm)lru_add_drain ();flush_cache_mm (mm); -tlb_gather_mmu (&tlb, mm,0 ,-1 ); +tlb_gather_mmu_exit (&tlb, mm);/* 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 );
Signature · 7 lines
2.15.0 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 6 lines · Michal Hocko
This all is nice but
tlb_gatherusers are not aware of that and this can actually cause some real problems. E.g. theoom_reapertries to reap the whole address space but it might race with threads accessing the memory [1]. It is possible that soft-dirty handling might suffer from the same problem [2] as soon as it starts supporting the feature.
So we fixed the oom reaper to just do proper TLB invalidates in commit 687cb0884a71 ("mm, oom_reaper: gather each vma to prevent leaking TLB entry").
So now "fullmm" should be the expected "exit" case, and it all should be unambiguous.
Do we really have any reason to apply this patch any more?
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 8 lines · Linus Torvalds
Quoted 6 lines · Michal Hocko
This all is nice but
tlb_gatherusers are not aware of that and this can actually cause some real problems. E.g. theoom_reapertries to reap the whole address space but it might race with threads accessing the memory [1]. It is possible that soft-dirty handling might suffer from the same problem [2] as soon as it starts supporting the feature.
Quoted 8 lines
So we fixed the oom reaper to just do proper TLB invalidates in commit
687cb0884a71("mm,oom_reaper: gather each vma to prevent leaking TLB entry").So now "fullmm" should be the expected "exit" case, and it all should be unambiguous.
Do we really have any reason to apply this patch any more?
Well, the point was the clarity. The bad behavior came as a surprise for the oom reaper and as Minchan mentioned we would see a similar problem with soft-dirty bits as soon as they are supported on arm64 or potentially other architectures which might do special handling for exit case.
So strictly speaking, this doesn't fix any known bug to me. But I would find it more robust if the very special handling was explicit.
Signature · 8 lines
Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 16 lines · Michal Hocko
Quoted 15 lines · Linus Torvalds
Quoted 6 lines · Michal Hocko
This all is nice but
tlb_gatherusers are not aware of that and this can actually cause some real problems. E.g. theoom_reapertries to reap the whole address space but it might race with threads accessing the memory [1]. It is possible that soft-dirty handling might suffer from the same problem [2] as soon as it starts supporting the feature.So we fixed the oom reaper to just do proper TLB invalidates in commit
687cb0884a71("mm,oom_reaper: gather each vma to prevent leaking TLB entry").So now "fullmm" should be the expected "exit" case, and it all should be unambiguous.
Quoted 7 lines
Quoted 1 line
Do we really have any reason to apply this patch any more?
Well, the point was the clarity. The bad behavior came as a surprise for the oom reaper and as Minchan mentioned we would see a similar problem with soft-dirty bits as soon as they are supported on arm64 or potentially other architectures which might do special handling for exit case.
I am not going to push this patch if it is considered pointless but I haven't heard back anything to the above argument.
Signature · 8 lines
Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>