From nobody Thu Feb 22 01:11:22 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4TgFTY14pFz5BkgT; Thu, 22 Feb 2024 01:11:29 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4TgFTX6W1cz4gSR; Thu, 22 Feb 2024 01:11:28 +0000 (UTC) (envelope-from kostikbel@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: from tom.home (kib@localhost [127.0.0.1] (may be forged)) by kib.kiev.ua (8.18.1/8.18.1) with ESMTP id 41M1BMTG016477; Thu, 22 Feb 2024 03:11:25 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua 41M1BMTG016477 Received: (from kostik@localhost) by tom.home (8.18.1/8.18.1/Submit) id 41M1BMos016476; Thu, 22 Feb 2024 03:11:22 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 22 Feb 2024 03:11:22 +0200 From: Konstantin Belousov To: Jessica Clarke Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Subject: Re: git: 8271d9b99a3b - main - libsys: remove usage of pthread_once and _once_stub Message-ID: References: <202402210029.41L0TOH5000231@gitrepo.freebsd.org> <964A29A2-4C51-4037-8EBE-320008D48AE0@freebsd.org> <4715B319-B7DE-4D06-9F27-00CFE5AF89A7@freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <4715B319-B7DE-4D06-9F27-00CFE5AF89A7@freebsd.org> X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=4.0.0 X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-14) on tom.home X-Spamd-Bar: ---- X-Rspamd-Queue-Id: 4TgFTX6W1cz4gSR X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:6939, ipnet:2001:470::/32, country:US] On Wed, Feb 21, 2024 at 05:23:10PM +0000, Jessica Clarke wrote: > On 21 Feb 2024, at 14:17, Konstantin Belousov wrote: > > > > On Wed, Feb 21, 2024 at 12:51:04AM +0000, Jessica Clarke wrote: > >> On 21 Feb 2024, at 00:29, Konstantin Belousov wrote: > >>> > >>> The branch main has been updated by kib: > >>> > >>> URL: https://cgit.FreeBSD.org/src/commit/?id=8271d9b99a3b98c662ee9a6257a144284b7e1728 > >>> > >>> commit 8271d9b99a3b98c662ee9a6257a144284b7e1728 > >>> Author: Konstantin Belousov > >>> AuthorDate: 2024-02-20 14:45:29 +0000 > >>> Commit: Konstantin Belousov > >>> CommitDate: 2024-02-21 00:26:11 +0000 > >>> > >>> libsys: remove usage of pthread_once and _once_stub > >>> > >>> that existed in auxv.c, use simple bool gate instead. This leaves a > >>> small window if two threads try to call _elf_aux_info(3) simultaneously. > >>> The situation is safe because auxv parsing is really idempotent. The > >>> parsed data is the same, and we store atomic types (int/long/ptr) so > >>> double-init does not matter. > >> > >> You still need to load acquire and store release aux_once though, > >> otherwise you can see aux_once as true yet read the pre-initialised > >> data. In practice that’s surely very hard to hit, but the code as > >> written is now wrong. Also, idempotence should probably be made > >> unnecessary by using 0/1/2 state for uninitialised/initialising/ > >> initialised, as it’s still technically UB from a C AM perspective due > >> to not being data race free if two threads initialise at the same time. > >> Better to just do the correct thing rather than risk things going wrong. > > > > There is too much to handle 'in process' state for loosing thread, I need > > the whole libthr machinery. > > What do you need libthr for? In pseudo-C: > > x = load_acquire(&aux_once) > if (__predict_true(x == 2)) > return; > if (x == 1 || !compare_exchange_strong_acquire(&aux_once, &x, 1)) { > while (x != 2) { > yield(); > x = load_acquire(&aux_once) > } > return; > } > /* initialise as before */ > store_release(&aux_once, 2); > > I believe that’s all you need. Or compare exchange 0 to 1 as the > initial operation; makes the source code shorter at the expense of a > more expensive fast path: > > x = 0; > if (__predict_true(!compare_exchange_strong_acquire(&aux_once, &x, 1)) { > while (__predict_false(x != 2)) { > yield(); > x = load_acquire(&aux_once) > } > return; > } > /* initialise as before */ > store_release(&aux_once, 2); > > I probably have bugs in the above, but you get the gist. The bug in the fragment above is with the yield(). If low-priority thread enters the '1' (in progress) block, and then is preempted by high-priority thread also entering init_auxv(), the process would never make a progress. This is why I said that we need libthr (or umtx), to use real locking and move the waiting thread off cpu. In kernel, yield can be used in similar situations because we can bump the priority, although it is tricky. > > > I added the fences, thanks for noting. > > Thanks. > > Jess > > > WRT being UB from pure C, we already have much more assumptions about > > atomicity. > >