From patchwork Wed Jan 4 17:20:42 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 711061 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ttyJS5Ndmz9t10 for ; Thu, 5 Jan 2017 04:20:56 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id F1C98BF0; Wed, 4 Jan 2017 17:20:53 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id CB750A6E for ; Wed, 4 Jan 2017 17:20:52 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 054E6127 for ; Wed, 4 Jan 2017 17:20:51 +0000 (UTC) Received: from mfilter31-d.gandi.net (mfilter31-d.gandi.net [217.70.178.162]) by relay3-d.mail.gandi.net (Postfix) with ESMTP id 519FAA80D2; Wed, 4 Jan 2017 18:20:50 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at mfilter31-d.gandi.net Received: from relay3-d.mail.gandi.net ([IPv6:::ffff:217.70.183.195]) by mfilter31-d.gandi.net (mfilter31-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id KfIgP9zPUNgH; Wed, 4 Jan 2017 18:20:48 +0100 (CET) X-Originating-IP: 173.228.112.229 Received: from ovn.org (173-228-112-229.dsl.dynamic.fusionbroadband.com [173.228.112.229]) (Authenticated sender: blp@ovn.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 23D20A8106; Wed, 4 Jan 2017 18:20:46 +0100 (CET) Date: Wed, 4 Jan 2017 09:20:42 -0800 From: Ben Pfaff To: Alin Serdean Message-ID: <20170104172042.GP2016@ovn.org> References: <20161228222705.16212-1-aserdean@cloudbasesolutions.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161228222705.16212-1-aserdean@cloudbasesolutions.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: "dev@openvswitch.org" Subject: Re: [ovs-dev] [PATCH] Windows tests: Applications abort when using threading (ovs_rwlock_init) X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org On Wed, Dec 28, 2016 at 10:27:17PM +0000, Alin Serdean wrote: > Commit number: 1a15f390afd66efd161db78b86600832214dfb3c > https://github.com/openvswitch/ovs/commit/1a15f390afd66efd161db78b86600832214dfb3c > switched from `NULL` to `attr`. According to POSIX documentation this is > correct, unfortunately on Windows the current implementation of pthreads does > not support a pre-initialized attribute. Please see a fork of the implementation > https://github.com/GerHobbelt/pthread-win32/blob/19fd5054b29af1b4e3b3278bfffbb6274c6c89f5/pthread_rwlock_init.c#L59-L63 > This is the same implementation as the official version found under: > ftp://sourceware.org/pub/pthreads-win32/) Thanks. I didn't like that this was Windows-specific, since it was possible to make it generic. I changed the patch to the following, and applied it to master. --8<--------------------------cut here-------------------------->8-- From: Alin Serdean Date: Wed, 28 Dec 2016 22:27:17 +0000 Subject: [PATCH] ovs-thread: Avoid pthread_rwlockattr_t on Windows. A recent commit fixed ovs_rwlock_init() to pass the pthread_rwlockattr_t that it initialized to pthread_rwlock_init(). According to POSIX documentation this is correct, but on Windows the current implementation of pthreads does not support a pre-initialized attribute. Please see a fork of the implementation https://github.com/GerHobbelt/pthread-win32/blob/19fd5054b29af1b4e3b3278bfffbb6274c6c89f5/pthread_rwlock_init.c#L59-L63 This is the same implementation as the official version found under: ftp://sourceware.org/pub/pthreads-win32/) A short debug output from `vswitch` to confirm the above: >k Index Function -------------------------------------------------------------------------------- *1 ovs-vswitchd.exe!ovs_rwlock_init(const ovs_rwlock * l_=0x000001721c7da250) 2 ovs-vswitchd.exe!open_dpif_backer(const char * type=0x000001721c7d8d60, dpif_backer * * backerp=0x000001721c7d89c0) 3 ovs-vswitchd.exe!construct(ofproto * ofproto_=0x000001721c7d87d0) 4 ovs-vswitchd.exe!ofproto_create(const char * datapath_name=0x000001721c7d86e0, const char * datapath_type=0x000001721c7d8750, ofproto * * ofprotop=0x000001721c7d80b8) 5 ovs-vswitchd.exe!bridge_reconfigure(const ovsrec_open_vswitch * ovs_cfg=0x000001721c7e05b0) 6 ovs-vswitchd.exe!bridge_run() 7 ovs-vswitchd.exe!main(int argc=6, char * * argv=0x000001721c729e10) 8 [External Code] >? error 22 https://github.com/openvswitch/ovs/blob/master/lib/ovs-thread.c#L243 This patch is critical because the majority (over 800) of the unit tests are failing. Fixes: 1a15f390afd6 ("lib/ovs-thread: set prefer writer lock for ovs_rwlock_init()") Signed-off-by: Alin Gabriel Serdean Acked-by: Shashank Ram [blp@ovn.org changed the details of the approach] Signed-off-by: Ben Pfaff --- lib/ovs-thread.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c index 058c434..1f4995b 100644 --- a/lib/ovs-thread.c +++ b/lib/ovs-thread.c @@ -230,21 +230,27 @@ void ovs_rwlock_init(const struct ovs_rwlock *l_) { struct ovs_rwlock *l = CONST_CAST(struct ovs_rwlock *, l_); - pthread_rwlockattr_t attr; int error; l->where = ""; - xpthread_rwlockattr_init(&attr); #ifdef PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP + pthread_rwlockattr_t attr; + xpthread_rwlockattr_init(&attr); xpthread_rwlockattr_setkind_np( &attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP); -#endif error = pthread_rwlock_init(&l->lock, &attr); + xpthread_rwlockattr_destroy(&attr); +#else + /* It is important to avoid passing a rwlockattr in this case because + * Windows pthreads 2.9.1 (and earlier) fail and abort if passed one, even + * one without any special attributes. */ + error = pthread_rwlock_init(&l->lock, NULL); +#endif + if (OVS_UNLIKELY(error)) { ovs_abort(error, "pthread_rwlock_init failed"); } - xpthread_rwlockattr_destroy(&attr); } /* Provides an error-checking wrapper around pthread_cond_wait().