On 06/15/2009 01:20 AM, John David Anglin wrote:
I am convinced that interrupts need to be disabled on SMP kernels to
prevent deadlock. On UP kernels, I am not convinced that anything bad
happens if we do a tlb purge while handling an interrupt since we don't
have to worry about preventing bus conflicts. The UP code can't
deadlock. I'm thinking that we can stay with disabling preemption.
Dave is right and it took me long to understand his point...
I continued testing and we can simply just disable preemption for UP
kernels and use an irq-safe spinlock for SMP kernels.
Below is the latest and greatest patch which fixes this bug.
Run-tested onUP kernels and compile-tested on SMP kernels.
Kyle, it would be nice if you could apply it as soon as possible.
The bug is existing since a long time, so it might make sense to backport
it to older kernels as well...
-----------
Patch: [parisc] fix system hangs due to unsafe locking of the TLB-protection spinlock
Importance: high, fixes complete kernel stalls
The TLB flushing functions on hppa, which causes PxTLB broadcasts on the system
bus, needs to be protected by irq-safe spinlocks to avoid irq handlers to deadlock
the kernel. The deadlocks only happened during I/O intensive loads and triggered
pretty seldom, which is why this bug went so long unnoticed.
Fix this bug by using spin_lock_irqsafe() for SMP kernels and preempt_disable()
for UP kernels.
Signed-off-by: Helge Deller <deller@xxxxxx>
diff --git a/arch/parisc/include/asm/tlbflush.h b/arch/parisc/include/asm/tlbflush.h
index 1f6fd4f..3d82fc8 100644
--- a/arch/parisc/include/asm/tlbflush.h
+++ b/arch/parisc/include/asm/tlbflush.h
@@ -10,16 +10,21 @@
/* This is for the serialisation of PxTLB broadcasts. At least on the
* N class systems, only one PxTLB inter processor broadcast can be
- * active at any one time on the Merced bus. This tlb purge
- * synchronisation is fairly lightweight and harmless so we activate
- * it on all SMP systems not just the N class. We also need to have
- * preemption disabled on uniprocessor machines, and spin_lock does that
- * nicely.
+ * active at any one time on the Merced bus. To be on the safe side, we
+ * unconditionally protect those broadcasts for all SMP kernels with a
+ * spinlock.
+ * On uniprocessor machines we save this overhead by just disabling
+ * preemption.
*/
-extern spinlock_t pa_tlb_lock;
-#define purge_tlb_start(x) spin_lock(&pa_tlb_lock)
-#define purge_tlb_end(x) spin_unlock(&pa_tlb_lock)
+#ifdef CONFIG_SMP
+extern spinlock_t pa_tlb_lock;
+# define purge_tlb_start(flags) spin_lock_irqsave(&pa_tlb_lock, flags)
+# define purge_tlb_end(flags) spin_unlock_irqrestore(&pa_tlb_lock, flags)
+#else
+# define purge_tlb_start(flags) preempt_disable()
+# define purge_tlb_end(flags) preempt_enable()
+#endif
extern void flush_tlb_all(void);
extern void flush_tlb_all_local(void *);
@@ -64,13 +69,14 @@ static inline void flush_tlb_page(struct vm_area_struct *vma,
unsigned long addr)
{
/* For one page, it's not worth testing the split_tlb variable */
+ unsigned long flags __maybe_unused;
mb();
mtsp(vma->vm_mm->context,1);
- purge_tlb_start();
+ purge_tlb_start(flags);
pdtlb(addr);
pitlb(addr);
- purge_tlb_end();
+ purge_tlb_end(flags);
}
void __flush_tlb_range(unsigned long sid,
diff --git a/arch/parisc/kernel/cache.c b/arch/parisc/kernel/cache.c
index 837530e..9c85e4f 100644
--- a/arch/parisc/kernel/cache.c
+++ b/arch/parisc/kernel/cache.c
@@ -35,12 +35,14 @@ int icache_stride __read_mostly;
EXPORT_SYMBOL(dcache_stride);
+#ifdef CONFIG_SMP
/* On some machines (e.g. ones with the Merced bus), there can be
* only a single PxTLB broadcast at a time; this must be guaranteed
- * by software. We put a spinlock around all TLB flushes to
- * ensure this.
+ * by software. We put a spinlock around all TLB flushes on SMP
+ * kernels to ensure this.
*/
DEFINE_SPINLOCK(pa_tlb_lock);
+#endif
struct pdc_cache_info cache_info __read_mostly;
#ifndef CONFIG_PA20
@@ -400,10 +402,11 @@ void clear_user_page_asm(void *page, unsigned long vaddr)
{
/* This function is implemented in assembly in pacache.S */
extern void __clear_user_page_asm(void *page, unsigned long vaddr);
+ unsigned long flags __maybe_unused;
- purge_tlb_start();
+ purge_tlb_start(flags);
__clear_user_page_asm(page, vaddr);
- purge_tlb_end();
+ purge_tlb_end(flags);
}
#define FLUSH_THRESHOLD 0x80000 /* 0.5MB */
@@ -444,20 +447,22 @@ extern void clear_user_page_asm(void *page, unsigned long vaddr);
void clear_user_page(void *page, unsigned long vaddr, struct page *pg)
{
+ unsigned long flags __maybe_unused;
purge_kernel_dcache_page((unsigned long)page);
- purge_tlb_start();
+ purge_tlb_start(flags);
pdtlb_kernel(page);
- purge_tlb_end();
+ purge_tlb_end(flags);
clear_user_page_asm(page, vaddr);
}
EXPORT_SYMBOL(clear_user_page);
void flush_kernel_dcache_page_addr(void *addr)
{
+ unsigned long flags __maybe_unused;
flush_kernel_dcache_page_asm(addr);
- purge_tlb_start();
+ purge_tlb_start(flags);
pdtlb_kernel(addr);
- purge_tlb_end();
+ purge_tlb_end(flags);
}
EXPORT_SYMBOL(flush_kernel_dcache_page_addr);
@@ -490,8 +495,9 @@ void __flush_tlb_range(unsigned long sid, unsigned long start,
if (npages >= 512) /* 2MB of space: arbitrary, should be tuned */
flush_tlb_all();
else {
+ unsigned long flags __maybe_unused;
mtsp(sid, 1);
- purge_tlb_start();
+ purge_tlb_start(flags);
if (split_tlb) {
while (npages--) {
pdtlb(start);
@@ -504,7 +510,7 @@ void __flush_tlb_range(unsigned long sid, unsigned long start,
start += PAGE_SIZE;
}
}
- purge_tlb_end();
+ purge_tlb_end(flags);
}
}
diff --git a/arch/parisc/kernel/pci-dma.c b/arch/parisc/kernel/pci-dma.c
index 7d927ea..dbe6c8e 100644
--- a/arch/parisc/kernel/pci-dma.c
+++ b/arch/parisc/kernel/pci-dma.c
@@ -90,12 +90,13 @@ static inline int map_pte_uncached(pte_t * pte,
if (end > PMD_SIZE)
end = PMD_SIZE;
do {
+ unsigned long flags __maybe_unused;
if (!pte_none(*pte))
printk(KERN_ERR "map_pte_uncached: page already exists\n");
set_pte(pte, __mk_pte(*paddr_ptr, PAGE_KERNEL_UNC));
- purge_tlb_start();
+ purge_tlb_start(flags);
pdtlb_kernel(orig_vaddr);
- purge_tlb_end();
+ purge_tlb_end(flags);
vaddr += PAGE_SIZE;
orig_vaddr += PAGE_SIZE;
(*paddr_ptr) += PAGE_SIZE;
@@ -168,11 +169,12 @@ static inline void unmap_uncached_pte(pmd_t * pmd, unsigned long vaddr,
if (end > PMD_SIZE)
end = PMD_SIZE;
do {
+ unsigned long flags __maybe_unused;
pte_t page = *pte;
pte_clear(&init_mm, vaddr, pte);
- purge_tlb_start();
+ purge_tlb_start(flags);
pdtlb_kernel(orig_vaddr);
- purge_tlb_end();
+ purge_tlb_end(flags);
vaddr += PAGE_SIZE;
orig_vaddr += PAGE_SIZE;
pte++;
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
