Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

xtime_lock vs update_process_times

Commit d3d74453c34f8fd87674a8cf5b8a327c68f22e99 ("hrtimer: fixup the
HRTIMER_CB_IRQSAFE_NO_SOFTIRQ fallback") broke several archs, and since
only Russell bothered to merge the fix, and Greg to ACK his arch, I'm
sending this for merger.

I have confirmation that the Alpha bit results in a booting kernel.
That leaves: blackfin, frv, sh and sparc untested.

The deadlock in question was found by Russell:

IRQ handle
-> timer_tick() - xtime seqlock held for write
-> update_process_times()
-> run_local_timers()
-> hrtimer_run_queues()
-> hrtimer_get_softirq_time() - tries to get a read lock

Now, Thomas assures me the fix is trivial, only do_timer() needs to be
done under the xtime_lock, and update_process_times() can savely be
removed from under it.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Acked-by: Greg Ungerer <gerg@uclinux.org>
CC: Richard Henderson <rth@twiddle.net>
CC: Bryan Wu <bryan.wu@analog.com>
CC: David Howells <dhowells@redhat.com>
CC: Paul Mundt <lethal@linux-sh.org>
CC: William Irwin <wli@holomorphy.com>
Acked-by: Ingo Molnar <mingo@elte.hu>
Acked-by: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Peter Zijlstra and committed by
Linus Torvalds
aa02cd2d 10270d48

+28 -33
+8 -7
arch/alpha/kernel/time.c
··· 119 119 state.partial_tick = delta & ((1UL << FIX_SHIFT) - 1); 120 120 nticks = delta >> FIX_SHIFT; 121 121 122 - while (nticks > 0) { 123 - do_timer(1); 124 - #ifndef CONFIG_SMP 125 - update_process_times(user_mode(get_irq_regs())); 126 - #endif 127 - nticks--; 128 - } 122 + if (nticks) 123 + do_timer(nticks); 129 124 130 125 /* 131 126 * If we have an externally synchronized Linux clock, then update ··· 136 141 } 137 142 138 143 write_sequnlock(&xtime_lock); 144 + 145 + #ifndef CONFIG_SMP 146 + while (nticks--) 147 + update_process_times(user_mode(get_irq_regs())); 148 + #endif 149 + 139 150 return IRQ_HANDLED; 140 151 } 141 152
+5 -3
arch/blackfin/kernel/time.c
··· 137 137 138 138 do_timer(1); 139 139 140 - #ifndef CONFIG_SMP 141 - update_process_times(user_mode(get_irq_regs())); 142 - #endif 143 140 profile_tick(CPU_PROFILING); 144 141 145 142 /* ··· 158 161 last_rtc_update = xtime.tv_sec - 600; 159 162 } 160 163 write_sequnlock(&xtime_lock); 164 + 165 + #ifndef CONFIG_SMP 166 + update_process_times(user_mode(get_irq_regs())); 167 + #endif 168 + 161 169 return IRQ_HANDLED; 162 170 } 163 171
+4 -2
arch/frv/kernel/time.c
··· 63 63 /* last time the cmos clock got updated */ 64 64 static long last_rtc_update = 0; 65 65 66 + profile_tick(CPU_PROFILING); 66 67 /* 67 68 * Here we are in the timer irq handler. We just have irqs locally 68 69 * disabled but we don't know if the timer_bh is running on the other ··· 74 73 write_seqlock(&xtime_lock); 75 74 76 75 do_timer(1); 77 - update_process_times(user_mode(get_irq_regs())); 78 - profile_tick(CPU_PROFILING); 79 76 80 77 /* 81 78 * If we have an externally synchronized Linux clock, then update ··· 98 99 #endif /* CONFIG_HEARTBEAT */ 99 100 100 101 write_sequnlock(&xtime_lock); 102 + 103 + update_process_times(user_mode(get_irq_regs())); 104 + 101 105 return IRQ_HANDLED; 102 106 } 103 107
+7 -5
arch/m68knommu/kernel/time.c
··· 42 42 /* last time the cmos clock got updated */ 43 43 static long last_rtc_update=0; 44 44 45 + if (current->pid) 46 + profile_tick(CPU_PROFILING); 47 + 45 48 write_seqlock(&xtime_lock); 46 49 47 50 do_timer(1); 48 - #ifndef CONFIG_SMP 49 - update_process_times(user_mode(get_irq_regs())); 50 - #endif 51 - if (current->pid) 52 - profile_tick(CPU_PROFILING); 53 51 54 52 /* 55 53 * If we have an externally synchronized Linux clock, then update ··· 65 67 } 66 68 67 69 write_sequnlock(&xtime_lock); 70 + 71 + #ifndef CONFIG_SMP 72 + update_process_times(user_mode(get_irq_regs())); 73 + #endif 68 74 return(IRQ_HANDLED); 69 75 } 70 76
-9
arch/sh/kernel/timers/timer-cmt.c
··· 100 100 timer_status &= ~0x80; 101 101 ctrl_outw(timer_status, CMT_CMCSR_0); 102 102 103 - /* 104 - * Here we are in the timer irq handler. We just have irqs locally 105 - * disabled but we don't know if the timer_bh is running on the other 106 - * CPU. We need to avoid to SMP race with it. NOTE: we don' t need 107 - * the irq version of write_lock because as just said we have irq 108 - * locally disabled. -arca 109 - */ 110 - write_seqlock(&xtime_lock); 111 103 handle_timer_tick(); 112 - write_sequnlock(&xtime_lock); 113 104 114 105 return IRQ_HANDLED; 115 106 }
-2
arch/sh/kernel/timers/timer-mtu2.c
··· 100 100 ctrl_outb(timer_status, MTU2_TSR_1); 101 101 102 102 /* Do timer tick */ 103 - write_seqlock(&xtime_lock); 104 103 handle_timer_tick(); 105 - write_sequnlock(&xtime_lock); 106 104 107 105 return IRQ_HANDLED; 108 106 }
+1 -1
arch/sparc/kernel/pcic.c
··· 713 713 write_seqlock(&xtime_lock); /* Dummy, to show that we remember */ 714 714 pcic_clear_clock_irq(); 715 715 do_timer(1); 716 + write_sequnlock(&xtime_lock); 716 717 #ifndef CONFIG_SMP 717 718 update_process_times(user_mode(get_irq_regs())); 718 719 #endif 719 - write_sequnlock(&xtime_lock); 720 720 return IRQ_HANDLED; 721 721 } 722 722
+3 -4
arch/sparc/kernel/time.c
··· 128 128 clear_clock_irq(); 129 129 130 130 do_timer(1); 131 - #ifndef CONFIG_SMP 132 - update_process_times(user_mode(get_irq_regs())); 133 - #endif 134 - 135 131 136 132 /* Determine when to update the Mostek clock. */ 137 133 if (ntp_synced() && ··· 141 145 } 142 146 write_sequnlock(&xtime_lock); 143 147 148 + #ifndef CONFIG_SMP 149 + update_process_times(user_mode(get_irq_regs())); 150 + #endif 144 151 return IRQ_HANDLED; 145 152 } 146 153