diff --git a/linuxthreads/ChangeLog b/linuxthreads/ChangeLog index 795c064f90..121eceb83e 100644 --- a/linuxthreads/ChangeLog +++ b/linuxthreads/ChangeLog @@ -1,3 +1,37 @@ +2000-06-13 Kaz Kylheku + + A few optimizations. Got rid of unnecessary wakeups of timer threads, + tightened up some critical regions and micro-optimized some list + manipulation code. + + * sysdeps/pthread/timer_routines.c (__timer_thread_queue_timer): + Returns int value now to indicate whether timer was queued at head. + * sysdeps/pthread/posix-timer.h: Likewise. + * sysdeps/pthread/timer_settime.c (timer_settime): Takes advantage of + new return value from __timer_thread_queue_timer to avoid waking + up timer thread unnecessarily. + + * sysdeps/pthread/posix-timer.h (timer_id2ptr): No longer checks + inuse flag, because this requires mutex to be held. Callers updated + to do the check when they have the mutex. + * sysdeps/pthread/timer_getoverr.c: Add check for inuse here. + + * sysdeps/pthread/timer_settime.c (timer_settime): Tighter critical + regions: avoids making system calls while holding timer mutex, and + a few computations were moved outside of the mutex as well. + * sysdeps/pthread/timer_gettime.c (timer_gettime): Likewise. + + * sysdeps/pthread/posix-timer.h (list_unlink_ip): Function name changed + to list_unlink_ip, meaning idempotent. Pointer manipulation + changed to get better better code out of gcc. + * sysdeps/pthread/timer_routines.c (list_unlink): Non-idempotent + version of list_unlink added here. + * sysdeps/pthread/timer_delete.c: Use appropriate list unlink + function in all places: idempotent one for timers, non-idempotent + one for thread nodes. + * sysdeps/pthread/timer_settime: Likewise. + * sysdeps/pthread/timer_routines.c: Likewise. + 2000-06-13 Ulrich Drepper * sysdeps/unix/sysv/linux/bits/posix_opt.h (_POSIX_TIMERS): Define. diff --git a/linuxthreads/sysdeps/pthread/posix-timer.h b/linuxthreads/sysdeps/pthread/posix-timer.h index bb66c2d3d8..feeff39fa8 100644 --- a/linuxthreads/sysdeps/pthread/posix-timer.h +++ b/linuxthreads/sysdeps/pthread/posix-timer.h @@ -93,7 +93,7 @@ extern struct thread_node __timer_signal_thread_tclk; static inline struct timer_node * timer_id2ptr (timer_t timerid) { - if (timerid >= 0 && timerid < TIMER_MAX && __timer_array[timerid].inuse) + if (timerid >= 0 && timerid < TIMER_MAX) return &__timer_array[timerid]; return NULL; @@ -155,10 +155,17 @@ timespec_sub (struct timespec *diff, const struct timespec *left, /* We need one of the list functions in the other modules. */ static inline void -list_unlink (struct list_links *list) +list_unlink_ip (struct list_links *list) { - list->next->prev = list->prev; - list->prev->next = list->next; + struct list_links *lnext = list->next, *lprev = list->prev; + + lnext->prev = lprev; + lprev->next = lnext; + + /* The suffix ip means idempotent; list_unlink_ip can be called + * two or more times on the same node. + */ + list->next = list; list->prev = list; } @@ -173,6 +180,6 @@ extern struct thread_node *__timer_thread_find_matching (const pthread_attr_t *d extern struct thread_node *__timer_thread_alloc (const pthread_attr_t *desired_attr, clockid_t); extern void __timer_dealloc (struct timer_node *timer); extern void __timer_thread_dealloc (struct thread_node *thread); -extern void __timer_thread_queue_timer (struct thread_node *thread, +extern int __timer_thread_queue_timer (struct thread_node *thread, struct timer_node *insert); extern void __timer_thread_wakeup (struct thread_node *thread); diff --git a/linuxthreads/sysdeps/pthread/timer_delete.c b/linuxthreads/sysdeps/pthread/timer_delete.c index 9e83ae1ab7..4636bf707a 100644 --- a/linuxthreads/sysdeps/pthread/timer_delete.c +++ b/linuxthreads/sysdeps/pthread/timer_delete.c @@ -36,7 +36,7 @@ timer_delete (timerid) pthread_mutex_lock (&__timer_mutex); timer = timer_id2ptr (timerid); - if (timer == NULL) + if (timer == NULL || !timer->inuse) /* Invalid timer ID or the timer is not in use. */ errno = EINVAL; else @@ -58,7 +58,7 @@ timer_delete (timerid) } /* Remove timer from whatever queue it may be on and deallocate it. */ - list_unlink (&timer->links); + list_unlink_ip (&timer->links); __timer_dealloc (timer); retval = 0; } diff --git a/linuxthreads/sysdeps/pthread/timer_getoverr.c b/linuxthreads/sysdeps/pthread/timer_getoverr.c index f3aee45e8d..8630f57829 100644 --- a/linuxthreads/sysdeps/pthread/timer_getoverr.c +++ b/linuxthreads/sysdeps/pthread/timer_getoverr.c @@ -29,16 +29,15 @@ int timer_getoverrun (timerid) timer_t timerid; { + struct timer_node *timer; int retval = -1; pthread_mutex_lock (&__timer_mutex); - if (timer_id2ptr (timerid) == NULL) + if ((timer = timer_id2ptr (timerid)) == NULL || !timer->inuse) errno = EINVAL; else - { - retval = 0; /* TODO: overrun counting not supported */ - } + retval = 0; /* TODO: overrun counting not supported */ pthread_mutex_lock (&__timer_mutex); diff --git a/linuxthreads/sysdeps/pthread/timer_gettime.c b/linuxthreads/sysdeps/pthread/timer_gettime.c index 2db89a06eb..43b07598b7 100644 --- a/linuxthreads/sysdeps/pthread/timer_gettime.c +++ b/linuxthreads/sysdeps/pthread/timer_gettime.c @@ -37,7 +37,7 @@ timer_gettime (timerid, value) pthread_mutex_lock (&__timer_mutex); timer = timer_id2ptr (timerid); - if (timer == NULL) + if (timer == NULL && !timer->inuse) /* Invalid timer ID or the timer is not in use. */ errno = EINVAL; else @@ -46,7 +46,9 @@ timer_gettime (timerid, value) if (timer->armed) { + pthread_mutex_unlock (&__timer_mutex); clock_gettime (timer->clock, &now); + pthread_mutex_lock (&__timer_mutex); timespec_sub (&value->it_value, &timer->expirytime, &now); } else diff --git a/linuxthreads/sysdeps/pthread/timer_routines.c b/linuxthreads/sysdeps/pthread/timer_routines.c index 520f6ee2e7..ddf02fadd6 100644 --- a/linuxthreads/sysdeps/pthread/timer_routines.c +++ b/linuxthreads/sysdeps/pthread/timer_routines.c @@ -90,6 +90,20 @@ list_insbefore (struct list_links *list, struct list_links *newp) list_append (list, newp); } +/* + * Like list_unlink_ip, except that calling it on a node that + * is already unlinked is disastrous rather than a noop. + */ + +static inline void +list_unlink (struct list_links *list) +{ + struct list_links *lnext = list->next, *lprev = list->prev; + + lnext->prev = lprev; + lprev->next = lnext; +} + static inline struct list_links * list_first (struct list_links *list) { @@ -279,10 +293,7 @@ thread_cleanup (void *val) thread->current_timer = 0; if (list_isempty (&thread->timer_queue)) - { - list_unlink (&thread->links); __timer_thread_dealloc (thread); - } else (void) __timer_thread_start (thread); @@ -394,7 +405,7 @@ thread_func (void *arg) if (timespec_compare (&now, &timer->expirytime) < 0) break; - list_unlink (first); + list_unlink_ip (first); if (__builtin_expect (timer->value.it_interval.tv_sec, 0) != 0 || timer->value.it_interval.tv_nsec != 0) @@ -432,12 +443,16 @@ thread_func (void *arg) } -/* Enqueue a timer in wakeup order in the thread's timer queue. */ -void +/* Enqueue a timer in wakeup order in the thread's timer queue. + Returns 1 if the timer was inserted at the head of the queue, + causing the queue's next wakeup time to change. */ + +int __timer_thread_queue_timer (struct thread_node *thread, struct timer_node *insert) { struct list_links *iter; + int athead = 1; for (iter = list_first (&thread->timer_queue); iter != list_null (&thread->timer_queue); @@ -447,9 +462,11 @@ __timer_thread_queue_timer (struct thread_node *thread, if (timespec_compare (&insert->expirytime, &timer->expirytime) < 0) break; + athead = 0; } list_insbefore (iter, &insert->links); + return athead; } @@ -533,7 +550,7 @@ __timer_alloc (void) if (node != list_null (&timer_free_list)) { struct timer_node *timer = timer_links2ptr (node); - list_unlink (node); + list_unlink_ip (node); timer->inuse = 1; return timer; } diff --git a/linuxthreads/sysdeps/pthread/timer_settime.c b/linuxthreads/sysdeps/pthread/timer_settime.c index 63b117f797..858edc7657 100644 --- a/linuxthreads/sysdeps/pthread/timer_settime.c +++ b/linuxthreads/sysdeps/pthread/timer_settime.c @@ -35,11 +35,9 @@ timer_settime (timerid, flags, value, ovalue) struct timer_node *timer; struct thread_node *thread = NULL; struct timespec now; - int have_now = 0; + int have_now = 0, need_wakeup = 0; int retval = -1; - pthread_mutex_lock (&__timer_mutex); - timer = timer_id2ptr (timerid); if (timer == NULL) { @@ -56,14 +54,40 @@ timer_settime (timerid, flags, value, ovalue) goto bail; } + /* Will need to know current time since this is a relative timer; + might as well make the system call outside of the lock now! */ + + if ((flags & TIMER_ABSTIME) == 0) + { + clock_gettime (timer->clock, &now); + have_now = 1; + } + + pthread_mutex_lock (&__timer_mutex); + + /* One final check of timer validity; this one is possible only + until we have the mutex, which guards the inuse flag. */ + + if (!timer->inuse) + { + errno = EINVAL; + goto unlock_bail; + } + if (ovalue != NULL) { ovalue->it_interval = timer->value.it_interval; if (timer->armed) { - clock_gettime (timer->clock, &now); - have_now = 1; + if (! have_now) + { + pthread_mutex_unlock (&__timer_mutex); + clock_gettime (timer->clock, &now); + have_now = 1; + pthread_mutex_lock (&__timer_mutex); + } + timespec_sub (&ovalue->it_value, &timer->expirytime, &now); } else @@ -75,11 +99,12 @@ timer_settime (timerid, flags, value, ovalue) timer->value = *value; - list_unlink (&timer->links); + list_unlink_ip (&timer->links); timer->armed = 0; thread = timer->thread; + /* A value of { 0, 0 } causes the timer to be stopped. */ if (value->it_value.tv_sec != 0 || __builtin_expect (value->it_value.tv_nsec != 0, 1)) { @@ -87,25 +112,21 @@ timer_settime (timerid, flags, value, ovalue) /* The user specified the expiration time. */ timer->expirytime = value->it_value; else - { - if (! have_now) - clock_gettime (timer->clock, &now); + timespec_add (&timer->expirytime, &now, &value->it_value); - timespec_add (&timer->expirytime, &now, &value->it_value); - } - - __timer_thread_queue_timer (thread, timer); + /* Only need to wake up the thread if timer is inserted + at the head of the queue. */ + need_wakeup = __timer_thread_queue_timer (thread, timer); timer->armed = 1; } retval = 0; -bail: +unlock_bail: pthread_mutex_unlock (&__timer_mutex); - /* TODO: optimize this. Only need to wake up the thread if inserting - a new timer at the head of the queue. */ - if (thread != NULL) +bail: + if (thread != NULL && need_wakeup) __timer_thread_wakeup (thread); return retval;