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 D8A5D5A0271 for ; Wed, 3 Jan 2024 04:55:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1704254134; bh=gmHsqoe8l27KejDaJEQN5U7uAtxLLXhxJVOYnxh2HfI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=nF16hbmv2MSeIYYbwC+ETmewFVKt5ZgFL8RYlgA4blo63qG6Z0xhMUGz0EFsXqLUf E7ktoH++KnoUCeuUJoJn2AUKa/L0P4HfhBmdh+gJB+rdugX8h/QQ6x37QMR9S+VBU6 xtjRr20rurcEqt8otYwhO4KmepKP4YOJTGi2KGhqA7AOMRPpjQh40q0jp7/TmRbkfO NpQ3SfQAg2rfq40Vg0s+y0d1+o8F+W9PSHfhbTAnfEuEPBOtbmTNJVjglu2c+dTRDG CNUZRl5iEdMwrweBvw2CXaJVkBdSeOhDP4up/VWgzlGnYG4W5MKlDk6oVV0p+jUrIc okpR2y3/48zsw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4T4bTy5CDrz4wxZ; Wed, 3 Jan 2024 14:55:34 +1100 (AEDT) Date: Wed, 3 Jan 2024 14:54:27 +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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="SA5Lw/jUB/pLG8BW" Content-Disposition: inline In-Reply-To: <20240102191335.413b2b04@elisabeth> Message-ID-Hash: MJVI5X3PGUFFZ3MAH2XPK5FMOEQCDDWC X-Message-ID-Hash: MJVI5X3PGUFFZ3MAH2XPK5FMOEQCDDWC 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: --SA5Lw/jUB/pLG8BW Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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: > > > 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 true g= lobal > > > > variable and what's in the quasi-global 'context structure'. The > > > > flow_count field is one such example: it's in the context structure, > > > > 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 problematically > > > too big to live on the stack, unlike flow_count. > > >=20 > > > But anyway, as far as thoughts of multithreading are concerned, both > > > should probably be global. And sure, it's more consistent this way. > > > =20 > > > > Move flow_count to be a regular global to match. For now it needs = 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 her= e. =20 > >=20 > > Uh... what? "static" here is meaning module-global rather than > > global-global, which has no bearing on initialisation. AFAIK globals > > are zero-initialised whether they're static or not. >=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. 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. > 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 type > 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. Again, I'm pretty sure that's not true, even for C99 and C89. AIUI, 'static' locals and *all* globals have "static storage diration". I'm not sure where to get the actual text of the standards but see for example https://en.cppreference.com/w/c/language/static_storage_duration Here 'flow_count' has external linkage, thus satisfying the conditions for static storage duration. Fwiw, I'm pretty sure the kernel has relied on zero-initialization of non-static globals for many years. > If you're strongly against it, fine, C11 says it's zero... but it makes > me a bit nervous nevertheless. >=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 --SA5Lw/jUB/pLG8BW Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmWU2mcACgkQzQJF27ox 2GdErxAApgQKue+35GjyFfCOmLvJTK1Akh0Vwl6nekiLVMbmjuNCvqeLNn7qogJx q6XSJ1iyYv6OxOj/bvl104nmdjI7+G7Jc7xWPdCe25Rjo2HHZN8jQ44oSx2sD50c B/xLlc4qUoiN0X7WbFTLKvYlUHJ7YuTrqoAQ2Y1LABdoasynUVdJP3fYS55emDB+ oEY/a/D0EOu9ff+WsyP0Fi5b0aK2DjrxUU18WW7dSfO1Lm6iQGJhhhr99r+x2AA5 VUoNCBtB4AXks98nAUmC/uHHUpuh4W/A9+CGqgrWk6ZwOc1FK25PL5iQ/4RNbUf9 cp4RzDX9C2Tnquhq/VClre0D2MZDjSPRSchQGgorywbfAiBm3r2aXxeSPcae3rE6 sRG77G0IcSeykAehvqtTRvD6QuR5HCzgRQrl9AfHPkS0Jv4etHmehbWDMg1pbec0 Jmzu2yTLOrPsUrM38cuix7IKoktZ6onDj1xc7Tqixg6d7+tpCy0DKz6Bn3YMbXiJ m83l89FMTPdnwHefIPPpdBAS30DpdfwO0LiTkiiuPVE7pZMMxUkx7EY/DU8ju9JG 40l+CfwtXgF2pzUOrxiNQqWowpQVrMFD2KdXnDlICVlQ8IYH9Pa0yNLxYcblA3DS g69PFc6qE/Sm1Ut6LllAE53mnT697h0omhez/WBEpJA2qJ/9FlY= =p0Kt -----END PGP SIGNATURE----- --SA5Lw/jUB/pLG8BW--