From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id C72225A026F for ; Wed, 3 Jan 2024 08:08:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704265728; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Nn1kYc9YoSFdDlrwgRGUgrUfrK5bTd/1ozkpbuPSWLY=; b=StuU2LGY7wv1QnL0GSjaB9B+mmu2rsxRE8qVBPpMCCrY/s8H1KJQkR5M/lGA9UxAY40r4o ExHDZRsgHOf+5slS4tjVcpfdjoiG1QhDrzAZKf+/HVGo4ASYtXWBJUxEaqlkbO7sf9AvDb en2H58SABaF7cujiZvKMQki9Zx3r8UQ= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-606-cxdUphgfMo6GApmuUOUadQ-1; Wed, 03 Jan 2024 02:08:45 -0500 X-MC-Unique: cxdUphgfMo6GApmuUOUadQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D11721C29EA2; Wed, 3 Jan 2024 07:08:44 +0000 (UTC) Received: from elisabeth (unknown [10.39.208.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 66106492BC6; Wed, 3 Jan 2024 07:08:42 +0000 (UTC) Date: Wed, 3 Jan 2024 08:08:34 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v3 10/13] flow: Move flow_count from context structure to a global Message-ID: <20240103080834.24fa0a7a@elisabeth> In-Reply-To: References: <20231221061549.976358-1-david@gibson.dropbear.id.au> <20231221061549.976358-11-david@gibson.dropbear.id.au> <20231228192459.312cc508@elisabeth> <20240102191335.413b2b04@elisabeth> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: DHQ4A5D5Z2QPFMAWWYKD57EW4GYRSPD3 X-Message-ID-Hash: DHQ4A5D5Z2QPFMAWWYKD57EW4GYRSPD3 X-MailFrom: sbrivio@redhat.com 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: On Wed, 3 Jan 2024 14:54:27 +1100 David Gibson wrote: > I'm not sure where to get the actual text of the standards 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. Those are not the same as the standards, but differences from the final draft are also published... and they are usually not substantial. This is _very_ informative: https://stackoverflow.com/questions/81656/where-do-i-find-the-current-c-o= r-c-standard-documents Wikipedia also has the links, by the way. Anyway, in practice: - 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.com= /2014/10/02/ansi-iso-9899-1990-1/ansi-iso-9899-1990-1.pdf > 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 true= global > > > > > variable and what's in the quasi-global 'context structure'. The > > > > > flow_count field is one such example: it's in the context structu= re, > > > > > 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 problematicall= y > > > > too big to live on the stack, unlike flow_count. > > > >=20 > > > > But anyway, as far as thoughts of multithreading are concerned, bot= h > > > > 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 need= s to be > > > > > public, rather than static, but we expect to be able to change th= at in > > > > > future. =20 > > > >=20 > > > > If it's not static, it should be initialised, and that's not done h= ere. =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 > >=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 o= r > > 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. So, C11 defines static storage duration here: 6.2.4 Storage durations of objects [...] 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. do we have any linkage here? I would have said no -- but, going back to C99 for this, "6.2.2 Linkages of identifiers": 5 [...] If the declaration of an identifier for an object has file scope and no storage-class specifier, its linkage is external. which supports your paragraph below. By the way, C11 now says: 6.11.2 Linkages of identifiers 1 Declaring an identifier with internal linkage at file scope without the static storage-class specifier is an obsolescent feature > > 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 typ= e > > were assigned a null pointer constant. If an object that has > > automatic storage duration is not initialized explicitly, its value i= s > > 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 o= n > > 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. Right. Well, for C99 and C11 at least. For C89 things are slightly different: 6.1.2.4 Storage durations of objects [...] An object whose identifier is declared with external or internal linkage. or with the storage-class specifier static has static storage duration. [...] An object whose identifier is declared with no linkage and without the storage-class specifier static has automatic storage duration. 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). > Fwiw, I'm pretty sure the kernel has relied on zero-initialization of > non-static globals for many years. 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 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/ ? 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 Stefano