Skip to contentSkip to conversation

mmu_hash_lock deadlock causes kernel stuck at 2.6.21 SMP powerpc 32bit

PATCH New Gaash Hazan 2 participants 3 messages Mar 30, 2008 PowerPC

lore
Share & tools
lore
#0 lore raw

(reposting)

Hello PPC SMP MM experts,

mmu_hash_lock (arch/powerpc/mm/hash_low_32.S) is a (non-standard) spin lock that protects the CPU MMU hashing table. It exists and used only with SMP configurations.

In some scenarios, the spin lock is taken when interrupts are *enabled* causing kernel deadlock at the next take attempt in the same CPU.

The deadlock happened on 2.6.21 kernel, Powerpc 32 bit with SMP enabled. At this moment system had one active CPU. The sequence I saw was:

do_exit (program termination)
exit_mm
mmput
exit_mmap
free_pgtables
free_pgd_range
unmap_vmas
pte_free
hash_page_sync (takes mmu_hash_lock. Note: interrupts are enabled)

timer_interrupt (timer interrupts occurs during hash_page_sync, lock is taken) irq_exit do_softirq __do_softirq net_rx_action (packet received from network) ( ... omitted ... ) xdr_skb_read_bits skb_copy_bits memcpy - memcpy causes DSI exception(0x300). This is OK.
DSI exception handler calls hash_page hash_page waits for mmu_mash_lock. It waits forever since the lock is already taken.
Deadlock! with interrupts disabled. kernel is dead.

I think the rout cause of the problem is hash_page_sync() taking the mmu_hash_lock spin lock without disabling interrupts. This leads to the deadlock.

To verify the theory, hash_page_sync() was wrapped with interrupts disabled code and problem never occurred again. Of course this is temporary workaround as there are several places needed to be fixed.

What do you think?

Thanks,

Gaash

#1 lore raw
Quoted 4 lines · Gaash Hazan

To verify the theory, hash_page_sync() was wrapped with interrupts disabled code and problem never occurred again. Of course this is temporary workaround as there are several places needed to be fixed.

It is definitely a bug in hash_page_sync() which should clear MSR:EE while holding the lock.

I'll do a patch.

Thanks for finding that out !

Cheers,
Ben.

#2 lore raw

PATCH 1/1 powerpc: Fix deadlock with mmu_hash_lock in hash_page_sync

In reply to Gaash Hazan

hash_page_sync() takes and releases the low level mmu hash lock in order to sync with other processors disposing of page tables. Because that lock can be needed to service hash misses triggered by interrupt handler, taking it must be done with interrupts off. However, hash_page_sync() appear to be called with interrupts enabled, thus causing occasional deadlocks.

We fix it by making sure hash_page_sync() masks interrupts while holding the lock.

---

Please test and report asap thought it's probably too late for 2.6.25, it can still go into stable later on.

arch/powerpc/mm/hash_low_32.S +6 -2
Index: linux-work/arch/powerpc/mm/hash_low_32.S
===================================================================
@@ -44,6 +44,9 @@ mmu_hash_lock:
 #ifdef CONFIG_SMP
 	.text
 _GLOBAL(hash_page_sync)
+	mfmsr   r10
+	rlwinm  r0,r10,0,17,15          /* clear bit 16 (MSR_EE) */
+	mtmsr   r0
 	lis	r8,mmu_hash_lock@h
 	ori	r8,r8,mmu_hash_lock@l
 	lis	r0,0x0fff
@@ -60,8 +63,9 @@ _GLOBAL(hash_page_sync)
 	eieio
 	li	r0,0
 	stw	r0,0(r8)
-	blr	
-#endif
+	mtmsr	r10
+	blr
+#endif /* CONFIG_SMP */
 
 /*
  * Load a PTE into the hash table, if possible.