Date: Sun, 19 Dec 2004 08:12:07 GMT From: David Xu <davidxu@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 67346 for review Message-ID: <200412190812.iBJ8C7BI025867@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=67346 Change 67346 by davidxu@davidxu_tiger on 2004/12/19 08:11:50 simplify some mutex locking code, this causes performance of super-smack test to be increased about 15%. Affected files ... .. //depot/projects/davidxu_thread/src/lib/libthread/Makefile#5 edit .. //depot/projects/davidxu_thread/src/lib/libthread/sys/lock.h#4 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_fork.c#4 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_kern.c#4 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_mutex.c#4 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_private.h#6 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_spinlock.c#3 edit Differences ... ==== //depot/projects/davidxu_thread/src/lib/libthread/Makefile#5 (text+ko) ==== ==== //depot/projects/davidxu_thread/src/lib/libthread/sys/lock.h#4 (text+ko) ==== ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_fork.c#4 (text+ko) ==== @@ -88,28 +88,14 @@ /* Child process */ errsave = errno; - /* clear aother thread locked us. */ - _UMTX_REINIT(&curthread->lock); - thr_self(&curthread->tid); + _thr_mutex_reinit(&_thr_atfork_mutex); - /* reinitialize libc spinlocks, this includes __malloc_lock. */ - _thr_spinlock_init(); - - /* - * reinitialize all mutexes the curthread owns, these include - * _thr_fork_mutex. - */ - TAILQ_FOREACH(m, &curthread->mutexq, m_qe) { - _thr_mutex_reinit(curthread, &m); - } /* Reinitialize lib kernel. */ _thr_single_thread(curthread); /* Restore signal mask. */ __sys_sigprocmask(SIG_SETMASK, &oldset, NULL); - _pthread_mutex_unlock(&_thr_atfork_mutex); - /* Run down atfork child handlers. */ TAILQ_FOREACH(af, &_thr_atfork_list, qe) { if (af->child != NULL) ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_kern.c#4 (text+ko) ==== @@ -124,6 +124,14 @@ void _thr_single_thread(struct pthread *curthread) { + curthread->cancelflags &= ~THR_CANCELLING; + /* clear aother thread locked us. */ + _UMTX_REINIT(&curthread->lock); + thr_self(&curthread->tid); + /* reinitialize libc spinlocks, this includes __malloc_lock. */ + _thr_spinlock_init(); + TAILQ_INIT(&curthread->mutexq); + curthread->priority_mutex_count = 0; _libpthread_init(curthread); #if 0 if (__isthreaded) { ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_mutex.c#4 (text+ko) ==== @@ -74,8 +74,7 @@ /* * Prototypes */ -static long mutex_handoff(struct pthread *, - struct pthread_mutex *); +static long mutex_handoff(struct pthread *, struct pthread_mutex *); static inline int mutex_self_trylock(struct pthread *, pthread_mutex_t); static inline int mutex_self_lock(struct pthread *, pthread_mutex_t); static int mutex_unlock_common(pthread_mutex_t *, int); @@ -134,7 +133,7 @@ /* Check mutex protocol: */ else if (((*mutex_attr)->m_protocol < PTHREAD_PRIO_NONE) || - ((*mutex_attr)->m_protocol > PTHREAD_MUTEX_RECURSIVE)) + ((*mutex_attr)->m_protocol > PTHREAD_PRIO_PROTECT)) /* Return an invalid argument error: */ ret = EINVAL; @@ -203,16 +202,13 @@ } void -_thr_mutex_reinit(struct pthread *curthread, pthread_mutex_t *mutex) +_thr_mutex_reinit(pthread_mutex_t *mutex) { _UMTX_REINIT(&(*mutex)->m_lock); TAILQ_INIT(&(*mutex)->m_queue); - if (curthread == NULL || - (*mutex)->m_owner != curthread) { - MUTEX_INIT_LINK(*mutex); - (*mutex)->m_owner = NULL; - (*mutex)->m_count = 0; - } + MUTEX_INIT_LINK(*mutex); + (*mutex)->m_owner = NULL; + (*mutex)->m_count = 0; (*mutex)->m_refcount = 0; (*mutex)->m_prio = 0; (*mutex)->m_saved_prio = 0; @@ -221,16 +217,12 @@ int _pthread_mutex_destroy(pthread_mutex_t *mutex) { - struct pthread *curthread = _get_curthread(); pthread_mutex_t m; int ret = 0; if (mutex == NULL || *mutex == NULL) ret = EINVAL; else { - /* Lock the mutex structure: */ - THR_LOCK_ACQUIRE(curthread, &(*mutex)->m_lock); - /* * Check to see if this mutex is in use: */ @@ -238,9 +230,6 @@ (TAILQ_FIRST(&(*mutex)->m_queue) != NULL) || ((*mutex)->m_refcount != 0)) { ret = EBUSY; - - /* Unlock the mutex structure: */ - THR_LOCK_RELEASE(curthread, &(*mutex)->m_lock); } else { /* * Save a pointer to the mutex so it can be free'd @@ -249,9 +238,6 @@ m = *mutex; *mutex = NULL; - /* Unlock the mutex structure: */ - THR_LOCK_RELEASE(curthread, &m->m_lock); - /* * Free the memory allocated for the mutex * structure: @@ -307,6 +293,28 @@ THR_ASSERT((mutex != NULL) && (*mutex != NULL), "Uninitialized mutex in pthread_mutex_trylock_basic"); + if ((*mutex)->m_protocol == PTHREAD_PRIO_NONE) { + ret = umtx_trylock(&(*mutex)->m_lock, curthread->tid); + if (ret == 0) { + (*mutex)->m_owner = curthread; + /* + * XXX there should be a separated list for owned mutex, + * separated it from priority mutex list + */ +#if 0 + /* Add to the list of owned mutexes: */ + MUTEX_ASSERT_NOT_OWNED(*mutex); + TAILQ_INSERT_TAIL(&curthread->mutexq, + (*mutex), m_qe); +#endif + } else if ((*mutex)->m_owner == curthread) + ret = mutex_self_trylock(curthread, *mutex); + else + ret = EBUSY; + + return (ret); + } + /* Lock the mutex structure: */ THR_LOCK_ACQUIRE(curthread, &(*mutex)->m_lock); @@ -322,24 +330,6 @@ /* Process according to mutex type: */ switch ((*mutex)->m_protocol) { - /* Default POSIX mutex: */ - case PTHREAD_PRIO_NONE: - /* Check if this mutex is not locked: */ - if ((*mutex)->m_owner == NULL) { - /* Lock the mutex for the running thread: */ - (*mutex)->m_owner = curthread; - - /* Add to the list of owned mutexes: */ - MUTEX_ASSERT_NOT_OWNED(*mutex); - TAILQ_INSERT_TAIL(&curthread->mutexq, - (*mutex), m_qe); - } else if ((*mutex)->m_owner == curthread) - ret = mutex_self_trylock(curthread, *mutex); - else - /* Return a busy error: */ - ret = EBUSY; - break; - /* POSIX priority inheritence mutex: */ case PTHREAD_PRIO_INHERIT: /* Check if this mutex is not locked: */ @@ -472,6 +462,47 @@ THR_ASSERT((m != NULL) && (*m != NULL), "Uninitialized mutex in mutex_lock_common"); + if ((*m)->m_protocol == PTHREAD_PRIO_NONE) { + /* Default POSIX mutex: */ + ret = umtx_trylock(&(*m)->m_lock, curthread->tid); + if (ret == 0) { + (*m)->m_owner = curthread; + /* + * XXX there should be a separated list for owned mutex, + * separated it from priority mutex list + */ +#if 0 + /* Add to the list of owned mutexes: */ + MUTEX_ASSERT_NOT_OWNED(*m); + TAILQ_INSERT_TAIL(&curthread->mutexq, + (*m), m_qe); +#endif + } else if ((*m)->m_owner == curthread && + !((*m)->m_type == PTHREAD_MUTEX_NORMAL && + abstime != NULL)) { + ret = mutex_self_lock(curthread, *m); + } else { + if (abstime != NULL) { + ret = umtx_timedlock(&(*m)->m_lock, curthread->tid, + (struct timespec *)abstime); + if (ret == EAGAIN || ret == EINTR) + ret = ETIMEDOUT; + } else { + ret = _UMTX_LOCK(&(*m)->m_lock, curthread->tid); + } + if (ret == 0) { + (*m)->m_owner = curthread; +#if 0 + /* Add to the list of owned mutexes: */ + MUTEX_ASSERT_NOT_OWNED(*m); + TAILQ_INSERT_TAIL(&curthread->mutexq, + (*m), m_qe); +#endif + } + } + return (ret); + } + if (abstime != NULL && (abstime->tv_sec < 0 || abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)) return (EINVAL); @@ -505,68 +536,6 @@ /* Process according to mutex type: */ switch ((*m)->m_protocol) { - /* Default POSIX mutex: */ - case PTHREAD_PRIO_NONE: - if ((*m)->m_owner == NULL) { - /* Lock the mutex for this thread: */ - (*m)->m_owner = curthread; - - /* Add to the list of owned mutexes: */ - MUTEX_ASSERT_NOT_OWNED(*m); - TAILQ_INSERT_TAIL(&curthread->mutexq, - (*m), m_qe); - - /* Unlock the mutex structure: */ - THR_LOCK_RELEASE(curthread, &(*m)->m_lock); - } else if ((*m)->m_owner == curthread) { - ret = mutex_self_lock(curthread, *m); - - /* Unlock the mutex structure: */ - THR_LOCK_RELEASE(curthread, &(*m)->m_lock); - } else { - /* Set the wakeup time: */ - if (abstime) { - curthread->wakeup_time.tv_sec = - abstime->tv_sec; - curthread->wakeup_time.tv_nsec = - abstime->tv_nsec; - } - - /* - * Join the queue of threads waiting to lock - * the mutex and save a pointer to the mutex. - */ - mutex_queue_enq(*m, curthread); - curthread->data.mutex = *m; - curthread->sigbackout = mutex_lock_backout; - /* - * This thread is active and is in a critical - * region (holding the mutex lock); we should - * be able to safely set the state. - */ - THR_LOCK_SWITCH(curthread); - THR_SET_STATE(curthread, PS_MUTEX_WAIT); - - /* Unlock the mutex structure: */ - THR_LOCK_RELEASE(curthread, &(*m)->m_lock); - - /* Schedule the next thread: */ - _thr_sched_switch_unlocked(curthread); - - if (THR_IN_MUTEXQ(curthread)) { - THR_LOCK_ACQUIRE(curthread, &(*m)->m_lock); - mutex_queue_remove(*m, curthread); - THR_LOCK_RELEASE(curthread, &(*m)->m_lock); - } - /* - * Only clear these after assuring the - * thread is dequeued. - */ - curthread->data.mutex = NULL; - curthread->sigbackout = NULL; - } - break; - /* POSIX priority inheritence mutex: */ case PTHREAD_PRIO_INHERIT: /* Check if this mutex is not locked: */ @@ -907,11 +876,8 @@ int ret; curthread = _get_curthread(); - if ((ret = _pthread_mutex_lock(m)) == 0) { - THR_LOCK_ACQUIRE(curthread, &(*m)->m_lock); + if ((ret = _pthread_mutex_lock(m)) == 0) (*m)->m_refcount--; - THR_LOCK_RELEASE(curthread, &(*m)->m_lock); - } return (ret); } @@ -994,13 +960,7 @@ if (m == NULL || *m == NULL) ret = EINVAL; else { - /* Lock the mutex structure: */ - THR_LOCK_ACQUIRE(curthread, &(*m)->m_lock); - - /* Process according to mutex type: */ - switch ((*m)->m_protocol) { - /* Default POSIX mutex: */ - case PTHREAD_PRIO_NONE: + if ((*m)->m_protocol == PTHREAD_PRIO_NONE) { /* * Check if the running thread is not the owner of the * mutex: @@ -1008,30 +968,48 @@ if ((*m)->m_owner != curthread) ret = EPERM; else if (((*m)->m_type == PTHREAD_MUTEX_RECURSIVE) && - ((*m)->m_count > 0)) + ((*m)->m_count > 0)) { /* Decrement the count: */ (*m)->m_count--; - else { + if (add_reference) + (*m)->m_refcount++; + } else { /* * Clear the count in case this is a recursive * mutex. */ (*m)->m_count = 0; - + (*m)->m_owner = NULL; + /* + * XXX there should be a separated list + * for owned mutex, separated it from + * priority mutex list + */ +#if 0 /* Remove the mutex from the threads queue. */ MUTEX_ASSERT_IS_OWNED(*m); TAILQ_REMOVE(&(*m)->m_owner->mutexq, (*m), m_qe); MUTEX_INIT_LINK(*m); - +#endif + if (add_reference) + (*m)->m_refcount++; /* * Hand off the mutex to the next waiting * thread: */ - tid = mutex_handoff(curthread, *m); + ret = umtx_unlock(&(*m)->m_lock, + curthread->tid); + /* XXX decrease refcount if failed ? */ } - break; + return (ret); + } + + /* Lock the mutex structure: */ + THR_LOCK_ACQUIRE(curthread, &(*m)->m_lock); + /* Process according to mutex type: */ + switch ((*m)->m_protocol) { /* POSIX priority inheritence mutex: */ case PTHREAD_PRIO_INHERIT: /* ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_private.h#6 (text+ko) ==== @@ -776,7 +776,7 @@ struct pthread *_thr_alloc(struct pthread *); void _thr_exit(char *, int, char *); void _thr_exit_cleanup(void); -void _thr_mutex_reinit(struct pthread *, pthread_mutex_t *); +void _thr_mutex_reinit(pthread_mutex_t *); int _thr_ref_add(struct pthread *, struct pthread *, int); void _thr_ref_delete(struct pthread *, struct pthread *); void _thr_rtld_init(void); ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_spinlock.c#3 (text+ko) ==== @@ -129,9 +129,9 @@ int i; if (initialized != 0) { - _thr_mutex_reinit(_get_curthread(), &spinlock_static_lock); + _thr_mutex_reinit(&spinlock_static_lock); for (i = 0; i < spinlock_count; i++) - _thr_mutex_reinit(_get_curthread(), &extra[i].lock); + _thr_mutex_reinit(&extra[i].lock); } else { if (pthread_mutex_init(&spinlock_static_lock, NULL)) PANIC("Cannot initialize spinlock_static_lock");
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200412190812.iBJ8C7BI025867>