From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/6014 Path: news.gmane.org!not-for-mail From: Jens Gustedt Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH 7/8] add the thrd_xxxxxx functions Date: Sun, 31 Aug 2014 15:19:21 +0200 Message-ID: <1409491161.4476.283.camel@eris.loria.fr> 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> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-OYLLFMd8iaX6x+L/oNQR" X-Trace: ger.gmane.org 1409491170 11307 80.91.229.3 (31 Aug 2014 13:19:30 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 31 Aug 2014 13:19:30 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-6021-gllmg-musl=m.gmane.org@lists.openwall.com Sun Aug 31 15:19:25 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 1XO52T-0007Uh-6w for gllmg-musl@plane.gmane.org; Sun, 31 Aug 2014 15:19:25 +0200 Original-Received: (qmail 19990 invoked by uid 550); 31 Aug 2014 13:19:24 -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 19973 invoked from network); 31 Aug 2014 13:19:24 -0000 X-IronPort-AV: E=Sophos;i="5.04,436,1406584800"; d="scan'";a="92364780" In-Reply-To: <20140831125745.GW12888@brightrain.aerifal.cx> Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwCAYAAABXAvmHAAAABHNCSVQICAgIfAhkiAAAEb9JREFUaN7VmXuQ3lV5xz/n/O7v7333fffdW3azSTY3Qi6wGy4aEc0qWlGhWcAq7WiDrWNpO61paS29EpzWaevYpvfL2JK0VRBbBxQtg0U2BRGKhIAI4ZKwSTbZ+3u//G7nnP7BtGM7UyVAnfb56/xznuf7nXnOnOf5fuH/eYjXI8mDn/5k8ch90xNj+eKka8xkI1MTGmY6Rk/XpD5w091fPvnf7xz8sQ/sLTQ6k54SY2mW1mQY3DVX8Kdv/PvPnfyBE7hn797bxBPP3NBv28RAqgyzJqNgJIs5q7bhHW+94Yd+5/fuBrjj5l/cs/LUs/vNiVMTo5aNJQX9lks3iaivHqlt/9D7xs778Y/Uf6AE/uWa6x7wT52ZTDyLKEkoOHmW4oSaSDFxQmq7tHPuDJ5TUkv10iAGgSLvediZotf16HS6KG1RuXDT/g99/vO3vtLa8vUgELuCVAja0mVJKRaylGbgYnwP27bImYRcozkmKrWS53vYjosfhNSUIjXQUoq2K0ksRdZqTZ5Lbfs/Dl/66E/uSebmb8hF8URab48JS+IPDVJtto9uuHj8wKW//6lD/1OSznxtLEPRv2kdPH2MSBpkqlDGkLoOIgGkIWcEiTYsk1JSDpGUFAAcBzfRCKXxV+ql7/XWnn7uxcm3vOGS2rrR1UcLV+6pC4B//tjPjGdHjhwttFOEcMmMAgFeLkezWsOTkmigfyYZGTzYN7HtwFt u+rU6wJnprxQf/JvP7beffH5fBIwGIZqEutJUlCKSNoFjYbQiihOkFmih0FJgSUGIpGxstE6RwiLQmsyzcXZesO8N1773YOHKPfUv/uxP726ePjsVNVoT9WZjMq02kRpEKay9+UPXjQmAL1991UuDZ+fGQulSVQrpuqg4pWkkupSjVGshtGZJpzQ9DysfQqZIkgzR7 jAc+qAFrm2xTIRJJKnr0okTfCnQKLRlo9MUicGWL7dcnCnytoOTKTxpERpBv8moomm7AZFjEddb9EiHdhJhPJdYanQKUZrivmH7Pnlm+itFuVgdi7WkpSSJdFlqRwRugLAkQakXJQ06cAhcB0tnRI0mstOllKYEvk8qJJGj8IygID16hwZBGIzIsIXCRREq6DGSsu9RkBYBBldKbCFwbMnA2mH8wKIrbQrGY1WkKbQ6DFkWOQEicMks8JTEQRJI0LOLJbvxwosTMlMk0kYbjTCGci4gzrr0Sod0YQFHWiSuRTHMkbRaCCxMZlh2FLGQLEjBmHFIhUBrEI0OlgYnyNN0LFKliYxGeT69rouqtOjP5+lLEtxugosgOjNHj7ZRGmwjaDoGX9skKEQQIFptkAIjIWckCQIrE9ija8aOznk5wiwh9lw8aeMlMamUeEKis4wOFiaFRtpEYxBJSoQA4ZM4ksgSPG4E9Siibjt02xUaUQsnzFFfSZlPM4TRlPp7yQWCs/PzlMs53HrMRLGPt/f2UFYRbkOzbGm6JkPgYjAgbVo6I/Ntup0unuPgWAK0RaLVy//AfVe864lgcWkicz1yloNvoJMmpFLTsiCJNIFt0VERHdcnGyxPW5598BmlJo6tVPc9v1RB2CHdqMWOYpkLCwHnhyHVTHNcpTxUrVCp1rAtyYZ1w0i3SLFk89A3/g2UZH3O4ubL30RxsU5ldhkpDFJIbGFRE4rYSLQQRHGEJQQeFm7gYHaev98CuO6NF62q1hqTKktJlKarNW0JDZUgHA9dys/Y q4emnQ3Df3nxj7z7+t2f+pO/Ysv5U0dm525uKYty3wCetPjRdYNcjaaQabq1Oq1MkVOKkc1reGFhiWIuj7C6JGlKyZNsNor3n7eFStLlkcoKz61fvX/79u3T9cWliTBWvrQEHa0QQmIZgdIK27FQmaKVxIRbN+77z5/4sT/+1PjJEycmosWVmXJvP3YpZM3aNYyuWz dTuHLPf5lPrnr3VXtPnDp10HNDlIqROEwO9fG+VosXspQ7lyto3+fE8ln2rlpDfniQP332WRCCd75tB43jZ7jUDRlspTiugxAeR+I2960s0hBy3503fmT69FcfONian5uQliDIF+m2YyKlUUIRxRF968eOfuC+r+4851Hill//+PiRx79ztNrs0mm0KZaLZEnGWwsB725WOFRv8HAmSDodSo7D3oF+bMfirxbOYkKHQDhc5lusznxaaO6fO8O2fIE9vf1M0+Go0jM73nT55EcvGa8duedr+3S1si8v7VLnzDxRpkjCHGJ4aHr1utVT7/mzP6/b5wJ++v57izf9wi8dVNrHcT3yxQK27RJ3Y6zM0LI1b+0ZoFldIvFsxvsH6LMELQOpkST1GBVq3NIAvvC54ztPcTwRLOs2b+sf4u2RS5JFY1++5yt3XbXnvZPX3/n5W4FbH/nLA+ueuOeBG/zefG3ynW87+N3D3jkRuP3226fiVEwkOiV0bNIsJep28QOfqOAjl2DEElw1WCZKFGUtkK6h7fj4DpRKfURJjeGwSE/UJcJga4d6p0lHCoo64w1BkfsqtYnf/LVPHgSuAdh1476TwMsD3t9/9jUMc9pMSiGxJRgV0+22Wa7XqTfqHH7pON8u9/NE6BB5PnZPD42BkMe14IG4gVXIsVSvkijFU2fPIrOUraPrsD3Jlt4SnTQlsi2qtqLSarG0VJnaObHztu8HyTqX9vn6Aw/cMTw0TJpkeH6AhaIncCn3FkmFzWNzc3y7U+ObjTqPVJs8Vq1h1oScabdoV xWxyQg9l57BATZZDpuCgH5PMrVxM3G3g513WfNDW3liYRGsPN1ua2LHtq0zM6dOPfmqCDx15227/+ILd58EaNa717/00smp3bsvo7fcS3VpGTcAKV2q1QZJ3MVIC88JsFJN2c+xbrDM+aHPlqE+ji0vY7kWvaFhaaXJzs3nYaoNNq/tJ1/tsiwtnpEOtz/+HHNLMXG UkmYp7XarVKlWD53TQtO89+7iN//stoPWqdkpPx/Wur3F6WccZ+KFdnuskXNZaCVkiWJ++Qy+59EbuHi6y1t2XkL27Ak2CJssS6lHMYPCYDLDP/dJvj1fQ0qb+WoMqk3BzrFlYAidc5hZXAQnh9DQ6TbRQhF3U7Is4eZf+YWJj9308Se/5z7w3fH8s8fGnNOnp1ZJie52S+VOd2oIeJObo2Vs5i2JWZWn1e/TrjQZK/WSiyPCE2eImh1S12PJaEJtYbsCN81oL7eROPQVodVI6ZiA1MvxQhyhOxGWlcOkKUHg4aQOaaoQ0hDFXf7pi3dPAk+e00r56HuurubnzpZsY5NKiYMhwUagcY1CCINB0pACIQXLoaBHWdTbXbTwWHEhHwQE9SbVgRJ/Mz/Hi4sr7H3fOIuVJvc+OEsYFnG8AKETGs0WljBYtk2j3nkZmRDoqMPqdSNHHztyZOc5rZTpQHlfIh2MpdDSgAZbZ/hGgzRIaSEF9AiNLyWylRFbNh0bfKEZsCxkK2JxZJi/Pv0CJyornLd5DQ8++hxHv7NIEFg0W00ajRqNRh3XtciUot5okBlFZgzSssm04fjxExN/+kcHxs+JwOWHDh06u3bkQNV1a7ZSIDWehEQoUgEaQGtMlpHlLXp6XPLdjFyqwBMsAvfR5D4Z0fFKbNqwgWajgnQHWF6K8V0H35EIrQFodWKSzGAJQc63sS2JylKkZaO1ZnTNqplXpUo07727+NV/vPuAv3D2hrWpRtYaZJbEwmBlINf00144S35kgMZsxEpgc0zEfL3Spu 1LklihlU2u6NBtNZDaohXHIDKadYWRLsZkZFhgNCaLcB2XzECWalAZhR6v9vzx472v+BF/dxSu3FPf9cY3z0TNLgO+YFNfHzozJHFE4CvW1CPGBvtYabQ5YQuON+aI7YCqUTSX2hgTUeop025IstSiv1wiUQu4fkiz2SBTHaSQ9AQOaZqBk6MTdzFGYAnB0FAvWjD9 fVWJ7xUjI+XJ54/NMNeVLM3XMEZjWRYGzf2dWUp5kJZNmlmE+RDdaXHpznFOzc6xOL8AKIqlArVaTKdTQSvD4sIifb29KOOgshaOJal0UzSgjcQYyNKU4zMzXHTR+F2vSRdKOm3CnIvnO4QFn77ePL7vErVaqLhN4OUYcGHL+j7iVoeBoX5mT5+mtjzHeRtLDA2UWFqYx3c8Ot2MocE875y8iMCHLElIogytDVJKjFIYLUiiCKVihkeGD379Xx869JoICGHj2C4qU2ASFqsLNFptvMBj9ZoSlzgen7hiFz919QQDoWB+9gwqU4SFXk6eXGHN6kH6e21Wls7SX3Zot5t889HvUKlFL8ssJkOgAIXv2+gswqiYjRvHjv7qr+zb94qEre8Vnu/juRlKCTJlgQhBxdRaTVb1lNh7yRb41tOUR99G72Afza6h0e7gezZWOMzRY7NIZVHoyVOtxli2hSUEaarJlwLqOqOTaoTMYVmagXKOQnHk6O/+7u9MTl5xZf01D3NrR0dK7W58pe/naHS7SB1z7fg6fm58I2/OuRSencH2HIIffjNzLRhfv5ZGo85itU6aSRr1Jq6laEcJjcgwWA7YMOoS5B2WKx38oEAUJURRhko1nm9P/8EffvrK7wf+FbfQjT/zUwdzoTvTjVv4UlK0XHZs3UTZd1mbaqyiCwKc02d4v51ycW2Zy9Zvw+AQ5DxW9RcoFwRDZZ9VA2UAdu26iD3vupQLto5SrXSwpIPjKsbWr+aDH/zg/lcC/pzU6Y98eO/4iReem67WOyU/6KFdX2S kELLRU3y0d5Cg1aEZBqSxQJdtprXL5546guXm6QtCwmKOTrvC2NoSq8tw8fYSDx9t8tATVSqNOo16lzdech7Ndrz/a4e/9fqr05+57dCTW7edtz/fE2JIySyHlxqKr8+3OFXooykND8ocf1vwuD1WHO1G5AKPUi5EYVGpLeA4OcqhZtVASKtjkLZFpjM6nZjNG0dnN p63fepcwL8qf+BdV+x+YGmxNimRtNOUJI3Z1APX7NjBPxx5ntjLQRLTTBKGSz4jwz3UajWGhgap1uqcPFXh0ovX88zTz1Mo9tJNQdiF6d273zh1y29/un6ueM7ZH7hw24apfGDPJN02ri0phXlUaZi/e3qGSpziOgKjI8qhS73eQKuUIBAsLS3i+x5DQ0W6zRUyY1hpiNrAwND+PXve+6rAv2qH5rdv+dXxr933tbtWlqpj/cOjkAkWaosUHUW5UGBmYZF1AwM0WjUKvX2g25TLJZ56dh7Pztg5vmFm8vIL9udHLrtr6trrXxXw12wxTd9/b/ETt95y15lTC5Pbtu2gUVkgaTeItSJKDI4Fo6t66cZtbNuiUu8QlvpnVq0auuGzd3758P8Jl3L6/nuLf3Tg05OWEJOFfH4iiuLJOMlYXmnSrDURaIxJEFJzwYXb9x+640u38v8hPvazP717+5atZtvmrWbN6vVm9dBqc83VP7znf6OW/Xok+a2b941XG9HY7KmTE81me/Khww+P5XMFkszQiFIKPQEQTwF3/58zui+98IK9KA4W8gVOn10kF4Tk8z7SdWl2I6JWA0fE9A+VqdXq0x/+iQ/f8PM3/cbJ14vAa7JZr3rXFR+bn188aFk2veUCxXIPQeiDkdQbbXKuZP3qIju3rcVKm7z9su2TTxy+a+Yzn3z/La8XAevVXrzu2j3jBnlXrVonVSmtbpdms40AwlyeTGeEtk2r00HYLhdsXc/OrR4XbVaU/Mbkb930nqme0rrowUefffIHTuC6a/fsPfbM C3d0WolfKhTp6clRyhcg04xtGCHt1Dh/rExRamYWGyxXa7xn1yYuvkAwP7fClg0Zhx+qr/rM7d+Yesc73nHD5OW7Zh478tRzPxACH/iRa245duzkAdsJfMuy8aXm/FUFlpeWWOm2WZidI9WapaUlFIaTlQ4jfR6/vO9D2NE32LTeYnZOUmnA6PAohx9+qnTy1NnrP/ Cj15cu23XZI4/826Px/xqB37z547sPH374oOvnKTsOb+1zuCoImZARM8ZiLlKkXcWOcoFRJ+Do/CwFO4e2DEe+9Ti+ZXA8ny/cU+X4bMKRZxZIVEiqXI6/+NKuUzPHb7z40ovnX3zx+Ctuq38HyuqWG7Tu+A0AAAAASUVORK5CYII= X-Mailer: Evolution 3.4.4-3 Xref: news.gmane.org gmane.linux.lib.musl.general:6014 Archived-At: --=-OYLLFMd8iaX6x+L/oNQR Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: quoted-printable Am Sonntag, den 31.08.2014, 08:57 -0400 schrieb Rich Felker: > On Sun, Aug 31, 2014 at 09:57:34AM +0200, Jens Gustedt wrote: > > > > if (attrp) attr =3D *attrp; > > > > =20 > > > > __acquire_ptc(); > > > > @@ -234,7 +253,10 @@ static int __pthread_create(pthread_t *restric= t res, const pthread_attr_t *restr > > > > new->canary =3D self->canary; > > > > =20 > > > > a_inc(&libc.threads_minus_1); > > > > - ret =3D __clone(start, stack, flags, new, &new->tid, TP_ADJ(new),= &new->tid); > > > > + if (c11) > > > > + ret =3D __clone(start_c11, stack, flags, new, &new->tid, TP_ADJ(= new), &new->tid); > > > > + else > > > > + ret =3D __clone(start, stack, flags, new, &new->tid, TP_ADJ(new)= , &new->tid); > > >=20 > > > Couldn't this be "c11 ? start_c11 : start" to avoid duplicating the > > > rest of the call? > >=20 > > I think that the ternary expression together with the other > > parenthesized paramenter and the length of the line would make the > > line barely readable. > >=20 > > This are some of the pivots lines of the implementation, I'd rather > > have them stick out. > >=20 > > Also the assembler that is produced should be identical. >=20 > Whether or not the output is identical, your code is much less > readable and maintainable: an active effort is required by the reader > to determine that the only difference between the two code paths is > the function pointer being passed. This is why I prefer the use of ?:. >=20 > The following looks perfectly readable to me: >=20 > ret =3D __clone(c11 ? start_c11 : start, stack, flags, > new, &new->tid, TP_ADJ(new), &new->tid); No to me (seriously!) because my builtin parser has difficulties to capture the end of the ternary expression. Would it be acceptable to have parenthesis around? > > > Also, since it doesn't depend on anything in pthread_create.c, > >=20 > > It does, __THRD_C11 :) >=20 > 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. > > > it would be nice to put it in a separate thrd_create.c. It's not a bi= g > > > deal but it shaves off a small function in POSIX programs that don't > > > use thrd_create. > >=20 > > I'd really prefer to keep all the logic of thrd_create together in one > > file. All the other additions at this point are tightly bound to be in > > the same TU as pthread_create, for all this weak symbol stuff that is > > going on with tsd and friends. > >=20 > > Also see that as an incentive to accept patch 8/8 to separate both > > implementations more cleanly :) >=20 > Yes I'm aware, but I don't want gratuitous incentives for patch 8/8 > just because patch 7/8 is done intentionally suboptimally. :-) If the > approaches are being compared, we should be comparing the preferred > efficient versions of both. And I'd like to wait to seriously think > about 8/8 until everything else is fully ready to commit, or > preferably already committed. I know. 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'd really just prefer that all of these can't-fail cases be a > > > straight tail call with no support for nonzero thrd_success values. > > > But as long as the code is correct and the inefficiency is trivially > > > optimized out, I'm not going to spend a lot of time arguing about it. > > > I do think it's telling, though, that the (albeit minimal) complexity > > > of trying to handle the case where thrd_success is nonzero seems to > > > have caused a couple bugs -- this is part of why I don't like having > > > multiple code paths where one path is untestable/untested. To me, a > > > code path that's never going to get tested is a much bigger offense > > > than an assumption that a constant has the value we decided it has. >=20 > Do you have any thoughts on this part? ah, I should have deleted that in my reply, since I basically agree. In the new version that I am preparing there are tail calls with corresponding comments. > > > > diff --git a/src/thread/thrd_join.c b/src/thread/thrd_join.c > > > > new file mode 100644 > > > > index 0000000..a8c7aed > > > > --- /dev/null > > > > +++ b/src/thread/thrd_join.c > > > > @@ -0,0 +1,16 @@ > > > > +#include "pthread_impl.h" > > > > +#include > > > > +#include > > > > + > > > > +int __munmap(void *, size_t); > > > > + > > > > +/* C11 threads cannot be canceled, so there is no need for a > > > > + cancelation function pointer, here. */ > > > > +int thrd_join(thrd_t t, int *res) > > > > +{ > > > > + int tmp; > > > > + while ((tmp =3D t->tid)) __timedwait(&t->tid, tmp, 0, 0, 0, 0, 0)= ; > > > > + if (res) *res =3D (int)(intptr_t)t->result; > > > > + if (t->map_base) __munmap(t->map_base, t->map_size); > > > > + return thrd_success; > > > > +} > > >=20 > > > I'd rather avoid duplicating this function too. > >=20 > > No this ain't a duplicate. The cast here is necessary and plain use of > > pthread_join would have us interpret an int* as void**, so the > > assignment could potentially overwrite the second half of the word res > > is pointing to. > >=20 > > I'll have that visually stick out with a comment >=20 > 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: >=20 > int thrd_join(thrd_t t, int *res) > { > void *pthread_res; > __pthread_join(t, &pthread_res); > if (res) *res =3D (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 Jens --=20 :: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS ::: :: ::::::::::::::: office Strasbourg : +33 368854536 :: :: :::::::::::::::::::::: gsm France : +33 651400183 :: :: ::::::::::::::: gsm international : +49 15737185122 :: :: http://icube-icps.unistra.fr/index.php/Jens_Gustedt :: --=-OYLLFMd8iaX6x+L/oNQR Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEABECAAYFAlQDINoACgkQD9PoadrVN+KLpQCfXA4ao2k73auOF1vkAjNAthW+ RDkAnjj7qJk7kjsJvC0uec8X39HUAVuk =UW/m -----END PGP SIGNATURE----- --=-OYLLFMd8iaX6x+L/oNQR--