Re: PATCH: Race in 2.6.0-test2 timer code

From: linas@austin.ibm.com
Date: Thu Jul 31 2003 - 12:23:34 EST


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)

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Jul 31 2003 - 22:00:50 EST