From owner-svn-src-all@freebsd.org Sat Jun 25 18:36:55 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5B845B81EFC; Sat, 25 Jun 2016 18:36:55 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from mail.netplex.net (mail.netplex.net [204.213.176.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.netplex.net", Issuer "RapidSSL SHA256 CA - G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 187EE2A06; Sat, 25 Jun 2016 18:36:54 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from sea.ntplx.net (sea.ntplx.net [204.213.176.11]) by mail.netplex.net (8.15.1/8.15.1/NETPLEX) with ESMTP id u5PIaqua011898; Sat, 25 Jun 2016 14:36:52 -0400 X-Virus-Scanned: by AMaViS and Clam AntiVirus (mail.netplex.net) X-Greylist: Message whitelisted by DRAC access database, not delayed by milter-greylist-4.4.3 (mail.netplex.net [204.213.176.9]); Sat, 25 Jun 2016 14:36:52 -0400 (EDT) Date: Sat, 25 Jun 2016 14:36:52 -0400 (EDT) From: Daniel Eischen X-X-Sender: eischen@sea.ntplx.net Reply-To: Daniel Eischen To: Konstantin Belousov cc: Jilles Tjoelker , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r302194 - head/lib/libthr/thread In-Reply-To: <20160625172956.GE38613@kib.kiev.ua> Message-ID: References: <201606251130.u5PBUeGC001988@repo.freebsd.org> <20160625171440.GA19698@stack.nl> <20160625172956.GE38613@kib.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 25 Jun 2016 18:36:55 -0000 On Sat, 25 Jun 2016, Konstantin Belousov wrote: > On Sat, Jun 25, 2016 at 07:14:40PM +0200, Jilles Tjoelker wrote: >> On Sat, Jun 25, 2016 at 11:30:40AM +0000, Konstantin Belousov wrote: >>> Author: kib >>> Date: Sat Jun 25 11:30:40 2016 >>> New Revision: 302194 >>> URL: https://svnweb.freebsd.org/changeset/base/302194 >> >>> Log: >>> For pthread_mutex_trylock() call on owned error-check or non-portable >>> adaptive mutex, return EDEADLK as required by POSIX. The >>> pthread_mutex_lock() is already compliant. >> >>> Tested by: Guy Yur >>> Sponsored by: The FreeBSD Foundation >>> MFC after: 2 weeks >>> Approved by: re (gjb) >> >>> Modified: >>> head/lib/libthr/thread/thr_mutex.c >> >>> Modified: head/lib/libthr/thread/thr_mutex.c >>> ============================================================================== >>> --- head/lib/libthr/thread/thr_mutex.c Sat Jun 25 10:08:04 2016 (r302193) >>> +++ head/lib/libthr/thread/thr_mutex.c Sat Jun 25 11:30:40 2016 (r302194) >>> @@ -850,9 +850,12 @@ mutex_self_trylock(struct pthread_mutex >>> >>> switch (PMUTEX_TYPE(m->m_flags)) { >>> case PTHREAD_MUTEX_ERRORCHECK: >>> - case PTHREAD_MUTEX_NORMAL: >>> case PTHREAD_MUTEX_ADAPTIVE_NP: >>> - ret = EBUSY; >>> + ret = EDEADLK; >>> + break; >>> + >>> + case PTHREAD_MUTEX_NORMAL: >>> + ret = EBUSY; >>> break; >>> >>> case PTHREAD_MUTEX_RECURSIVE: >> >> I think POSIX (SUSv4tc1, XSH 3 pthread_mutex_lock) is clear that only >> pthread_mutex_lock() can fail with [EDEADLK], not >> pthread_mutex_trylock(). Instead, the error [EBUSY] listed for >> pthread_mutex_trylock() applies whenever the mutex could not be acquired >> because it was already locked. This includes the case where the mutex is >> owned by the current thread. Note that POSIX intends to allow not >> storing the owning thread's ID in non-recursive non-robust mutexes. >> >> Failing pthread_mutex_trylock() on owned mutexes only with [EBUSY] also >> matches our code before this commit and NetBSD's code, and is apparently >> expected by other code. >> >> Therefore, I think this commit should be reverted. >> > > I already asked re for approval of the reversal and got it. But I am still > hesitating doing the revert vs. returning EDEADLK for error-checking > mutexes. > > My initial mistake was reading the statement about PTHREAD_MUTEX_ERRORCHECK > returning EDEADLK as the requirement for both functions. It was induced > by reading the following code in samba: > https://github.com/samba-team/samba/blob/master/lib/tdb/common/mutex.c#L928 > I did extracted this into stand-alone test and checked that glibc does > return EDEADLK in this case. BTW, if somebody has Solaris machine available > to test this, I would be grateful. Code is available at > https://www.kib.kiev.ua/kib/pshared/pthread_samba.c > > I.e., plain revert would disable the only known to me consumer of the > robust mutexes. The patch which I mailed last time, returns EDEADLK for > trylock on ERRORCHECKed mutexes only. And I am tending toward glibc > compatibility there, over the literal POSIX compliance, but I want to > see the confirmation from the Klimenko' test first. That doesn't make glibc correct. Unless the standards change, we should return EBUSY in this case. It is also unexpected if an implementation's default mutex scheme is PTHREAD_MUTEX_ERRORCHECK and it returns EDEADLK in this case. It's not mentioned in the standard, and no portable application will be expecting it. -- DE