From owner-freebsd-threads@FreeBSD.ORG Sat Sep 25 02:53:22 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id 1D3F7106564A; Sat, 25 Sep 2010 02:53:22 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: David Xu Date: Fri, 24 Sep 2010 22:53:01 -0400 User-Agent: KMail/1.6.2 References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009241943.10401.jkim@FreeBSD.org> <4C9DBFD9.5000004@freebsd.org> In-Reply-To: <4C9DBFD9.5000004@freebsd.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009242253.05383.jkim@FreeBSD.org> Cc: Daniel Eischen , freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 25 Sep 2010 02:53:22 -0000 On Saturday 25 September 2010 05:24 am, David Xu wrote: > Jung-uk Kim wrote: > > On Friday 24 September 2010 05:24 pm, John Baldwin wrote: > >> On Friday, September 24, 2010 11:37:53 am Jung-uk Kim wrote: > >>> On Friday 24 September 2010 09:26 am, John Baldwin wrote: > >>>> On Thursday, September 23, 2010 11:48:40 pm Jung-uk Kim wrote: > >>>>> On Thursday 23 September 2010 06:44 pm, Daniel Eischen wrote: > >>>>>> You shouldn't have to call pthread_mutex_init() on a mutex > >>>>>> initialized with PTHREAD_MUTEX_INITIALIZER. Our > >>>>>> implementation should auto initialize the mutex when it is > >>>>>> first used; if it doesn't, I think that is a bug. > >>>>> > >>>>> Ah, I see. I verified that libthr does it correctly. > >>>>> However, that's a hack and it is far from real static > >>>>> allocation although it should work pretty well in reality, > >>>>> IMHO. More over, it will have a side-effect, i.e., any > >>>>> destroyed mutex may be resurrected if it is used again. > >>>>> POSIX seems to say it should return EINVAL when it happens. > >>>>> > >>>>> :-( > >>>> > >>>> I think the fix there is that we should put a different value > >>>> ((void *)1 for example) into "destroyed" mutex objects than 0 > >>>> so that destroyed mutexes can be differentiated from > >>>> statically initialized mutexes. This would also allow us to > >>>> properly return EBUSY, etc. > >>> > >>> It would be nice if we had "uninitialized" as (void *)0 and > >>> "static initializer" as (void *)1. IMHO, it looks more > >>> natural, i.e., "uninitialized" or "destroyed" one gets zero, > >>> and "dynamically initialized" or "statically initialized" one > >>> gets non-zero. Can we break the ABI for 9, maybe? ;-) > >> > >> I actually find the (void *)1 more natural for a destroyed state > >> FWIW. One thing I would advocate is that regardless of the > >> values chosen, use constants for both the INITIALIZER and > >> DESTROYED values. That would make the code more obvious. In > >> general I think your patch in your followup is correct, but > >> having explicitly DESTROYED constants that you check against > >> instead of NULL would improve the code. > > > > Here goes more complicated patch. Not really tested, though. > > > > Jung-uk Kim > > The patch touches too many lines which is unnecessary to be > modified, it seems you are arbitrarily restructure the code > according to you hobby. Second, breaking ABI compatible is not > accepted, I think jhb@ is right here, destroyed mutex should have > pointer 1 or others, non-null PTHREAD_MUTEX_INITIALIZER causes data > to be put in data segment not in bss, result is large binary size, > if it can be avoided, it should be avoided. Third, inserting extra > branch in critical path should be caution, it is visible in > performance losting in past I have compared it with competitor. I > think to minimize abnormal case, I may use > if (((uintptr_t)mutex) <= 1) { > if (mutex == NULL) init it. > else if (mutex == destroyed) > return EINVAL; > } > This should have same overhead with current code. Don't get me wrong, I have no intention to commit the patch. In fact, I don't disagree with you. It was a proof-of-concept, friday night experiment and that's about it. Probably you missed smiley in the first patch. Sorry if I had wasted too much of your valuable time. :-/ Jung-uk Kim