Content-Transfer-Encoding: 7bit
Content-Type: multipart/mixed; boundary="_----------=_158249806731320"
MIME-Version: 1.0
Date: Sun, 23 Feb 2020 22:47:47 +0000
From: "Tobias Nygren" <tnn@netbsd.org>
Subject: CVS commit: pkgsrc/x11/libxshmfence
To: pkgsrc-changes@NetBSD.org
Reply-To: tnn@netbsd.org
Message-Id: <20200223224747.79864FBF4@cvs.NetBSD.org>
Sender: pkgsrc-changes-owner@NetBSD.org
Precedence: bulk

This is a multi-part message in MIME format.

--_----------=_158249806731320
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="US-ASCII"

Module Name:	pkgsrc
Committed By:	tnn
Date:		Sun Feb 23 22:47:47 UTC 2020

Modified Files:
	pkgsrc/x11/libxshmfence: Makefile
	pkgsrc/x11/libxshmfence/files: xshmfence_semaphore.c
	    xshmfence_semaphore.h

Log Message:
libxshmfence: improve performance of semaphore backend. Bump rev.

It used more locking that necessary. We only need two semaphores.
One to tell waiters to wake up and one to let the last waiter that
wakes up notify xshmfence_trigger() it may now return.


To generate a diff of this commit:
cvs rdiff -u -r1.13 -r1.14 pkgsrc/x11/libxshmfence/Makefile
cvs rdiff -u -r1.7 -r1.8 pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.c
cvs rdiff -u -r1.2 -r1.3 pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


--_----------=_158249806731320
Content-Disposition: inline
Content-Length: 6663
Content-Transfer-Encoding: binary
Content-Type: text/x-diff; charset=us-ascii

Modified files:

Index: pkgsrc/x11/libxshmfence/Makefile
diff -u pkgsrc/x11/libxshmfence/Makefile:1.13 pkgsrc/x11/libxshmfence/Makefile:1.14
--- pkgsrc/x11/libxshmfence/Makefile:1.13	Sat Jan 18 23:35:54 2020
+++ pkgsrc/x11/libxshmfence/Makefile	Sun Feb 23 22:47:47 2020
@@ -1,7 +1,7 @@
-# $NetBSD: Makefile,v 1.13 2020/01/18 23:35:54 rillig Exp $
+# $NetBSD: Makefile,v 1.14 2020/02/23 22:47:47 tnn Exp $
 
 DISTNAME=	libxshmfence-1.3
-PKGREVISION=	1
+PKGREVISION=	2
 CATEGORIES=	x11
 MASTER_SITES=	${MASTER_SITE_XORG:=lib/}
 EXTRACT_SUFX=	.tar.bz2

Index: pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.c
diff -u pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.c:1.7 pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.c:1.8
--- pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.c:1.7	Sun Feb 23 18:26:59 2020
+++ pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.c	Sun Feb 23 22:47:47 2020
@@ -35,10 +35,10 @@
 
 static sem_t *mksemtemp(char *, size_t, const char *);
 
-#define LOCK() do {} while (sem_wait(f->lock) != 0)
-#define UNLOCK() do { sem_post(f->lock); } while (0)
-#define COND_WAIT() do {} while (sem_wait(f->cond) != 0)
-#define COND_SIGNAL() do { sem_post(f->cond); } while (0)
+#define COND_WAIT_W() do {} while (sem_wait(f->cond_w) != 0)
+#define COND_SIGNAL_W() do { sem_post(f->cond_w); } while (0)
+#define COND_WAIT_T() do {} while (sem_wait(f->cond_t) != 0)
+#define COND_SIGNAL_T() do { sem_post(f->cond_t); } while (0)
 
 /**
  * xshmfence_trigger:
@@ -52,30 +52,23 @@ static sem_t *mksemtemp(char *, size_t, 
 int
 xshmfence_trigger(struct xshmfence *f) {
 	int v, waiting;
-	LOCK();
 	v = __sync_bool_compare_and_swap(&f->triggered, 0, 1);
 	if (v == 0) {
 		/* already triggered */
-		UNLOCK();
 		return 0;
 	}
-	
-	waiting = __sync_fetch_and_add(&f->waiting, 0);
 
-	while (waiting > 0) {
-		COND_SIGNAL();
-		waiting--;
-	}
-
-	while (__sync_fetch_and_add(&f->waiting, 0) > 0) {
-		/*
-		 * Busy wait until they all woke up.
-		 * No new sleepers should arrive since
-		 * the lock is still held.
-		 */
-		/* yield(); */
+	while ((waiting = __sync_fetch_and_add(&f->waiting, 0)) > 0) {
+		if (__sync_bool_compare_and_swap(&f->waiting, waiting, 0)) {
+			__sync_fetch_and_add(&f->wakeups, waiting);
+			while (waiting--) {
+				COND_SIGNAL_W();
+			}
+			COND_WAIT_T();
+			return 0;
+		}
 	}
-	UNLOCK();
+
 	return 0;
 }
 
@@ -92,24 +85,18 @@ xshmfence_trigger(struct xshmfence *f) {
 int
 xshmfence_await(struct xshmfence *f) {
 
-	LOCK();
 	if (__sync_fetch_and_add(&f->triggered, 0) == 1) {
-		UNLOCK();
 		return 0;
 	}
 	do {
 		__sync_fetch_and_add(&f->waiting, 1);
-		/*
-		 * These next operations are not atomic.
-		 * But we busy-wait in xshmfence_trigger, so that's ok.
-		 */
-		UNLOCK();
-		COND_WAIT();
-		__sync_fetch_and_sub(&f->waiting, 1);
-		LOCK();
+		COND_WAIT_W();
+	} while (__sync_fetch_and_add(&f->triggered, 0) == 0);
+
+	if (__sync_sub_and_fetch(&f->wakeups, 1) == 0) {
+		COND_SIGNAL_T();
 	}
-	while (__sync_fetch_and_add(&f->triggered, 0) == 0);
-	UNLOCK();
+
 	return 0;
 }
 
@@ -121,11 +108,7 @@ xshmfence_await(struct xshmfence *f) {
  **/
 int
 xshmfence_query(struct xshmfence *f) {
-	int ret;
-	LOCK();
-	ret = __sync_fetch_and_add(&f->triggered, 0);
-	UNLOCK();
-	return ret;
+	return __sync_fetch_and_add(&f->triggered, 0);
 }
 
 /**
@@ -137,9 +120,7 @@ xshmfence_query(struct xshmfence *f) {
  **/
 void
 xshmfence_reset(struct xshmfence *f) {
-	LOCK();
 	__sync_bool_compare_and_swap(&f->triggered, 1, 0);
-	UNLOCK();
 }
 
 /**
@@ -151,27 +132,26 @@ xshmfence_reset(struct xshmfence *f) {
 void
 xshmfence_init(int fd)
 {
-	sem_t *lock;
-	sem_t *cond;
+	sem_t *cond_w, *cond_t;
 	struct xshmfence f;
 
 	__sync_fetch_and_and(&f.refcnt, 0);
 	__sync_fetch_and_and(&f.triggered, 0);
 	__sync_fetch_and_and(&f.waiting, 0);
-	
-	lock = mksemtemp(f.lockname, sizeof(f.lockname), "l");
-	if (lock == SEM_FAILED) {
+	__sync_fetch_and_and(&f.wakeups, 0);
+
+	cond_w = mksemtemp(f.condname_w, sizeof(f.condname_w), "w");
+	if (cond_w == SEM_FAILED) {
 		err(EXIT_FAILURE, "xshmfence_init: sem_open");
 	}
-
-	cond = mksemtemp(f.condname, sizeof(f.condname), "c");
-	if (cond == SEM_FAILED) {
+	cond_t = mksemtemp(f.condname_t, sizeof(f.condname_t), "t");
+	if (cond_t == SEM_FAILED) {
 		err(EXIT_FAILURE, "xshmfence_init: sem_open");
 	}
-	
-	sem_close(lock);
-	sem_close(cond);
-	
+
+	sem_close(cond_w);
+	sem_close(cond_t);
+
 	pwrite(fd, &f, sizeof(f), 0);
 }
 
@@ -187,7 +167,7 @@ xshmfence_open_semaphore(struct xshmfenc
 	/*
 	 * map process local memory to page 2
 	 */
-	if (mmap ((void*)&f->lock,
+	if (mmap ((void*)&f->cond_w,
 		  LIBXSHM_PAGESIZE,
 		  PROT_READ|PROT_WRITE,
 		  MAP_FIXED|MAP_ANON,
@@ -195,11 +175,11 @@ xshmfence_open_semaphore(struct xshmfenc
 		errx(EXIT_FAILURE, "xshmfence_open_semaphore: mmap failed");
 	}
 
-	if ((f->lock = sem_open(f->lockname, 0 , 0)) == SEM_FAILED) {
+	if ((f->cond_w = sem_open(f->condname_w, 0)) == SEM_FAILED) {
 		errx(EXIT_FAILURE, "xshmfence_open_semaphore: sem_open failed");
 	}
 
-	if ((f->cond = sem_open(f->condname, 0 , 0)) == SEM_FAILED) {
+	if ((f->cond_t = sem_open(f->condname_t, 0)) == SEM_FAILED) {
 		errx(EXIT_FAILURE, "xshmfence_open_semaphore: sem_open failed");
 	}
 	__sync_fetch_and_add(&f->refcnt, 1);
@@ -214,11 +194,11 @@ xshmfence_open_semaphore(struct xshmfenc
 void
 xshmfence_close_semaphore(struct xshmfence *f)
 {
-	sem_close(f->lock);
-	sem_close(f->cond);
+	sem_close(f->cond_w);
+	sem_close(f->cond_t);
 	if (__sync_sub_and_fetch(&f->refcnt, 1) == 0) {
-		sem_unlink(f->lockname);
-		sem_unlink(f->condname);
+		sem_unlink(f->condname_w);
+		sem_unlink(f->condname_t);
 	}
 }
 
@@ -231,7 +211,7 @@ mksemtemp(char *name, size_t namelen, co
 	for(;;) {
 		if (snprintf(name, namelen, "/xshmf%s-%d", suffix, p) >= namelen)
 			return SEM_FAILED;
-		ret = sem_open(name, O_CREAT|O_EXCL, 0600, 1);
+		ret = sem_open(name, O_CREAT|O_EXCL, 0600, 0);
 		if (ret == SEM_FAILED) {
 			if (errno == EEXIST) {
 				p++;

Index: pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.h
diff -u pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.h:1.2 pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.h:1.3
--- pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.h:1.2	Thu Sep 24 23:57:27 2015
+++ pkgsrc/x11/libxshmfence/files/xshmfence_semaphore.h	Sun Feb 23 22:47:47 2020
@@ -43,11 +43,12 @@ struct xshmfence {
 	int refcnt LOCK_ALIGN;
 	int triggered LOCK_ALIGN;
 	int waiting LOCK_ALIGN;
-	char lockname[16];
-	char condname[16];
+	int wakeups LOCK_ALIGN;
+	char condname_w[16];
+	char condname_t[16];
 	/* page 2*/
-	sem_t *lock PAGE_ALIGN;
-	sem_t *cond;
+	sem_t *cond_w PAGE_ALIGN;
+	sem_t *cond_t;
 };
 
 void


--_----------=_158249806731320--