Messages in this thread | | | Subject | Re: [patch 32/78] fix bad macro param in timer.c | From | Steven Rostedt <> | Date | Sun, 30 Jul 2006 19:31:15 -0400 |
| |
On Sun, 2006-07-30 at 03:03 -0700, akpm@osdl.org wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > We have > > #define INDEX(N) (base->timer_jiffies >> (TVR_BITS + N * TVN_BITS)) & TVN_MASK > > and it's used via > > list = varray[i + 1]->vec + (INDEX(i + 1)); > > So, due to underparenthesisation, this INDEX(i+1) is now a ... (TVR_BITS + i > + 1 * TVN_BITS)) ... > > So this bugfix changes behaviour. Why did it work before?
Shear luck!
Well the outside most parenthesis was OK to be missing because the use of those where done like this:
list = base->tv2.vec + (INDEX(0));
Where the user of the INDEX macro put the parenthesis at location of use and not in the macro (where it belonged).
The broken code was here:
for (i = 0; i < 4; i++) { j = INDEX(i); do { if (list_empty(varray[i]->vec + j)) { j = (j + 1) & TVN_MASK; continue; } list_for_each_entry(nte, varray[i]->vec + j, entry) if (time_before(nte->expires, expires)) expires = nte->expires; if (j < (INDEX(i)) && i < 3) list = varray[i + 1]->vec + (INDEX(i + 1)); goto found; } while (j != (INDEX(i))); } found: if (list) { /* * The search wrapped. We need to look at the next list * from next tv element that would cascade into tv element * where we found the timer element. */ list_for_each_entry(nte, list, entry) { if (time_before(nte->expires, expires)) expires = nte->expires; } }
Where the INDEX(i+1) was used. So why did it work?
Simple answer: It didn't
If i was anything but 0, it was broken. But this was only used by s390 and arm. Since it was for the next interrupt, could that next interrupt be a problem (going into the second cascade)? But it was probably seldom wrong. That is, this would fail if the next interrupt was in the second cascade, and was wrapped. Which may never of happened. Also if it did happen, it would have just missed the interrupt.
If an interrupt was missed, and no one was there to miss it, was it really missed :-)
So, this is a bug fix, that changes behavior, but I believe that the behavior that was changed, is for the better.
-- Steve
- 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/
| |