From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/5780 Path: news.gmane.org!not-for-mail From: Timo Teras Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] private futex support Date: Fri, 8 Aug 2014 11:38:57 +0300 Message-ID: <20140808113857.1839babf@vostro> References: <20140626184803.GA8845@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1407487166 7005 80.91.229.3 (8 Aug 2014 08:39:26 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 8 Aug 2014 08:39:26 +0000 (UTC) Cc: musl@lists.openwall.com To: Rich Felker Original-X-From: musl-return-5785-gllmg-musl=m.gmane.org@lists.openwall.com Fri Aug 08 10:39:21 2014 Return-path: Envelope-to: gllmg-musl@plane.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1XFfho-0003vk-Kt for gllmg-musl@plane.gmane.org; Fri, 08 Aug 2014 10:39:20 +0200 Original-Received: (qmail 9621 invoked by uid 550); 8 Aug 2014 08:39:20 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 9613 invoked from network); 8 Aug 2014 08:39:20 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=2Norfcddoy3BtzplOJ2XY/0Pw2pdogr91C6PJxwqaQc=; b=YLPQTNdXoWN6sAarQEj/zlPAoqrs5HYns4X8d7gfhgQgSquGJpWdo22/3eH6Sf4gai m37QHAK3tqb3vkwaviVDS4ZuZg/1v1W+MA3sr0khcwiTYWJ9Cn6VvNwcyEKI70aA8Eeo cOdPHRLbWlwTkQEgAi6WBrZSPxZFYdG4gtHoUt9d18Ry291W6OsQ78vIlFB4tRz4oNho 0ILgQWNOT57WdUpEOJOPp+Y2VBbRUnhKJ59zCh9n0c7IOJeFKlC4767CPZGe4SsSReen F2J48A8kFgdnC0/YweEVvfKPigOV9tsgRu+PIlxc52C5NqViA0Ta24sLvR9ij78bnDA6 KBkg== X-Received: by 10.152.10.193 with SMTP id k1mr763409lab.79.1407487148780; Fri, 08 Aug 2014 01:39:08 -0700 (PDT) Original-Sender: =?UTF-8?Q?Timo_Ter=C3=A4s?= In-Reply-To: <20140626184803.GA8845@brightrain.aerifal.cx> X-Mailer: Claws Mail 3.10.1 (GTK+ 2.24.23; x86_64-alpine-linux-musl) Xref: news.gmane.org gmane.linux.lib.musl.general:5780 Archived-At: On Thu, 26 Jun 2014 14:48:03 -0400 Rich Felker wrote: > Here is a semi-proposed patch for private futex support, one of the > items on the roadmap for the next release cycle. However I've been > unable to demonstrate it yielding better performance than non-private > futex, and since it adds (small, but nonzero) complexity and worse > behavior on outdated (pre-2.6.22) kernels, I'm a bit hesitant to Seems the patch is not fully complete. At least pthread_cond_t is not fully using private-futexes yet. Before testing, I did add the private flag in: 1) pthread_cond_broadcast.c for the FUTEX_REQUEUE call (can likely be unconditionally added, as pshared conditions have separate code path) 2) pthread_cond_timedwait.c for the __timedwait call (though this probably needs to be based on the pshared attribute) My changes being: diff --git a/src/thread/pthread_cond_broadcast.c b/src/thread/pthread_cond_broadcast.c index 0901daf..4327a0e 100644 --- a/src/thread/pthread_cond_broadcast.c +++ b/src/thread/pthread_cond_broadcast.c @@ -27,7 +27,7 @@ int pthread_cond_broadcast(pthread_cond_t *c) /* Perform the futex requeue, waking one waiter unless we know * that the calling thread holds the mutex. */ - __syscall(SYS_futex, &c->_c_seq, FUTEX_REQUEUE, + __syscall(SYS_futex, &c->_c_seq, 128 | FUTEX_REQUEUE, !m->_m_type || (m->_m_lock&INT_MAX)!=__pthread_self()->tid, INT_MAX, &m->_m_lock); diff --git a/src/thread/pthread_cond_timedwait.c b/src/thread/pthread_cond_timedwait.c index 99d62cc..73c1781 100644 --- a/src/thread/pthread_cond_timedwait.c +++ b/src/thread/pthread_cond_timedwait.c @@ -64,7 +64,7 @@ int pthread_cond_timedwait(pthread_cond_t *restrict c, pthread_mutex_t *restrict pthread_mutex_unlock(m); - do e = __timedwait(&c->_c_seq, seq, c->_c_clock, ts, cleanup, &cm, 0); + do e = __timedwait(&c->_c_seq, seq, c->_c_clock, ts, cleanup, &cm, 1); while (c->_c_seq == seq && (!e || e==EINTR)); if (e == EINTR) e = 0; It seems that things worked even without these changes, but everything was a lot slower. Not sure why. But the code sounds buggy anyway, because FUTEX_REQUEUE was transferring waiters to &m->_m_lock which was already treated as private address in other futex calls causing a mismatch. > actually commit it. If anyone is interested in this feature, please > see if you can find some examples that demonstrate that it measurably > improves performance. And running my simple test-case of having two threads wake up each other using a condition variable, seems to yield noticeable performance speed up from private futexes. See at the end of mail for the code. The low and high numbers from few test runs are as follows from musl git 4fe57cad709fdfb377060 without and with the futex patch are as follows: ~/privfutex $ time ~/oss/musl/lib/libc.so ./test count=2516417 real 0m 2.00s user 0m 1.68s sys 0m 2.30s ~/privfutex $ time ~/oss/musl/lib/libc.so ./test count=2679381 real 0m 2.00s user 0m 1.59s sys 0m 2.39s Private futexes: ~/privfutex $ time ~/oss/musl/lib/libc.so ./test count=3839470 real 0m 2.00s user 0m 1.68s sys 0m 1.98s ~/privfutex $ time ~/oss/musl/lib/libc.so ./test count=5350852 real 0m 2.00s user 0m 1.66s sys 0m 2.32s You can see essentially lowered sys time use, and up to doubled throughput of wait/wake operations. So I suspect your test case was not measuring right thing. Private futexes speed up only specific loads, and this type of pthread_cond_t usage would probably be the pattern benefiting most. Please reconsidering adding this after addressing the found deficiencies stated in the beginning. Thanks, Timo And the test case code: #include #include #include static pthread_mutex_t mtx = PTHREAD_MUTEX_INITIALIZER; static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; static volatile long run = 1, count = 0; static void *func(void *arg) { long my_id = (long) arg; do { pthread_mutex_lock(&mtx); while (run == my_id) pthread_cond_wait(&cond, &mtx); count++; if (run) run = (my_id == 1) ? 2 : 1; pthread_cond_broadcast(&cond); pthread_mutex_unlock(&mtx); } while (run); return NULL; } int main(void) { void *ret; pthread_t t1, t2; pthread_create(&t1, NULL, func, (void*) 1); pthread_create(&t2, NULL, func, (void*) 2); sleep(2); pthread_mutex_lock(&mtx); run = 0; pthread_cond_broadcast(&cond); pthread_mutex_unlock(&mtx); pthread_join(t1, &ret); pthread_join(t2, &ret); printf("count=%ld\n", count); return 0; }