Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 5 Jan 2005 05:05:52 GMT
From:      David Xu <davidxu@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 68291 for review
Message-ID:  <200501050505.j0555q5h081040@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=68291

Change 68291 by davidxu@davidxu_celeron on 2005/01/05 05:04:56

	with new umtx generic wait queue code, mutex and condition variable
	are not needed to implement semaphore, this also eliminates the
	       problem with signal.

Affected files ...

.. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_sem.c#4 edit

Differences ...

==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_sem.c#4 (text+ko) ====

@@ -43,11 +43,8 @@
 #include "thr_private.h"
 
 
-extern int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *);
-extern int pthread_cond_timedwait(pthread_cond_t *, pthread_mutex_t *,
-		struct timespec *);
-
 __weak_reference(_sem_init, sem_init);
+__weak_reference(_sem_trywait, sem_trywait);
 __weak_reference(_sem_wait, sem_wait);
 __weak_reference(_sem_timedwait, sem_timedwait);
 __weak_reference(_sem_post, sem_post);
@@ -65,19 +62,6 @@
 	}
 }
 
-static void
-decrease_nwaiters(void *arg)
-{
-	sem_t *sem = (sem_t *)arg;
-
-	(*sem)->nwaiters--;
-	/*
-	 * this function is called from cancellation point,
-	 * the mutex should already be hold.
-	 */
-	_pthread_mutex_unlock(&(*sem)->lock);
-}
-
 static sem_t
 sem_alloc(unsigned int value, semid_t semid, int system_sem)
 {
@@ -93,23 +77,11 @@
 		errno = ENOSPC;
 		return (NULL);
 	}
-
 	/*
-	 * Initialize the semaphore.
+	 * Fortunatly count and nwaiters are adjacency, so we can
+	 * use umtx_wait to wait on it, umtx_wait needs an address
+	 * can be accessed as a long interger.
 	 */
-	if (_pthread_mutex_init(&sem->lock, NULL) != 0) {
-		free(sem);
-		errno = ENOSPC;
-		return (NULL);
-	}
-
-	if (_pthread_cond_init(&sem->gtzero, NULL) != 0) {
-		_pthread_mutex_destroy(&sem->lock);
-		free(sem);
-		errno = ENOSPC;
-		return (NULL);
-	}
-
 	sem->count = (u_int32_t)value;
 	sem->nwaiters = 0;
 	sem->magic = SEM_MAGIC;
@@ -137,11 +109,29 @@
 }
 
 int
+_sem_trywait(sem_t *sem)
+{
+	int val;
+
+	if (sem_check_validity(sem) != 0)
+		return (-1);
+
+	if ((*sem)->syssem != 0)
+ 		return (ksem_trywait((*sem)->semid));
+
+	if ((val = (*sem)->count) > 0) {
+		if (atomic_cmpset_acq_int(&(*sem)->count, val, val - 1))
+			return (0);
+	}
+	errno = EAGAIN;
+	return (-1);
+}
+
+int
 _sem_wait(sem_t *sem)
 {
 	struct pthread *curthread;
-	int oldcancel;
-	int retval;
+	int val, oldcancel, retval;
 
 	if (sem_check_validity(sem) != 0)
 		return (-1);
@@ -151,114 +141,84 @@
 		oldcancel = _thr_cancel_enter(curthread);
 		retval = ksem_wait((*sem)->semid);
 		_thr_cancel_leave(curthread, oldcancel);
+		return (retval);
 	}
-	else {
-		pthread_testcancel();
-		_pthread_mutex_lock(&(*sem)->lock);
 
-		while ((*sem)->count <= 0) {
-			(*sem)->nwaiters++;
-			THR_CLEANUP_PUSH(curthread, decrease_nwaiters, sem);
-			pthread_cond_wait(&(*sem)->gtzero, &(*sem)->lock);
-			THR_CLEANUP_POP(curthread, 0);
-			(*sem)->nwaiters--;
+	pthread_testcancel();
+	do {
+		while ((val = (*sem)->count) > 0) {
+		    if (atomic_cmpset_acq_int(&(*sem)->count, val, val - 1))
+			return (0);
 		}
-		(*sem)->count--;
-
-		_pthread_mutex_unlock(&(*sem)->lock);
-
-		retval = 0;
-	}
-	return (retval);
+		oldcancel = _thr_cancel_enter(curthread);
+		retval = umtx_wait((struct umtx *)&(*sem)->count, 0);
+		_thr_cancel_leave(curthread, oldcancel);
+	} while (retval == 0);
+	errno = retval;
+	return (-1);
 }
 
 int
-_sem_timedwait(sem_t * __restrict sem,
-    struct timespec * __restrict abs_timeout)
+_sem_timedwait(sem_t * __restrict sem, struct timespec * __restrict abstime)
 {
 	struct pthread *curthread;
-	int retval;
-	int oldcancel;
-	int timeout_invalid;
+	int val, oldcancel, retval;
 
 	if (sem_check_validity(sem) != 0)
 		return (-1);
 
+	curthread = _get_curthread();
 	if ((*sem)->syssem != 0) {
-		curthread = _get_curthread();
 		oldcancel = _thr_cancel_enter(curthread);
-		retval = ksem_timedwait((*sem)->semid, abs_timeout);
+		retval = ksem_timedwait((*sem)->semid, abstime);
 		_thr_cancel_leave(curthread, oldcancel);
+		return (retval);
 	}
-	else {
-		/*
-		 * The timeout argument is only supposed to
-		 * be checked if the thread would have blocked.
-		 * This is checked outside of the lock so a
-		 * segfault on an invalid address doesn't end
-		 * up leaving the mutex locked.
-		 */
-		pthread_testcancel();
-		timeout_invalid = (abs_timeout->tv_nsec >= 1000000000) ||
-		    (abs_timeout->tv_nsec < 0);
-		_pthread_mutex_lock(&(*sem)->lock);
 
-		if ((*sem)->count <= 0) {
-			if (timeout_invalid) {
-				_pthread_mutex_unlock(&(*sem)->lock);
-				errno = EINVAL;
-				return (-1);
-			}
-			(*sem)->nwaiters++;
-			pthread_cleanup_push(decrease_nwaiters, sem);
-			pthread_cond_timedwait(&(*sem)->gtzero,
-			    &(*sem)->lock, abs_timeout);
-			pthread_cleanup_pop(0);
-			(*sem)->nwaiters--;
+	/*
+	 * The timeout argument is only supposed to
+	 * be checked if the thread would have blocked.
+	 */
+	pthread_testcancel();
+	do {
+		while ((val = (*sem)->count) > 0) {
+		    if (atomic_cmpset_acq_int(&(*sem)->count, val, val-1))
+			return (0);
 		}
-		if ((*sem)->count == 0) {
-			errno = ETIMEDOUT;
-			retval = -1;
+		if (abstime == NULL) {
+			errno = EINVAL;
+			return (-1);
 		}
-		else {
-			(*sem)->count--;
-			retval = 0;
-		}	
-
-		_pthread_mutex_unlock(&(*sem)->lock);
-	}
-
-	return (retval);
+		oldcancel = _thr_cancel_enter(curthread);
+		retval = umtx_timedwait((struct umtx *)&(*sem)->count, 0, abstime);
+		_thr_cancel_leave(curthread, oldcancel);
+	} while (retval == 0);
+	if (retval == EAGAIN)
+		retval = ETIMEDOUT;
+	errno = retval;
+	return (-1);
 }
 
 int
 _sem_post(sem_t *sem)
 {
-	struct pthread *curthread;
-	int retval;
+	int val, retval;
 	
 	if (sem_check_validity(sem) != 0)
 		return (-1);
 
 	if ((*sem)->syssem != 0)
-		retval = ksem_post((*sem)->semid);
-	else {
-		/*
-		 * sem_post() is required to be safe to call from within
-		 * signal handlers.  Thus, we must enter a critical region.
-		 */
-		curthread = _get_curthread();
-		_thr_critical_enter(curthread);
-		_pthread_mutex_lock(&(*sem)->lock);
+		return (ksem_post((*sem)->semid));
 
-		(*sem)->count++;
-		if ((*sem)->nwaiters > 0)
-			_pthread_cond_signal(&(*sem)->gtzero);
-
-		_pthread_mutex_unlock(&(*sem)->lock);
-		_thr_critical_leave(curthread);
+	/*
+	 * sem_post() is required to be safe to call from within
+	 * signal handlers, these code should work as that.
+	 */
+	do {
+		val = (*sem)->count;
+	} while (!atomic_cmpset_acq_int(&(*sem)->count, val, val + 1));
+	retval = umtx_wake((struct umtx *)&(*sem)->count, val + 1);
+	if (retval > 0)
 		retval = 0;
-	}
-
 	return (retval);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200501050505.j0555q5h081040>