[PATCH v1 0/8] Update to C11, fix signal undefined behavior

[PATCH v1 0/8] Update to C11, fix signal undefined behavior

[Ian Rogers]

The use of C11 is mainstream in the kernel [1]. There was some
confusion on volatile and signal handlers in [2]. Switch to using
stdatomic.h (requires C11) and sig_atomic_t as per [3]. Thanks to Leo
Yan <leo.yan@linaro.org> for the suggestions.

[1] https://lore.kernel.org/lkml/CAHk-=whWbENRz-vLY6vpESDLj6kGUTKO3khGtVfipHqwewh2HQ@mail.gmail.com/
[2] https://lore.kernel.org/lkml/20221024011024.462518-1-irogers@google.com/
[3] https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers


Ian Rogers (8):
  perf build: Update to C standard to gnu11
  perf record: Use sig_atomic_t for signal handlers
  perf daemon: Use sig_atomic_t to avoid UB
  perf ftrace: Use sig_atomic_t to avoid UB
  perf session: Change type to avoid UB
  perf stat: Use sig_atomic_t to avoid UB
  perf top: Use sig_atomic_t to avoid UB
  perf trace: Use sig_atomic_t to avoid UB

 tools/perf/Makefile.config  | 2 +-
 tools/perf/builtin-daemon.c | 3 ++-
 tools/perf/builtin-ftrace.c | 4 ++--
 tools/perf/builtin-record.c | 9 +++++----
 tools/perf/builtin-stat.c   | 9 +++++----
 tools/perf/builtin-top.c    | 4 ++--
 tools/perf/builtin-trace.c  | 4 ++--
 tools/perf/util/session.c   | 3 ++-
 8 files changed, 21 insertions(+), 17 deletions(-)

-- 
2.38.0.135.g90850a2211-goog

[PATCH v1 1/8] perf build: Update to C standard to gnu11

[Ian Rogers]

C11 has become the standard for mainstream kernel development [1],
allowing it in the perf build enables libraries like stdatomic.h to be
assumed to be present. This came up in the context of [2].

[1] https://lore.kernel.org/lkml/CAHk-=whWbENRz-vLY6vpESDLj6kGUTKO3khGtVfipHqwewh2HQ@mail.gmail.com/
[2] https://lore.kernel.org/lkml/20221024011024.462518-1-irogers@google.com/

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Makefile.config | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 6fd4b1384b97..29c49e6e76a1 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -307,7 +307,7 @@ CORE_CFLAGS += -ggdb3
 CORE_CFLAGS += -funwind-tables
 CORE_CFLAGS += -Wall
 CORE_CFLAGS += -Wextra
-CORE_CFLAGS += -std=gnu99
+CORE_CFLAGS += -std=gnu11
 
 CXXFLAGS += -std=gnu++14 -fno-exceptions -fno-rtti
 CXXFLAGS += -Wall
-- 
2.38.0.135.g90850a2211-goog

[PATCH v1 2/8] perf record: Use sig_atomic_t for signal handlers

[Ian Rogers]

This removes undefined behavior as described in:
https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers

Suggested-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-record.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e128b855ddde..5332b9c05f28 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -61,6 +61,7 @@
 #include <locale.h>
 #include <poll.h>
 #include <pthread.h>
+#include <stdatomic.h>
 #include <unistd.h>
 #ifndef HAVE_GETTID
 #include <syscall.h>
@@ -646,10 +647,10 @@ static int record__pushfn(struct mmap *map, void *to, void *bf, size_t size)
 	return record__write(rec, map, bf, size);
 }
 
-static volatile int signr = -1;
-static volatile int child_finished;
+static volatile sig_atomic_t signr = -1;
+static volatile sig_atomic_t child_finished;
 #ifdef HAVE_EVENTFD_SUPPORT
-static volatile int done_fd = -1;
+static volatile sig_atomic_t done_fd = -1;
 #endif
 
 static void sig_handler(int sig)
@@ -1926,7 +1927,7 @@ static void record__read_lost_samples(struct record *rec)
 
 }
 
-static volatile int workload_exec_errno;
+static volatile sig_atomic_t workload_exec_errno;
 
 /*
  * evlist__prepare_workload will send a SIGUSR1
-- 
2.38.0.135.g90850a2211-goog

[PATCH v1 3/8] perf daemon: Use sig_atomic_t to avoid UB

[Ian Rogers]

Use sig_atomic_t for a variable written to in a signal handler and
read elsewhere. This is undefined behavior as per:
https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-daemon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 6cb3f6cc36d0..d05084952c09 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -9,6 +9,7 @@
 #include <string.h>
 #include <sys/file.h>
 #include <signal.h>
+#include <stdatomic.h>
 #include <stdlib.h>
 #include <time.h>
 #include <stdio.h>
@@ -105,7 +106,7 @@ static const char * const daemon_usage[] = {
 	NULL
 };
 
-static bool done;
+static volatile sig_atomic_t done;
 
 static void sig_handler(int sig __maybe_unused)
 {
-- 
2.38.0.135.g90850a2211-goog

[PATCH v1 4/8] perf ftrace: Use sig_atomic_t to avoid UB

[Ian Rogers]

Use sig_atomic_t for a variable written to in a signal handler and
read elsewhere. This is undefined behavior as per:
https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-ftrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 7de07bb16d23..d7fe00f66b83 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -36,8 +36,8 @@
 
 #define DEFAULT_TRACER  "function_graph"
 
-static volatile int workload_exec_errno;
-static bool done;
+static volatile sig_atomic_t workload_exec_errno;
+static volatile sig_atomic_t done;
 
 static void sig_handler(int sig __maybe_unused)
 {
-- 
2.38.0.135.g90850a2211-goog

[PATCH v1 5/8] perf session: Change type to avoid UB

[Ian Rogers]

session_done is written to inside the signal handler of perf report
and script. Switch its type to avoid undefined behavior.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/session.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1a4f10de29ff..ba52cc4092dc 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <errno.h>
+#include <stdatomic.h>
 #include <inttypes.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
@@ -2022,7 +2023,7 @@ static int perf_session__flush_thread_stacks(struct perf_session *session)
 					 NULL);
 }
 
-volatile int session_done;
+volatile sig_atomic_t session_done;
 
 static int __perf_session__process_decomp_events(struct perf_session *session);
 
-- 
2.38.0.135.g90850a2211-goog

[PATCH v1 6/8] perf stat: Use sig_atomic_t to avoid UB

[Ian Rogers]

Use sig_atomic_t for variables written/accessed in signal
handlers. This is undefined behavior as per:
https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-stat.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 265b05157972..13bb8508d228 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -83,6 +83,7 @@
 #include <inttypes.h>
 #include <locale.h>
 #include <math.h>
+#include <stdatomic.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
@@ -173,7 +174,7 @@ static struct target target = {
 
 #define METRIC_ONLY_LEN 20
 
-static volatile pid_t		child_pid			= -1;
+static volatile sig_atomic_t	child_pid			= -1;
 static int			detailed_run			=  0;
 static bool			transaction_run;
 static bool			topdown_run			= false;
@@ -208,7 +209,7 @@ struct perf_stat {
 static struct perf_stat		perf_stat;
 #define STAT_RECORD		perf_stat.record
 
-static volatile int done = 0;
+static volatile sig_atomic_t done = 0;
 
 static struct perf_stat_config stat_config = {
 	.aggr_mode		= AGGR_GLOBAL,
@@ -569,7 +570,7 @@ static void disable_counters(void)
 	}
 }
 
-static volatile int workload_exec_errno;
+static volatile sig_atomic_t workload_exec_errno;
 
 /*
  * evlist__prepare_workload will send a SIGUSR1
@@ -1029,7 +1030,7 @@ static void print_counters(struct timespec *ts, int argc, const char **argv)
 	evlist__print_counters(evsel_list, &stat_config, &target, ts, argc, argv);
 }
 
-static volatile int signr = -1;
+static volatile sig_atomic_t signr = -1;
 
 static void skip_signal(int signo)
 {
-- 
2.38.0.135.g90850a2211-goog

[PATCH v1 7/8] perf top: Use sig_atomic_t to avoid UB

[Ian Rogers]

Use sig_atomic_t for variables written/accessed in signal
handlers. This is undefined behavior as per:
https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-top.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 4b3ff7687236..bb5bd241246b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -87,8 +87,8 @@
 #include <linux/ctype.h>
 #include <perf/mmap.h>
 
-static volatile int done;
-static volatile int resize;
+static volatile sig_atomic_t done;
+static volatile sig_atomic_t resize;
 
 #define HEADER_LINE_NR  5
 
-- 
2.38.0.135.g90850a2211-goog

[PATCH v1 8/8] perf trace: Use sig_atomic_t to avoid UB

[Ian Rogers]

Use sig_atomic_t for variables written/accessed in signal
handlers. This is undefined behavior as per:
https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-trace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index d3c757769b96..72991528687e 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1535,8 +1535,8 @@ static size_t trace__fprintf_tstamp(struct trace *trace, u64 tstamp, FILE *fp)
 }
 
 static pid_t workload_pid = -1;
-static bool done = false;
-static bool interrupted = false;
+static volatile sig_atomic_t done = false;
+static volatile sig_atomic_t interrupted = false;
 
 static void sighandler_interrupt(int sig __maybe_unused)
 {
-- 
2.38.0.135.g90850a2211-goog

Re: [PATCH v1 0/8] Update to C11, fix signal undefined behavior

[Arnaldo Carvalho de Melo]

Em Mon, Oct 24, 2022 at 10:35:15AM -0700, Ian Rogers escreveu:
> The use of C11 is mainstream in the kernel [1]. There was some
> confusion on volatile and signal handlers in [2]. Switch to using
> stdatomic.h (requires C11) and sig_atomic_t as per [3]. Thanks to Leo
> Yan <leo.yan@linaro.org> for the suggestions.
> 
> [1] https://lore.kernel.org/lkml/CAHk-=whWbENRz-vLY6vpESDLj6kGUTKO3khGtVfipHqwewh2HQ@mail.gmail.com/
> [2] https://lore.kernel.org/lkml/20221024011024.462518-1-irogers@google.com/
> [3] https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers

I think I'll apply this to perf/core, i.e. for 6.3, ok?

- Arnaldo
 
> 
> Ian Rogers (8):
>   perf build: Update to C standard to gnu11
>   perf record: Use sig_atomic_t for signal handlers
>   perf daemon: Use sig_atomic_t to avoid UB
>   perf ftrace: Use sig_atomic_t to avoid UB
>   perf session: Change type to avoid UB
>   perf stat: Use sig_atomic_t to avoid UB
>   perf top: Use sig_atomic_t to avoid UB
>   perf trace: Use sig_atomic_t to avoid UB
> 
>  tools/perf/Makefile.config  | 2 +-
>  tools/perf/builtin-daemon.c | 3 ++-
>  tools/perf/builtin-ftrace.c | 4 ++--
>  tools/perf/builtin-record.c | 9 +++++----
>  tools/perf/builtin-stat.c   | 9 +++++----
>  tools/perf/builtin-top.c    | 4 ++--
>  tools/perf/builtin-trace.c  | 4 ++--
>  tools/perf/util/session.c   | 3 ++-
>  8 files changed, 21 insertions(+), 17 deletions(-)
> 
> -- 
> 2.38.0.135.g90850a2211-goog

-- 

- Arnaldo

Re: [PATCH v1 0/8] Update to C11, fix signal undefined behavior

[Ian Rogers]

On Mon, Oct 24, 2022 at 10:51 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Mon, Oct 24, 2022 at 10:35:15AM -0700, Ian Rogers escreveu:
> > The use of C11 is mainstream in the kernel [1]. There was some
> > confusion on volatile and signal handlers in [2]. Switch to using
> > stdatomic.h (requires C11) and sig_atomic_t as per [3]. Thanks to Leo
> > Yan <leo.yan@linaro.org> for the suggestions.
> >
> > [1] https://lore.kernel.org/lkml/CAHk-=whWbENRz-vLY6vpESDLj6kGUTKO3khGtVfipHqwewh2HQ@mail.gmail.com/
> > [2] https://lore.kernel.org/lkml/20221024011024.462518-1-irogers@google.com/
> > [3] https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers
>
> I think I'll apply this to perf/core, i.e. for 6.3, ok?

Sounds good to me. 6.3 or 6.2? I suspect there is more cleanup like
this and to the iterators (from C11) that can be done.

Thanks,
Ian

> - Arnaldo
>
> >
> > Ian Rogers (8):
> >   perf build: Update to C standard to gnu11
> >   perf record: Use sig_atomic_t for signal handlers
> >   perf daemon: Use sig_atomic_t to avoid UB
> >   perf ftrace: Use sig_atomic_t to avoid UB
> >   perf session: Change type to avoid UB
> >   perf stat: Use sig_atomic_t to avoid UB
> >   perf top: Use sig_atomic_t to avoid UB
> >   perf trace: Use sig_atomic_t to avoid UB
> >
> >  tools/perf/Makefile.config  | 2 +-
> >  tools/perf/builtin-daemon.c | 3 ++-
> >  tools/perf/builtin-ftrace.c | 4 ++--
> >  tools/perf/builtin-record.c | 9 +++++----
> >  tools/perf/builtin-stat.c   | 9 +++++----
> >  tools/perf/builtin-top.c    | 4 ++--
> >  tools/perf/builtin-trace.c  | 4 ++--
> >  tools/perf/util/session.c   | 3 ++-
> >  8 files changed, 21 insertions(+), 17 deletions(-)
> >
> > --
> > 2.38.0.135.g90850a2211-goog
>
> --
>
> - Arnaldo

Re: [PATCH v1 0/8] Update to C11, fix signal undefined behavior

[Ian Rogers]

On Mon, Oct 24, 2022 at 10:59 AM Ian Rogers <irogers@google.com> wrote:
>
> On Mon, Oct 24, 2022 at 10:51 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Mon, Oct 24, 2022 at 10:35:15AM -0700, Ian Rogers escreveu:
> > > The use of C11 is mainstream in the kernel [1]. There was some
> > > confusion on volatile and signal handlers in [2]. Switch to using
> > > stdatomic.h (requires C11) and sig_atomic_t as per [3]. Thanks to Leo
> > > Yan <leo.yan@linaro.org> for the suggestions.
> > >
> > > [1] https://lore.kernel.org/lkml/CAHk-=whWbENRz-vLY6vpESDLj6kGUTKO3khGtVfipHqwewh2HQ@mail.gmail.com/
> > > [2] https://lore.kernel.org/lkml/20221024011024.462518-1-irogers@google.com/
> > > [3] https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers
> >
> > I think I'll apply this to perf/core, i.e. for 6.3, ok?
>
> Sounds good to me. 6.3 or 6.2? I suspect there is more cleanup like
> this and to the iterators (from C11) that can be done.
>
> Thanks,
> Ian

So I noticed a few changes missing #include-ing stdatomic.h and
sig_atomic_t is actually in signal.h. I'm not sure we need the C11
change then, but it seems like the right thing to do anyway. I'll do a
v2 to drop the unneeded (currently) include of stdatomic.h.

Thanks,
Ian

> > - Arnaldo
> >
> > >
> > > Ian Rogers (8):
> > >   perf build: Update to C standard to gnu11
> > >   perf record: Use sig_atomic_t for signal handlers
> > >   perf daemon: Use sig_atomic_t to avoid UB
> > >   perf ftrace: Use sig_atomic_t to avoid UB
> > >   perf session: Change type to avoid UB
> > >   perf stat: Use sig_atomic_t to avoid UB
> > >   perf top: Use sig_atomic_t to avoid UB
> > >   perf trace: Use sig_atomic_t to avoid UB
> > >
> > >  tools/perf/Makefile.config  | 2 +-
> > >  tools/perf/builtin-daemon.c | 3 ++-
> > >  tools/perf/builtin-ftrace.c | 4 ++--
> > >  tools/perf/builtin-record.c | 9 +++++----
> > >  tools/perf/builtin-stat.c   | 9 +++++----
> > >  tools/perf/builtin-top.c    | 4 ++--
> > >  tools/perf/builtin-trace.c  | 4 ++--
> > >  tools/perf/util/session.c   | 3 ++-
> > >  8 files changed, 21 insertions(+), 17 deletions(-)
> > >
> > > --
> > > 2.38.0.135.g90850a2211-goog
> >
> > --
> >
> > - Arnaldo

Re: [PATCH v1 0/8] Update to C11, fix signal undefined behavior

[Arnaldo Carvalho de Melo]

Em Mon, Oct 24, 2022 at 10:59:03AM -0700, Ian Rogers escreveu:
> On Mon, Oct 24, 2022 at 10:51 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Mon, Oct 24, 2022 at 10:35:15AM -0700, Ian Rogers escreveu:
> > > The use of C11 is mainstream in the kernel [1]. There was some
> > > confusion on volatile and signal handlers in [2]. Switch to using
> > > stdatomic.h (requires C11) and sig_atomic_t as per [3]. Thanks to Leo
> > > Yan <leo.yan@linaro.org> for the suggestions.
> > >
> > > [1] https://lore.kernel.org/lkml/CAHk-=whWbENRz-vLY6vpESDLj6kGUTKO3khGtVfipHqwewh2HQ@mail.gmail.com/
> > > [2] https://lore.kernel.org/lkml/20221024011024.462518-1-irogers@google.com/
> > > [3] https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers
> >
> > I think I'll apply this to perf/core, i.e. for 6.3, ok?
> 
> Sounds good to me. 6.3 or 6.2? I suspect there is more cleanup like
> this and to the iterators (from C11) that can be done.

oops, 6.2, sure, 6.1 is the current one, merge window closed. :-)

- Arnaldo

RE: [PATCH v1 0/8] Update to C11, fix signal undefined behavior

[David Laight]

From: Ian Rogers
> Sent: 24 October 2022 19:12
> 
> On Mon, Oct 24, 2022 at 10:59 AM Ian Rogers <irogers@google.com> wrote:
> >
> > On Mon, Oct 24, 2022 at 10:51 AM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> > >
> > > Em Mon, Oct 24, 2022 at 10:35:15AM -0700, Ian Rogers escreveu:
> > > > The use of C11 is mainstream in the kernel [1]. There was some
> > > > confusion on volatile and signal handlers in [2]. Switch to using
> > > > stdatomic.h (requires C11) and sig_atomic_t as per [3]. Thanks to Leo
> > > > Yan <leo.yan@linaro.org> for the suggestions.
> > > >
> > > > [1] https://lore.kernel.org/lkml/CAHk-=whWbENRz-
> vLY6vpESDLj6kGUTKO3khGtVfipHqwewh2HQ@mail.gmail.com/
> > > > [2] https://lore.kernel.org/lkml/20221024011024.462518-1-irogers@google.com/
> > > > [3] https://wiki.sei.cmu.edu/confluence/display/c/SIG31-
> C.+Do+not+access+shared+objects+in+signal+handlers
> > >
> > > I think I'll apply this to perf/core, i.e. for 6.3, ok?
> >
> > Sounds good to me. 6.3 or 6.2? I suspect there is more cleanup like
> > this and to the iterators (from C11) that can be done.
> >
> > Thanks,
> > Ian
> 
> So I noticed a few changes missing #include-ing stdatomic.h and
> sig_atomic_t is actually in signal.h. I'm not sure we need the C11
> change then, but it seems like the right thing to do anyway. I'll do a
> v2 to drop the unneeded (currently) include of stdatomic.h.

While the C standard requires you use sig_atomic_t (to avoid
wider RMW being done for writes) the kernel very much requires
that volatiles accesses are atomic.
So the compilers used will do that even when the C standard
would allow them to do otherwise.

Pretty much the last mainstream cpu that couldn't do atomic
byte writes was an old alpha.

So for anything that Linux is going to run on these changes
aren't really needed.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v1 0/8] Update to C11, fix signal undefined behavior

[Leo Yan]

On Tue, Oct 25, 2022 at 10:36:16AM +0000, David Laight wrote:

[...]

> > So I noticed a few changes missing #include-ing stdatomic.h and
> > sig_atomic_t is actually in signal.h. I'm not sure we need the C11
> > change then, but it seems like the right thing to do anyway. I'll do a
> > v2 to drop the unneeded (currently) include of stdatomic.h.
> 
> While the C standard requires you use sig_atomic_t (to avoid
> wider RMW being done for writes) the kernel very much requires
> that volatiles accesses are atomic.
> So the compilers used will do that even when the C standard
> would allow them to do otherwise.
> 
> Pretty much the last mainstream cpu that couldn't do atomic
> byte writes was an old alpha.

One case that it might break atomicity for the data with int type,
I.e. on Arm CPU when CPU access data without alignment, then it
cannot promise atomicity.  I am not sure if the data with int type
will be always compiled with 4-byte alignment, if it's packed
without alignment then sig_atomic_t is still required.

Clarify that I don't have deep understanding for compiler, so sorry
if I introduce any noise.

Thanks,
Leo

> So for anything that Linux is going to run on these changes
> aren't really needed.

> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)