diff mbox series

[1/1] suricatta: prefer CLOCK_MONOTONIC for suricatta_wait

Message ID 20240515183033.97232-1-james.hilliard1@gmail.com
State Accepted
Headers show
Series [1/1] suricatta: prefer CLOCK_MONOTONIC for suricatta_wait | expand

Commit Message

James Hilliard May 15, 2024, 6:30 p.m. UTC
When calling sem_timedwait there may be side effects caused by
discontinuous jumps in the system time which can cause the clock
to go backwards.

To try and avoid side effects caused by these issues use the new
sem_clockwait function with CLOCK_MONOTONIC which should be available
on linux as of glibc version 2.30. Other libc's do not appear to have
support for sem_clockwait so we still have to fall back to
sem_timedwait on those systems.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 suricatta/suricatta.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

Comments

Stefano Babic June 1, 2024, 1:48 p.m. UTC | #1
Hi James,

On 15.05.24 20:30, James Hilliard wrote:
> When calling sem_timedwait there may be side effects caused by
> discontinuous jumps in the system time which can cause the clock
> to go backwards.
>
> To try and avoid side effects caused by these issues use the new
> sem_clockwait function with CLOCK_MONOTONIC which should be available
> on linux as of glibc version 2.30. Other libc's do not appear to have
> support for sem_clockwait so we still have to fall back to
> sem_timedwait on those systems.
>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>   suricatta/suricatta.c | 27 +++++++++++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/suricatta/suricatta.c b/suricatta/suricatta.c
> index 20eb427f..39798a6d 100644
> --- a/suricatta/suricatta.c
> +++ b/suricatta/suricatta.c
> @@ -25,6 +25,16 @@
>   #include "swupdate_settings.h"
>   #include <network_ipc.h>
>
> +/*
> + * Needs sem_clockwait which was added in glibc v2.30 on linux.
> + */
> +#if defined(__linux__) && defined(__GLIBC__) && \
> +	(__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 30))
> +#define USE_SEM_MONOTONIC 1
> +#else
> +#define USE_SEM_MONOTONIC 0
> +#endif
> +
>   static bool enable = true;
>   static bool trigger = false;
>   static struct option long_options[] = {
> @@ -185,12 +195,20 @@ int suricatta_wait(int seconds)
>   	int retval;
>   	int enable_entry = enable;
>
> +#if USE_SEM_MONOTONIC
> +	clock_gettime(CLOCK_MONOTONIC, &tp);
> +#else
>   	clock_gettime(CLOCK_REALTIME, &tp);
> -	int t_entry = tp.tv_sec;
> +#endif
> +	time_t t_entry = tp.tv_sec;
>
>   	tp.tv_sec += seconds;
>   	DEBUG("Sleeping for %d seconds.", seconds);
> +#if USE_SEM_MONOTONIC
> +	retval = sem_clockwait(&suricatta_enable_sema, CLOCK_MONOTONIC, &tp);
> +#else
>   	retval = sem_timedwait(&suricatta_enable_sema, &tp);
> +#endif
>
>   	if (retval) {
>   		if (errno != ETIMEDOUT) {
> @@ -200,7 +218,12 @@ int suricatta_wait(int seconds)
>   		/* else: Suricatta awakened because timeout expired */
>   	} else {
>   		/* suricatta_enable_sema unlocked */
> -		time_t t_wake = time(NULL);
> +#if USE_SEM_MONOTONIC
> +		clock_gettime(CLOCK_MONOTONIC, &tp);
> +#else
> +		clock_gettime(CLOCK_REALTIME, &tp);
> +#endif
> +		time_t t_wake = tp.tv_sec;
>
>   		TRACE("Suricatta woke up for IPC at %ld seconds", t_wake - t_entry);
>   		/*

Applied to -master, thanks !

Best regards,
Stefano Babic
diff mbox series

Patch

diff --git a/suricatta/suricatta.c b/suricatta/suricatta.c
index 20eb427f..39798a6d 100644
--- a/suricatta/suricatta.c
+++ b/suricatta/suricatta.c
@@ -25,6 +25,16 @@ 
 #include "swupdate_settings.h"
 #include <network_ipc.h>
 
+/*
+ * Needs sem_clockwait which was added in glibc v2.30 on linux.
+ */
+#if defined(__linux__) && defined(__GLIBC__) && \
+	(__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 30))
+#define USE_SEM_MONOTONIC 1
+#else
+#define USE_SEM_MONOTONIC 0
+#endif
+
 static bool enable = true;
 static bool trigger = false;
 static struct option long_options[] = {
@@ -185,12 +195,20 @@  int suricatta_wait(int seconds)
 	int retval;
 	int enable_entry = enable;
 
+#if USE_SEM_MONOTONIC
+	clock_gettime(CLOCK_MONOTONIC, &tp);
+#else
 	clock_gettime(CLOCK_REALTIME, &tp);
-	int t_entry = tp.tv_sec;
+#endif
+	time_t t_entry = tp.tv_sec;
 
 	tp.tv_sec += seconds;
 	DEBUG("Sleeping for %d seconds.", seconds);
+#if USE_SEM_MONOTONIC
+	retval = sem_clockwait(&suricatta_enable_sema, CLOCK_MONOTONIC, &tp);
+#else
 	retval = sem_timedwait(&suricatta_enable_sema, &tp);
+#endif
 
 	if (retval) {
 		if (errno != ETIMEDOUT) {
@@ -200,7 +218,12 @@  int suricatta_wait(int seconds)
 		/* else: Suricatta awakened because timeout expired */
 	} else {
 		/* suricatta_enable_sema unlocked */
-		time_t t_wake = time(NULL);
+#if USE_SEM_MONOTONIC
+		clock_gettime(CLOCK_MONOTONIC, &tp);
+#else
+		clock_gettime(CLOCK_REALTIME, &tp);
+#endif
+		time_t t_wake = tp.tv_sec;
 
 		TRACE("Suricatta woke up for IPC at %ld seconds", t_wake - t_entry);
 		/*