| Message ID | 20211008204825.6229-2-svens@stackframe.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | parisc: fixes for CONFIG_PREEMPT | expand |
Sven Schnelle <svens@stackframe.org> writes: > flush_cache_mm() fetches %sr3 via mtsp(3). If it matches mm->context, > it flushes caches and the TLB. However, the TLB is cpu-local, so if the > code gets preempted shortly after the mtsp(), and later resumed on another > CPU, the wrong TLB is flushed. > > Signed-off-by: Sven Schnelle <svens@stackframe.org> > --- > arch/parisc/kernel/cache.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/parisc/kernel/cache.c b/arch/parisc/kernel/cache.c > index 39e02227e231..90656c49bc07 100644 > --- a/arch/parisc/kernel/cache.c > +++ b/arch/parisc/kernel/cache.c > @@ -558,6 +558,7 @@ void flush_cache_mm(struct mm_struct *mm) > return; > } > > + preempt_disable(); > if (mm->context == mfsp(3)) { > for (vma = mm->mmap; vma; vma = vma->vm_next) { > flush_user_dcache_range_asm(vma->vm_start, vma->vm_end); > @@ -565,8 +566,10 @@ void flush_cache_mm(struct mm_struct *mm) > flush_user_icache_range_asm(vma->vm_start, vma->vm_end); > flush_tlb_range(vma, vma->vm_start, vma->vm_end); > } > + preempt_enable(); > return; > } > + preempt_enable(); > > pgd = mm->pgd; > for (vma = mm->mmap; vma; vma = vma->vm_next) { I noticed that flush_cache_range() has the same problem. Helge, let me know whether i should send a v2 with an additional patch, or a single follow up patch. Both functions also look very similar, so i think best would be to combine the code of these two functions.
On 10/8/21 23:35, Sven Schnelle wrote: > Sven Schnelle <svens@stackframe.org> writes: > >> flush_cache_mm() fetches %sr3 via mtsp(3). If it matches mm->context, >> it flushes caches and the TLB. However, the TLB is cpu-local, so if the >> code gets preempted shortly after the mtsp(), and later resumed on another >> CPU, the wrong TLB is flushed. >> >> Signed-off-by: Sven Schnelle <svens@stackframe.org> >> --- >> arch/parisc/kernel/cache.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/parisc/kernel/cache.c b/arch/parisc/kernel/cache.c >> index 39e02227e231..90656c49bc07 100644 >> --- a/arch/parisc/kernel/cache.c >> +++ b/arch/parisc/kernel/cache.c >> @@ -558,6 +558,7 @@ void flush_cache_mm(struct mm_struct *mm) >> return; >> } >> >> + preempt_disable(); >> if (mm->context == mfsp(3)) { >> for (vma = mm->mmap; vma; vma = vma->vm_next) { >> flush_user_dcache_range_asm(vma->vm_start, vma->vm_end); >> @@ -565,8 +566,10 @@ void flush_cache_mm(struct mm_struct *mm) >> flush_user_icache_range_asm(vma->vm_start, vma->vm_end); >> flush_tlb_range(vma, vma->vm_start, vma->vm_end); >> } >> + preempt_enable(); >> return; >> } >> + preempt_enable(); >> >> pgd = mm->pgd; >> for (vma = mm->mmap; vma; vma = vma->vm_next) { > > I noticed that flush_cache_range() has the same problem. Helge, let me > know whether i should send a v2 with an additional patch, or a single > follow up patch. Both functions also look very similar, so i think > best would be to combine the code of these two functions. I'm fine with either option. Thanks for your patches! Helge
diff --git a/arch/parisc/kernel/cache.c b/arch/parisc/kernel/cache.c index 39e02227e231..90656c49bc07 100644 --- a/arch/parisc/kernel/cache.c +++ b/arch/parisc/kernel/cache.c @@ -558,6 +558,7 @@ void flush_cache_mm(struct mm_struct *mm) return; } + preempt_disable(); if (mm->context == mfsp(3)) { for (vma = mm->mmap; vma; vma = vma->vm_next) { flush_user_dcache_range_asm(vma->vm_start, vma->vm_end); @@ -565,8 +566,10 @@ void flush_cache_mm(struct mm_struct *mm) flush_user_icache_range_asm(vma->vm_start, vma->vm_end); flush_tlb_range(vma, vma->vm_start, vma->vm_end); } + preempt_enable(); return; } + preempt_enable(); pgd = mm->pgd; for (vma = mm->mmap; vma; vma = vma->vm_next) {
flush_cache_mm() fetches %sr3 via mtsp(3). If it matches mm->context, it flushes caches and the TLB. However, the TLB is cpu-local, so if the code gets preempted shortly after the mtsp(), and later resumed on another CPU, the wrong TLB is flushed. Signed-off-by: Sven Schnelle <svens@stackframe.org> --- arch/parisc/kernel/cache.c | 3 +++ 1 file changed, 3 insertions(+)