Re: PATCH: Race in 2.6.0-test2 timer code

[linas@austin.ibm.com]

On Thu, Jul 31, 2003 at 01:56:07AM +0200, Andrea Arcangeli wrote:
> On Wed, Jul 30, 2003 at 06:43:18PM -0500, linas@austin.ibm.com wrote:
> > On Thu, Jul 31, 2003 at 12:17:17AM +0200, Andrea Arcangeli wrote:
> > > So the best fix would be to nuke the run_all_timers thing from 2.4 too.
> > Yes.
[...]
> > And so either of Andrea's fixes should fix this race.
> 
> exactly. If you want to nuke the run_all_timers from the 2.4 backport
> feel free, then we could drop the additional locking from
> add_timer/del_timer* and leave it only in mod_timer like 2.6 does, that
> will avoid cacheline bouncing in the small window when the local timer
> irq run on top of an irq handler. In the meantime the kernel is already
> solid (again ;).

OK, I looked at removing run_all_timers, it doesn't seem too hard. 

I would need to: 
-- add TIMER_SOFTIRQ to interrupts.h,
-- add open_softirq (run_timer_softirq) to timer.c init_timer()
-- move guts of run_local_timers() to run_timer_softirq()
-- remove bh locks in above, not yet sure about other locks
-- remove TIMER_BH everywhere.  Or rather, remove it for those
   arches that support cpu-local timer interupts (curently x86 & freinds, 
   soon hopefully ppc64, I attach it below, in case other arches want to 
   play with this).

Is that right?
Should I do this patch or will people loose interest in it?
Who should I send it to?  Who wants to review it?

> BTW, it's reassuring it wasn't lack of 2.6 stress testing, I apologize
> for claiming that.

Me too, sorry for making noise before figuring it all out. But then, 
I remember asking a college physics professor for a recommendation, 
and he didn't know me well.  "You sat in the back of the class and
didn't ask any questions, unlike so-n-so, (the #1 student in the class) 
who sat in front and asked a lot of questions."  I told him that that was 
because I knew the answers to all of the questions that were being asked.
(I  was trying to distance myself from the stupid people up front).
He wasn't impressed by my reply, I hurt pretty bad.

--------------------------------------------------------------------
Re the per-cpu, local timer patch, unless I'm badly mistaken, its pretty 
trivial, right:?

Index: kernel/timer.c
===================================================================
RCS file: /home/linas/cvsroot/linux24/kernel/timer.c,v
retrieving revision 1.1.1.1.4.1
diff -u -p -u -r1.1.1.1.4.1 timer.c
--- kernel/timer.c   15 Jul 2003 18:43:52 -0000 1.1.1.1.4.1
+++ kernel/timer.c   31 Jul 2003 15:30:26 -0000
@@ -764,7 +764,7 @@ void do_timer(struct pt_regs *regs)
   /* SMP process accounting uses the local APIC timer */
                                                                                
   update_process_times(user_mode(regs));
-#if defined(CONFIG_X86) || defined(CONFIG_IA64) /* x86-64 is also included by CONFIG_X86 */
+#if defined(CONFIG_X86) || defined(CONFIG_IA64) || defined(CONFIG_PPC64) /* x86-64 is also included by CONFIG_X86 */
   mark_bh(TIMER_BH);
 #endif
 #endif
@@ -772,7 +772,7 @@ void do_timer(struct pt_regs *regs)
    * Right now only x86-SMP calls run_local_timers() from a
    * per-CPU interrupt.
    */
-#if !defined(CONFIG_X86) && !defined(CONFIG_IA64) /* x86-64 is also included by CONFIG_X86 */
+#if !defined(CONFIG_X86) && !defined(CONFIG_IA64) && !defined(CONFIG_PPC64) /*
x86-64 is also included by CONFIG_X86 */
   mark_bh(TIMER_BH);
 #endif
   update_times();
Index: arch/ppc64/kernel/smp.c
===================================================================
RCS file: /home/linas/cvsroot/linux24/arch/ppc64/kernel/smp.c,v
retrieving revision 1.2.4.1
diff -u -p -u -r1.2.4.1 smp.c
--- arch/ppc64/kernel/smp.c   15 Jul 2003 18:41:56 -0000 1.2.4.1
+++ arch/ppc64/kernel/smp.c   31 Jul 2003 15:21:35 -0000
@@ -398,6 +398,8 @@ void smp_local_timer_interrupt(struct pt
      update_process_times(user_mode(regs));
      (get_paca()->prof_counter)=get_paca()->prof_multiplier;
   }
+
+  run_local_timers();
 }
                                                                                
 void smp_message_recv(int msg, struct pt_regs *regs)