@cryptotaxi247 / netdata-1 / commits / a54fcb7d7

Fix two helgrind reports (#13325)

* Use atomics ops with host->rrdpush_sender_connected. * Use different storage unit for rrdim's updated and exposed fields. The bitfields would end up in the same byte and thus requiring explicit protection with mutexes.

vkalintiris committed Jul 7, 2022 at 11:09 UTC a54fcb7d75655801b27ebc310320e505cadac85a
4 files changed +10 -10
daemon/analytics.c
+1 -1
@@ -382,7 +382,7 @@ void analytics_https(void)
382 BUFFER *b = buffer_create(30);
383 #ifdef ENABLE_HTTPS
384 analytics_exporting_connectors_ssl(b);
385 - buffer_strcat(b, netdata_client_ctx && localhost->ssl.flags == NETDATA_SSL_HANDSHAKE_COMPLETE && localhost->rrdpush_sender_connected == 1 ? "streaming|" : "|");
385 + buffer_strcat(b, netdata_client_ctx && localhost->ssl.flags == NETDATA_SSL_HANDSHAKE_COMPLETE && __atomic_load_n(&localhost->rrdpush_sender_connected, __ATOMIC_SEQ_CST) ? "streaming|" : "|");
386 buffer_strcat(b, netdata_srv_ctx ? "web" : "");
387 #else
388 buffer_strcat(b, "||");
database/rrd.h
+3 -3
@@ -264,8 +264,8 @@ struct rrddim {
264 RRD_MEMORY_MODE rrd_memory_mode; // the memory mode for this dimension
265 RRDDIM_FLAGS flags; // configuration flags for the dimension
266
267 - unsigned int updated:1; // 1 when the dimension has been updated since the last processing
268 - unsigned int exposed:1; // 1 when set what have sent this dimension to the central netdata
267 + bool updated; // 1 when the dimension has been updated since the last processing
268 + bool exposed; // 1 when set what have sent this dimension to the central netdata
269
270 collected_number multiplier; // the multiplier of the collected values
271 collected_number divisor; // the divider of the collected values
@@ -831,7 +831,7 @@ struct rrdhost {
831 netdata_thread_t rrdpush_sender_thread; // the sender thread
832 void *dbsync_worker;
833
834 - volatile unsigned int rrdpush_sender_connected; // 1 when the sender is ready to push metrics
834 + bool rrdpush_sender_connected; // 1 when the sender is ready to push metrics
835 int rrdpush_sender_socket; // the fd of the socket to the remote host, or -1
836
837 volatile unsigned int rrdpush_sender_error_shown; // 1 when we have logged a communication error
streaming/rrdpush.c
+2 -2
@@ -334,7 +334,7 @@ void rrdset_done_push(RRDSET *st) {
334 rrdpush_sender_thread_spawn(host);
335
336 // Handle non-connected case
337 - if(unlikely(!host->rrdpush_sender_connected)) {
337 + if(unlikely(!__atomic_load_n(&host->rrdpush_sender_connected, __ATOMIC_SEQ_CST))) {
338 if(unlikely(!host->rrdpush_sender_error_shown))
339 error("STREAM %s [send]: not ready - discarding collected metrics.", host->hostname);
340 host->rrdpush_sender_error_shown = 1;
@@ -383,7 +383,7 @@ void rrdpush_send_labels(RRDHOST *host) {
383
384 void rrdpush_claimed_id(RRDHOST *host)
385 {
386 - if(unlikely(!host->rrdpush_send_enabled || !host->rrdpush_sender_connected))
386 + if(unlikely(!host->rrdpush_send_enabled || !__atomic_load_n(&host->rrdpush_sender_connected, __ATOMIC_SEQ_CST)))
387 return;
388
389 if(host->sender->version < STREAM_VERSION_CLAIM)
streaming/sender.c
+4 -4
@@ -80,7 +80,7 @@ void sender_commit(struct sender_state *s) {
80
81
82 static inline void rrdpush_sender_thread_close_socket(RRDHOST *host) {
83 - host->rrdpush_sender_connected = 0;
83 + __atomic_clear(&host->rrdpush_sender_connected, __ATOMIC_SEQ_CST);
84
85 if(host->rrdpush_sender_socket != -1) {
86 close(host->rrdpush_sender_socket);
@@ -102,7 +102,7 @@ static inline void rrdpush_sender_add_host_variable_to_buffer_nolock(RRDHOST *ho
102 }
103
104 void rrdpush_sender_send_this_host_variable_now(RRDHOST *host, RRDVAR *rv) {
105 - if(host->rrdpush_send_enabled && host->rrdpush_sender_spawn && host->rrdpush_sender_connected) {
105 + if(host->rrdpush_send_enabled && host->rrdpush_sender_spawn && __atomic_load_n(&host->rrdpush_sender_connected, __ATOMIC_SEQ_CST)) {
106 sender_start(host->sender);
107 rrdpush_sender_add_host_variable_to_buffer_nolock(host, rv);
108 sender_commit(host->sender);
@@ -563,7 +563,7 @@ static void attempt_to_connect(struct sender_state *state)
563 state->sent_bytes_on_this_connection = 0;
564
565 // let the data collection threads know we are ready
566 - state->host->rrdpush_sender_connected = 1;
566 + __atomic_test_and_set(&state->host->rrdpush_sender_connected, __ATOMIC_SEQ_CST);
567 }
568 else {
569 // increase the failed connections counter
@@ -770,7 +770,7 @@ void *rrdpush_sender_thread(void *ptr) {
770 remote_clock_resync_iterations); // TODO: REMOVE FOR SLEW / GAPFILLING
771
772 // initialize rrdpush globals
773 - s->host->rrdpush_sender_connected = 0;
773 + __atomic_clear(&s->host->rrdpush_sender_connected, __ATOMIC_SEQ_CST);
774 if(pipe(s->host->rrdpush_sender_pipe) == -1) {
775 error("STREAM %s [send]: cannot create required pipe. DISABLING STREAMING THREAD", s->host->hostname);
776 return NULL;