public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: passt-dev@passt.top
Subject: Re: [PATCH v3 10/13] flow: Move flow_count from context structure to a global
Date: Tue, 2 Jan 2024 19:13:35 +0100	[thread overview]
Message-ID: <20240102191335.413b2b04@elisabeth> (raw)
In-Reply-To: <ZZEDDyEVuXUiopch@zatzit>

On Sun, 31 Dec 2023 16:58:39 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Dec 28, 2023 at 07:25:18PM +0100, Stefano Brivio wrote:
> > On Thu, 21 Dec 2023 17:15:46 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > 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 structure,
> > > although it's really part of the same data structure as flowtab[], which
> > > is a genuine global.  
> > 
> > Well, the reason is that flow_tab[FLOW_MAX] might be problematically
> > too big to live on the stack, unlike flow_count.
> > 
> > But anyway, as far as thoughts of multithreading are concerned, both
> > should probably be global. And sure, it's more consistent this way.
> >   
> > > 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.  
> > 
> > If it's not static, it should be initialised, and that's not done here.  
> 
> 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.

...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:

  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:

  [...]

  — if it has arithmetic type, it is initialized to (positive or
    unsigned) zero;

And 'flow_count' has thread storage duration. In C99, however (draft
N1256), Section 6.7.8 "Initialization", clause 10:

  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:

  [...]

note the missing "or thread storage duration".

C89, the one I was actually basing my observation on, says, at 3.5.7
"Initialization":

  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.

so... um. We won't go back to C99. But to me, and maybe others, not
having a "= 0;" for a "global" means pretty much that we don't rely on
any particular initial value.

If you're strongly against it, fine, C11 says it's zero... but it makes
me a bit nervous nevertheless.

-- 
Stefano


  reply	other threads:[~2024-01-02 18:13 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-21  6:15 [PATCH v3 00/13] Manage more flow related things from generic flow code David Gibson
2023-12-21  6:15 ` [PATCH v3 01/13] flow: Make flow_table.h #include the protocol specific headers it needs David Gibson
2023-12-21  6:15 ` [PATCH v3 02/13] treewide: Standardise on 'now' for current timestamp variables David Gibson
2023-12-21  6:15 ` [PATCH v3 03/13] tcp, tcp_splice: Remove redundant handling from tcp_timer() David Gibson
2023-12-21  6:15 ` [PATCH v3 04/13] tcp, tcp_splice: Move per-type cleanup logic into per-type helpers David Gibson
2023-12-21  6:15 ` [PATCH v3 05/13] flow, tcp: Add flow-centric dispatch for deferred flow handling David Gibson
2023-12-28 18:24   ` Stefano Brivio
2023-12-31  5:56     ` David Gibson
2024-01-02 18:13       ` Stefano Brivio
2024-01-03  3:45         ` David Gibson
2023-12-21  6:15 ` [PATCH v3 06/13] flow, tcp: Add handling for per-flow timers David Gibson
2023-12-21  6:15 ` [PATCH v3 07/13] epoll: Better handling of number of epoll types David Gibson
2023-12-21  6:15 ` [PATCH v3 08/13] tcp, tcp_splice: Avoid double layered dispatch for connected TCP sockets David Gibson
2023-12-21  6:15 ` [PATCH v3 09/13] flow: Move flow_log_() to near top of flow.c David Gibson
2023-12-21  6:15 ` [PATCH v3 10/13] flow: Move flow_count from context structure to a global David Gibson
2023-12-28 18:25   ` Stefano Brivio
2023-12-31  5:58     ` David Gibson
2024-01-02 18:13       ` Stefano Brivio [this message]
2024-01-03  3:54         ` David Gibson
2024-01-03  7:08           ` Stefano Brivio
2024-01-04  9:51             ` David Gibson
2024-01-05  7:55               ` Stefano Brivio
2024-01-07  5:23                 ` David Gibson
2023-12-21  6:15 ` [PATCH v3 11/13] flow: Abstract allocation of new flows with helper function David Gibson
2023-12-21  6:15 ` [PATCH v3 12/13] flow: Enforce that freeing of closed flows must happen in deferred handlers David Gibson
2023-12-21  6:15 ` [PATCH v3 13/13] flow: Avoid moving flow entries to compact table David Gibson
2023-12-28 18:25   ` Stefano Brivio
2023-12-30 10:33     ` Stefano Brivio
2024-01-01 12:01       ` David Gibson
2024-01-02 18:13         ` Stefano Brivio
2024-01-04 10:02           ` David Gibson
2024-01-05  8:33             ` Stefano Brivio
2024-01-05  9:39               ` David Gibson
2024-01-05 10:27                 ` Stefano Brivio
2024-01-06 11:32                   ` David Gibson
2024-01-06 13:02                     ` Stefano Brivio
2024-01-07  5:20                       ` David Gibson
2024-01-01 10:44     ` David Gibson
2024-01-02 18:13       ` Stefano Brivio
2024-01-05  9:45         ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240102191335.413b2b04@elisabeth \
    --to=sbrivio@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).