Re: Review of patch that uses "volatile sig_atomic_t"

On 2 Aug 2023, at 00:33, Rick Macklem <rick.macklem@gmail.com> wrote:
> 
> Just trying to understand what you are suggesting...
> 1 - Declare the variable _Atomic(int) OR atomic_int (is there a preference) and
>     not volatile.

Either is fine (the latter is a typedef for the former).  I am not a huge fan of the typedefs, some people like them, as far as I can tell it’s purely personal preference.

> 2 - Is there a need for signal_atomic_fence(memory_order_acquire); before the
>     assignment of the variable in the signal handler. (This exists in
> one place in
>     the source tree (bin/dd/misc,c), although for this example,
> neither volatile nor
>     _Atomic() are used for the variable's declaration.

You don’t need a fence if you use an atomic variable.  The fence prevents the compiler reordering things across it, using atomic operations also prevents this.  You might be able to use a fence and not use an atomic but I’d have to reread the spec very carefully to convince myself that this didn’t trigger undefined behaviour.

> 3 - Is there any need for other atomic_XXX() calls where the variable is used
>     outside of the signal handler?

No.  By default, _Atomic variables use sequentially consistent semantics.  You need to use the `atomic_` functions only for explicit memory orderings, which you might want to do for optimisation (very unlikely in this case).  Reading it outside the signal handler is the equivalent of doing `atomic_load` with a sequentially consistent memory order.  This is a stronger guarantee than you need, but it’s unlikely to cause performance problems if you’re doing more than a few dozen instructions worth of work between checks.

> In general, it is looking like FreeBSD needs to have a standard way of dealing
> with this and there will be assorted places that need to be fixed?

If we used a language that let you build abstractions, that would be easy (I have a C++ class that provides a static epoch counter that’s incremented in a signal handler and a local copy for each instance, so you can check if the signal handler has fired since it was last used.  It’s trivial to reuse in C++ projects but C doesn’t give you tools for doing this.

David