The attached patch is for 2.5.65. As of this moment, the bk patch has not been posted to the snapshots directory. I will wait for that to update.

For what its worth, can someone explain how the add_timer call from run_timers was causing a problem. The code looks right to me, unless the caller is so nasty as to continue to do the same thing (which would loop forever). In this case, the simple fix is to bump the base->timer_jiffies at the beginning of the loop rather than the end. This would cause the new timer to be put in the next jiffie instead of the current one AND it is free!

-g

Tim Schmielau wrote:> > On Tue, 18 Mar 2003, Andrew Morton wrote:> > >>george anzinger <george@mvista.com> wrote:>>>>>Here is a fix for the problem that eliminates the index from the>>>structure.> > [...]> >>Seems to be a nice change. I think it would be better to get Tim's fix into>>Linus's tree and let your rationalisation bake for a while in -mm.> > > I'm all for this way. Push my quick'n ugly patch to mainline soon to get> thinks working again. Have at least one mainline release before changing> again to start off from something working. Then add George's patch when> it has matured.> > >>There is currently a mysterious timer lockup happening on power4 machines.>>I'd like to keep these changes well-separated in time so we can get an>>understanding of what code changes correlate with changed behaviour.> > > Can this problem be reproduced with INITIAL_JIFFIES=0? Just to make sure I> didn't break something more.> > > On Tue, 18 Mar 2003, George Anzinger wrote:> > >>Here is a fix for the problem that eliminates the index from the>>structure. The index ALWAYS depends on the current value of>>base->timer_jiffies in a rather simple way which is I exploit. Either>>patch works, but this seems much simpler...> > [...]> >>@@ -384,22 +382,26 @@>> * This function cascades all vectors and executes all expired timer>> * vectors.>> */>>+#define INDEX(N) (base->timer_jiffies >> (TVR_BITS + N * TVN_BITS)) &> > TVN_MASK> > No, with the current implementation we need> #define INDEX(N) (base->timer_jiffies >> (TVR_BITS + N * TVN_BITS) +1) &> TVN_MASK> although I'd like to see that cleaned up.

I tried with the +1 and boot hangs trying to set up networking. I think the difference is that the init code is trying to set things up the way they would look AFTER cascade executes and this is doing it BEFORE the cascade call.> > >>+>>static inline void __run_timers(tvec_base_t *base)>> {>>+ int index = base->timer_jiffies & TVR_MASK;>> spin_lock_irq(&base->lock);>>+ if(jiffies - base->timer_jiffies > 0)>> while ((long)(jiffies - base->timer_jiffies) >= 0) {>> struct list_head *head, *curr;>>> > > Are the doubled 'if' and 'while' really what you meant?

Again, I removed the -1 in the attached.> > > Did you bother to test the patch? It doesn't even boot for me, and I don't> see how it is supposed to.> I'll look into it more closely in the evening. Have to go to work now.

The old one ran on 2.5.64 but not 2.5.65 ??? I found and fixed a bug (index needs to be caculated INSIDE the while loop) that seems to have been the cause.> > Tim