From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 791415A026F for ; Fri, 5 Jan 2024 08:34:34 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1704440070; bh=qowHwrpB4+ABEC7yxp3xy9ZNUnn+Dl8LscfINTw6po4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=mKe+VfOw/PKGU4KIbCSsM6583FpIe7LuKL0ltviWfJbRJnCVnv+WJoli/bYp6TxJK RKiy1lruUuCImppdQqHdOFgZyysOYDqwFAwcgcEAWJAh9e49GRVgBgAEOFbZxyidbN b7LJTKOqVadSmbxXzwIC02c2tlEfH4JFZr+Ox0Uo0N0tZ9xTh5PAqyd+WqE/z+vsxo LyjFoT6Sa7SBg3Yc8oP1gVv7+QciwmC5XA07enUKG3kCum+lgfHXm1LHIDAbUjaguL tA4Y6dmELvf1Qx916dgt262GPsoBLHSAVWE6eDNTa80uqKQLZg/ZvJFg2n+QWnntci lMtxs5fkommGg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4T5wFf5bQXz4wp0; Fri, 5 Jan 2024 18:34:30 +1100 (AEDT) Date: Thu, 4 Jan 2024 20:51:19 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 10/13] flow: Move flow_count from context structure to a global Message-ID: References: <20231221061549.976358-1-david@gibson.dropbear.id.au> <20231221061549.976358-11-david@gibson.dropbear.id.au> <20231228192459.312cc508@elisabeth> <20240102191335.413b2b04@elisabeth> <20240103080834.24fa0a7a@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mMs5jP4l9qQW68OX" Content-Disposition: inline In-Reply-To: <20240103080834.24fa0a7a@elisabeth> Message-ID-Hash: KNITUA5DMKNHW6YW5JAUGUREI3LM3ANM X-Message-ID-Hash: KNITUA5DMKNHW6YW5JAUGUREI3LM3ANM X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --mMs5jP4l9qQW68OX Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jan 03, 2024 at 08:08:34AM +0100, Stefano Brivio wrote: > On Wed, 3 Jan 2024 14:54:27 +1100 > David Gibson wrote: >=20 > > I'm not sure where to get the actual text of the standards >=20 > Let me answer this first: one (the?) trick is to use so-called final > drafts, which are made freely available (same as working drafts) by the > Working Group. >=20 > Those are not the same as the standards, but differences from the final > draft are also published... and they are usually not substantial. >=20 > This is _very_ informative: > https://stackoverflow.com/questions/81656/where-do-i-find-the-current-c= -or-c-standard-documents Ah, thanks. > Wikipedia also has the links, by the way. Anyway, in practice: >=20 > - C11: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf > - C99: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf > - C89: > https://web.archive.org/web/20200909074736if_/https://www.pdf-archive.c= om/2014/10/02/ansi-iso-9899-1990-1/ansi-iso-9899-1990-1.pdf >=20 > > On Tue, Jan 02, 2024 at 07:13:35PM +0100, Stefano Brivio wrote: > > > On Sun, 31 Dec 2023 16:58:39 +1100 > > > David Gibson wrote: > > > =20 > > > > On Thu, Dec 28, 2023 at 07:25:18PM +0100, Stefano Brivio wrote: =20 > > > > > On Thu, 21 Dec 2023 17:15:46 +1100 > > > > > David Gibson wrote: > > > > > =20 > > > > > > In general, the passt code is a bit haphazard about what's a tr= ue global > > > > > > variable and what's in the quasi-global 'context structure'. T= he > > > > > > flow_count field is one such example: it's in the context struc= ture, > > > > > > although it's really part of the same data structure as flowtab= [], which > > > > > > is a genuine global. =20 > > > > >=20 > > > > > Well, the reason is that flow_tab[FLOW_MAX] might be problematica= lly > > > > > too big to live on the stack, unlike flow_count. > > > > >=20 > > > > > But anyway, as far as thoughts of multithreading are concerned, b= oth > > > > > should probably be global. And sure, it's more consistent this wa= y. > > > > > =20 > > > > > > Move flow_count to be a regular global to match. For now it ne= eds to be > > > > > > public, rather than static, but we expect to be able to change = that in > > > > > > future. =20 > > > > >=20 > > > > > If it's not static, it should be initialised, and that's not done= here. =20 > > > >=20 > > > > Uh... what? "static" here is meaning module-global rather than > > > > global-global, which has no bearing on initialisation. AFAIK globa= ls > > > > are zero-initialised whether they're static or not. =20 > > >=20 > > > ...and to my utter surprise, I just discovered that if you talk C11, > > > you're right. From the N1570 draft (ISO/IEC 9899:201x), Section 6.7.9 > > > "Initialization", clause 10: > > >=20 > > > If an object that has automatic storage duration is not initialized > > > explicitly, its value is indeterminate. If an object that has static > > > or thread storage duration is not initialized explicitly, then: > > >=20 > > > [...] > > >=20 > > > =E2=80=94 if it has arithmetic type, it is initialized to (positive= or > > > unsigned) zero; > > >=20 > > > And 'flow_count' has thread storage duration. =20 > >=20 > > No.. I don't think it does. AFAICT only thread-local variables have > > thread storage duration. As a global flow_count will have static > > storage duration, even without the static keyword. >=20 > So, C11 defines static storage duration here: >=20 > 6.2.4 Storage durations of objects >=20 > [...] >=20 > 3 An object whose identifier is declared without the storage-class > specifier _Thread_local, and either with external or internal linkage > or with the storage-class specifier static, has static storage > duration. Its lifetime is the entire execution of the program and its > stored value is initialized only once, prior to program startup. >=20 > do we have any linkage here? I would have said no -- but, going back > to C99 for this, "6.2.2 Linkages of identifiers": >=20 > 5 [...] If the declaration of an identifier for an object has file > scope and no storage-class specifier, its linkage is external. >=20 > which supports your paragraph below. Right. > By the way, C11 now says: >=20 > 6.11.2 Linkages of identifiers >=20 > 1 Declaring an identifier with internal linkage at file scope without > the static storage-class specifier is an obsolescent feature Ok. I'm not even sure how you would do that. > > > In C99, however (draft > > > N1256), Section 6.7.8 "Initialization", clause 10: > > >=20 > > > If an object that has automatic storage duration is not initialized > > > explicitly, its value is indeterminate. If an object that has static > > > storage duration is not initialized explicitly, then: > > >=20 > > > [...] > > >=20 > > > note the missing "or thread storage duration". > > >=20 > > > C89, the one I was actually basing my observation on, says, at 3.5.7 > > > "Initialization": > > >=20 > > > If an object that has static storage duration is not initialized > > > explicitly, it is initialized implicitly as if every member that has > > > arithmetic type were assigned 0 and every member that has pointer t= ype > > > were assigned a null pointer constant. If an object that has > > > automatic storage duration is not initialized explicitly, its value= is > > > indeterminate. > > >=20 > > > so... um. We won't go back to C99. But to me, and maybe others, not > > > having a "=3D 0;" for a "global" means pretty much that we don't rely= on > > > any particular initial value. =20 > >=20 > > Again, I'm pretty sure that's not true, even for C99 and C89. AIUI, > > 'static' locals and *all* globals have "static storage diration". > >=20 > > I'm not sure where to get the actual text of the standards but see for > > example > >=20 > > https://en.cppreference.com/w/c/language/static_storage_duration > >=20 > > Here 'flow_count' has external linkage, thus satisfying the conditions > > for static storage duration. >=20 > Right. Well, for C99 and C11 at least. For C89 things are slightly > different: >=20 > 6.1.2.4 Storage durations of objects >=20 > [...] >=20 > An object whose identifier is declared with external or internal > linkage. or with the storage-class specifier static has static storage > duration. >=20 > [...] >=20 > An object whose identifier is declared with no linkage and without the > storage-class specifier static has automatic storage duration. >=20 > You might say it has external linkage. But it was not *declared with* > external linkage -- it just happens to have it (C89 and C99 don't > differ here). Hrm. We do have: extern unsigned flow_first_free; in flow_table.h. Does that cound as declaring with external linkage? > > Fwiw, I'm pretty sure the kernel has relied on zero-initialization of > > non-static globals for many years. >=20 > True, and the opposite is even considered as a style issue since 2007, > commit f0a594c1c74f ("update checkpatch.pl to version 0.08"). I also > found a discussion similar to this one: > https://lore.kernel.org/all/20201102184147.GA42288@localhost/#r >=20 > Anyway... a couple of years before that, it must have been a gcc version > in the late 2.x, I actually hit an issue with it. Was it a compiler > issue, or the correct interpretation of C89? Or maybe something on the > lines of: > https://www.thegoodpenguin.co.uk/blog/u-boot-relocation-bss-hang/ If it was an embedded setup, that last one is certainly possible. Zeroing the BSS is typically the loader's job, and I've certainly seen loader implementations - particularly in embedded firmware - that got this wrong. > ? I'll never know/remember. And then, after reading the standard, I > started obsessively adding those =3D 0. Well, good to know I can stop > doing it now. :) >=20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --mMs5jP4l9qQW68OX Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmWWf4IACgkQzQJF27ox 2GdFCw/7BaVEOgnGvzuN58NOJIXNPoeI1QJA4xoFHbdfcjr1VUecISRGzc7I6ct0 ZuSkd/uEfWl0vSm3BFDBELTIyDba1mXqfa3Y/bbmZNxecAhbBbB75Fx8N5o6r+Ij VZ8D5glChN5l840zn0/K/WIRi0n20oeFbcswU3vWiWMAdtD3g8gDh5KcMNkPdkdE BSP8bsF3dJbSVbbnJ3rFnKjWU5CPyrFBbwdfXspd0GAkNcJUkczlFka8px2cc1/g ty2OLxYG4ireq3Ii0vDsx0juNFWtsdWzxHkQHtcH72V1xqlwetf6C2p4uq8ur1ot Bcji36SLr6aB9m0LNbE2pQ8Pfx1qK8evAmZ4BoC/A0l8A+hDgMRmF5xPZGt2gcTc w2pHaz/v2yP11F+NSLWgqsOoRlPfM/m/IIkqDdBAKOWjfInnlbY0UrToETPQ5CQ+ OD3+JapFup2tLS3D0e9rBS3ECDcOSMkO7HyP8+Neqt/IFiDkje4FhBEr91uh8+lb n2OmIcYWDsSU+ROdRhzL8j/ESGzwXH9Pyq7L+4NyprRgSDcqbSNXIoeqp1pQGgJL /F1MbhW4J7ZHMFgV8l1Boz4e/G2T1tyceKgqwcInON0QDOE2OeWNWKF+B55aSGSI alitKSz8P8JWthji5ayWBn1MujPezQFrujl6eGt3qY+kDLCJwDE= =wV3l -----END PGP SIGNATURE----- --mMs5jP4l9qQW68OX--