From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/6017 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH 7/8] add the thrd_xxxxxx functions Date: Sun, 31 Aug 2014 12:06:30 -0400 Message-ID: <20140831160630.GZ12888@brightrain.aerifal.cx> References: <22215ff2f880382340930f78cc746565a625a806.1409423162.git.Jens.Gustedt@inria.fr> <20140831004643.GP12888@brightrain.aerifal.cx> <1409471854.4476.272.camel@eris.loria.fr> <20140831125745.GW12888@brightrain.aerifal.cx> <1409491161.4476.283.camel@eris.loria.fr> <20140831140533.GX12888@brightrain.aerifal.cx> <1409497642.4476.289.camel@eris.loria.fr> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1409501211 2017 80.91.229.3 (31 Aug 2014 16:06:51 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 31 Aug 2014 16:06:51 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-6024-gllmg-musl=m.gmane.org@lists.openwall.com Sun Aug 31 18:06:44 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 1XO7eN-0007rP-SC for gllmg-musl@plane.gmane.org; Sun, 31 Aug 2014 18:06:43 +0200 Original-Received: (qmail 17498 invoked by uid 550); 31 Aug 2014 16:06:43 -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 17490 invoked from network); 31 Aug 2014 16:06:42 -0000 Content-Disposition: inline In-Reply-To: <1409497642.4476.289.camel@eris.loria.fr> User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:6017 Archived-At: On Sun, Aug 31, 2014 at 05:07:22PM +0200, Jens Gustedt wrote: > > > > > > Also, since it doesn't depend on anything in pthread_create.c, > > > > > > > > > > It does, __THRD_C11 :) > > > > > > > > How about naming it to __ATTRP_C11_THREAD and putting it in > > > > pthread_impl.h then? > > > > > > ok, I'll do that > > > > > > But hopefully we didn't overlook any possible use pattern that would > > > drag in different versions of the tsd symbols. I am not too > > > comfortable with that. > > > > What do you mean here? I don't follow. > > When trying to separate the two implementations, for a while I > struggled with the fact that the game with the dummy functions > enforces that pthread_exit and pthread_create must be in the same > TU. I found that difficult to deal with, not knowing the code and the > interaction between the different TU too well. > > (There might be an independent possibility of improvement in > readability and maintainability in separating pthread_create and > pthread_exit more cleanly.) pthread_create inherently pulls in pthread_exit because the start function calls the latter. The other direction is not inherent -- for example, a single-threaded program could in principle exit via a direct call to pthread_exit or cancellation of pthread_self(). Regarding the tsd weak symbols, the logic is slightly complicated. The main thread's tsd pointer can be initialized during the first pthread_create call, or by pthread_key_create; it merely needs to be valid before any call to pthread_setspecific or pthread_getspecific. I'd kind of like to get rid of the initialization path in pthread_create, but the one in pthread_key_create cannot find the main thread unless the main thread is the only thread in existence when it's called. In any case, my quick analysis of the weak symbols leads me to believe it's fine to split the ones used by pthread_exit and pthread_create into separate TUs, but I'm still not convinced this is terribly useful to do. > > > But I just discovered another such incentive :) You were right, that > > > the error handling for thrd_create was not correct for C11, but it > > > wasn't my fault :) POSIX (and thus __pthread_create) basically maps > > > all errors to EAGAIN, where C11 requires us to distinguish ENOMEM from > > > other failures. > > > > > > Thus I had to integrate this difference into __pthread_create, which > > > was not difficult, but which intrudes even a little bit more into the > > > existing code. > > > > I think POSIX's EAGAIN is fully equivalent to C11's thrd_nomem: both > > should reflect any condition where the thread could not be created due > > to a resource exhaustion type failure. While you could argue that > > thrd_nomem should only indicate failure of the mmap, not the clone, I > > think this would be a useless distinction to callers (both of them are > > fundamentally allocation operations) and then you'd be forced to use > > thrd_error for clone failures, whereas I would think thrd_error should > > be reserved for either erroneous usage (but that's UB anyway) or more > > permanent failures (like lack of thread support on the OS). > > Having read up a bit, now, I don't think so, for C threads this > mapping is not correct. clone has several different error returns > that the actual code correctly maps to EAGAIN, but among them it also > has ENOMEM. > > So we have possible ENOMEM coming from clone and from mmap, and a > bunch of other obscure errors that should go to thrd_error. Like what? I see no possible errors except EAGAIN and ENOMEM. The only others listed in the man page are EINVAL and EPERM and they pertain to invalid arguments that aren't being used by pthread_create. > > > > I'm aware that you can't cast the int* to void**, but you can still > > > > implement the function as a trivial wrapper that doesn't introduce any > > > > duplication of internal logic for cleaning up an exited thread: > > > > > > > > int thrd_join(thrd_t t, int *res) > > > > { > > > > void *pthread_res; > > > > __pthread_join(t, &pthread_res); > > > > if (res) *res = (int)(intptr_t)pthread_res; > > > > return thrd_success; > > > > } > > > > > > dunno, doesn't look much simpler to me and drags in one more TU into C > > > thread applications > > > > What's simpler is that this version does not: > > > > - Need pthread_impl.h > > - Have knowledge of the mechanism for waiting for thread exit. > > - Have knowledge of how to obtain the exit code out of thread struct. > > - Have knowledge of thread deallocation mechanism. > > Ok, I'll move that separated implementation to patch 8, and use your > version for patch 7. OK. Rich