short description of timer.c/timer.h related changes in pre7-2,3

From: Ingo Molnar (mingo@elte.hu)
Date: Wed May 03 2000 - 07:57:56 EST


On Tue, 2 May 2000, Miles Lane wrote:

> fbcon.c: In function `cursor_timer_handler':
> fbcon.c:240: structure has no member named `next'
> fbcon.c:240: structure has no member named `prev'

yup - i cleaned up the timer code some more, but did not want to change
code that i'm not using personally. The changes only affect source code
which relied on internal timer details incorrectly. The timer code now
uses list.h lists, and the new thing is that timers expiring within the
same timer tick are executed in the same order as they were added (FIFO).
The list.h change caused timer initializations and other 'tricks' to fail.

The breakages are very simple in most cases, the most common categories:

- static struct timer_list sound_timer = { NULL, NULL, 0, 0,
- kd_nosound };
+ static struct timer_list sound_timer = { function: kd_nosound };

here the bug (well, not a real bug but a 'cleanliness issue') was that
sound_timer was initialized with the knowledge of the ->prev ->next
pointers. The fix is to initialize the only non-NULL field explicitly
(GCC will set everything else to zero).

 static struct timer_list dst_gc_timer =
- { NULL, NULL, DST_GC_MIN, 0L, dst_run_gc };
+ { data: DST_GC_MIN, function: dst_run_gc };

sometimes the 'data' field is set at compile-time as well.

- busy->timeout.prev = busy->timeout.next = NULL;
+ init_timer(&busy->timeout);

this is the same type of initialization bug, in a different way.

- if (fd_timer.next)
+ if (timer_pending(&fd_timer))

this was a more serious layering violation, the code relied on the
'timer->next' pointer to detect running timers. We've had the
timer_pending() API for a long time, so it's time to switch to that.

- if (fd_timer.prev)
+ if (timer_pending(&fd_timer))

this happens pretty often as well, the ->prev pointer is used to detect
running timers. This case nicely shows why such layering violations are a
loss in the long run: the '->prev' pointer is at structure offset 4, while
'->next' is at offset 0 (which is optimized by many CPUs), so GCC will
generate poorer and larger code in a percentage of these cases. The
'timer_pending()' function has internal knowledge about the timer
structure, it will always use the fastest method. Later on it can be
potentially changed without changing hundreds of files.

(so the fix in your fbcon.c case is to use init_timer() instead of
explicit initialization of the timer structure.)

        Ingo

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



This archive was generated by hypermail 2b29 : Sun May 07 2000 - 21:00:12 EST