From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/6016 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 17:07:22 +0200 Message-ID: <1409497642.4476.289.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> <1409491161.4476.283.camel@eris.loria.fr> <20140831140533.GX12888@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="=-XCDaHpBWWXO+XqyNxi6g" X-Trace: ger.gmane.org 1409497652 24342 80.91.229.3 (31 Aug 2014 15:07:32 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 31 Aug 2014 15:07:32 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-6023-gllmg-musl=m.gmane.org@lists.openwall.com Sun Aug 31 17:07:27 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 1XO6j1-0006Oi-5H for gllmg-musl@plane.gmane.org; Sun, 31 Aug 2014 17:07:27 +0200 Original-Received: (qmail 17668 invoked by uid 550); 31 Aug 2014 15:07:25 -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 17660 invoked from network); 31 Aug 2014 15:07:24 -0000 X-IronPort-AV: E=Sophos;i="5.04,436,1406584800"; d="scan'";a="92369597" In-Reply-To: <20140831140533.GX12888@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:6016 Archived-At: --=-XCDaHpBWWXO+XqyNxi6g Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: quoted-printable Am Sonntag, den 31.08.2014, 10:05 -0400 schrieb Rich Felker: > On Sun, Aug 31, 2014 at 03:19:21PM +0200, Jens Gustedt wrote: > > 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 *res= trict 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(n= ew), &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 t= he > > > > > 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); > >=20 > > No to me (seriously!) because my builtin parser has difficulties to > > capture the end of the ternary expression. > >=20 > > Would it be acceptable to have parenthesis around? >=20 > I wouldn't even mind if you'd prefer to rename start to start_pthread > and then make start a local variable: >=20 > start =3D c11 ? start_c11 : start_pthread; > ret =3D __clone(start, stack, flags, new, > &new->tid, TP_ADJ(new), &new->tid); >=20 > Would that be nicer? I opted for the parenthesis version, now. > > > > > 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? > >=20 > > ok, I'll do that > >=20 > > 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. >=20 > 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.) > > > 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. > >=20 > > I know. > >=20 > > 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. > >=20 > > 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. >=20 > 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. > > > 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 an= y > > > 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; > > > } > >=20 > > dunno, doesn't look much simpler to me and drags in one more TU into C > > thread applications >=20 > What's simpler is that this version does not: >=20 > - 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. 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 :: --=-XCDaHpBWWXO+XqyNxi6g 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) iEYEABECAAYFAlQDOioACgkQD9PoadrVN+InkwCgkqVgiBzwwQGpRTEGZFjRRQts QMcAnjyB2n+CzRX+7L4sCS8vx0JFe5jT =ltiU -----END PGP SIGNATURE----- --=-XCDaHpBWWXO+XqyNxi6g--