Date: Wed, 2 Aug 2023 09:12:06 +0100 From: David Chisnall <theraven@FreeBSD.org> To: Rick Macklem <rick.macklem@gmail.com> Cc: FreeBSD CURRENT <freebsd-current@freebsd.org> Subject: Re: Review of patch that uses "volatile sig_atomic_t" Message-ID: <9A5C0082-2FFD-4B2B-9381-7ACD9EB7C4DF@FreeBSD.org> In-Reply-To: <CAM5tNy7jNQ2NB91V96vgMQ8AeZSVp5rHMZcZ7PG0WPQyBEJ6XQ@mail.gmail.com> References: <CAM5tNy5aAyRk_CML57Q7OEPXGVEjM=o8fqWdLJCRkHubBYzCNQ@mail.gmail.com> <A5E9074A-5D63-4351-9C0B-D7990E12AC9E@freebsd.org> <CAM5tNy7jNQ2NB91V96vgMQ8AeZSVp5rHMZcZ7PG0WPQyBEJ6XQ@mail.gmail.com>
index | next in thread | previous in thread | raw e-mail
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. Davidhelp
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9A5C0082-2FFD-4B2B-9381-7ACD9EB7C4DF>