From 531ff3ddeca3d14104875bf8bd1e06a1bbdd6f11 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Sun, 12 Apr 2020 11:06:32 +0200 Subject: [PATCH] atomics: change SC_ATOMIC_ADD to 'fetch_add' Until this point the SC_ATOMIC_ADD macro pointed to a 'add_fetch' intrinsic. This patch changes it to a 'fetch_add'. There are 2 reasons for this: 1. C11 stdatomics.h has only 'atomic_fetch_add' and no 'add_fetch' So this patch prepares for adding support for C11 atomics. 2. It was not consistent with SC_ATOMIC_SUB, which did use 'fetch_sub' and not 'sub_fetch'. Most callers are not using the return value, so these are unaffected. The callers that do use the return value are updated. --- src/detect-engine-loader.c | 2 +- src/flow-manager.c | 4 ++-- src/log-pcap.c | 1 + src/log-tlsstore.c | 1 + src/output-filedata.c | 5 +++-- src/runmode-af-packet.c | 2 +- src/runmode-netmap.c | 2 +- src/runmode-pcap.c | 2 +- src/runmode-pfring.c | 2 +- src/source-af-packet.c | 6 +++--- src/util-atomic.h | 6 +++--- 11 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c index fba37b8fa8..271c31e950 100644 --- a/src/detect-engine-loader.c +++ b/src/detect-engine-loader.c @@ -550,7 +550,7 @@ static TmEcode DetectLoaderThreadInit(ThreadVars *t, const void *initdata, void if (ftd == NULL) return TM_ECODE_FAILED; - ftd->instance = SC_ATOMIC_ADD(detect_loader_cnt, 1) - 1; /* id's start at 0 */ + ftd->instance = SC_ATOMIC_ADD(detect_loader_cnt, 1); /* id's start at 0 */ SCLogDebug("detect loader instance %u", ftd->instance); /* pass thread data back to caller */ diff --git a/src/flow-manager.c b/src/flow-manager.c index 26a452a23d..6aecc9c8b3 100644 --- a/src/flow-manager.c +++ b/src/flow-manager.c @@ -660,9 +660,9 @@ static TmEcode FlowManagerThreadInit(ThreadVars *t, const void *initdata, void * /* set the min and max value used for hash row walking * each thread has it's own section of the flow hash */ uint32_t range = flow_config.hash_size / flowmgr_number; - if (ftd->instance == 1) + if (ftd->instance == 0) ftd->max = range; - else if (ftd->instance == flowmgr_number) { + else if ((ftd->instance + 1) == flowmgr_number) { ftd->min = (range * (ftd->instance - 1)); ftd->max = flow_config.hash_size; } else { diff --git a/src/log-pcap.c b/src/log-pcap.c index f813d15c50..dda5fbbc5c 100644 --- a/src/log-pcap.c +++ b/src/log-pcap.c @@ -209,6 +209,7 @@ void PcapLogRegister(void) PcapLogDataDeinit, NULL); PcapLogProfileSetup(); SC_ATOMIC_INIT(thread_cnt); + SC_ATOMIC_SET(thread_cnt, 1); /* first id is 1 */ return; } diff --git a/src/log-tlsstore.c b/src/log-tlsstore.c index 8ada7bcbad..8bf5d28573 100644 --- a/src/log-tlsstore.c +++ b/src/log-tlsstore.c @@ -418,6 +418,7 @@ void LogTlsStoreRegister (void) LogTlsStoreLogThreadDeinit, LogTlsStoreLogExitPrintStats); SC_ATOMIC_INIT(cert_id); + SC_ATOMIC_SET(cert_id, 1); SCLogDebug("registered"); } diff --git a/src/output-filedata.c b/src/output-filedata.c index 9414513759..42b52d7b12 100644 --- a/src/output-filedata.c +++ b/src/output-filedata.c @@ -258,7 +258,7 @@ static void LogFiledataLogLoadWaldo(const char *path) if (fgets(line, (int)sizeof(line), fp) != NULL) { if (sscanf(line, "%10u", &id) == 1) { SCLogInfo("id %u", id); - (void) SC_ATOMIC_CAS(&g_file_store_id, 0, id); + SC_ATOMIC_SET(g_file_store_id, id); } } fclose(fp); @@ -275,7 +275,7 @@ static void LogFiledataLogStoreWaldo(const char *path) { char line[16] = ""; - if (SC_ATOMIC_GET(g_file_store_id) == 0) { + if (SC_ATOMIC_GET(g_file_store_id) == 1) { SCReturn; } @@ -448,6 +448,7 @@ void OutputFiledataLoggerRegister(void) OutputFiledataLogThreadDeinit, OutputFiledataLogExitPrintStats, OutputFiledataLog, OutputFiledataLoggerGetActiveCount); SC_ATOMIC_INIT(g_file_store_id); + SC_ATOMIC_SET(g_file_store_id, 1); } void OutputFiledataShutdown(void) diff --git a/src/runmode-af-packet.c b/src/runmode-af-packet.c index 9fd1fa8f76..3e62ad3e86 100644 --- a/src/runmode-af-packet.c +++ b/src/runmode-af-packet.c @@ -88,7 +88,7 @@ static void AFPDerefConfig(void *conf) { AFPIfaceConfig *pfp = (AFPIfaceConfig *)conf; /* Pcap config is used only once but cost of this low. */ - if (SC_ATOMIC_SUB(pfp->ref, 1) == 0) { + if (SC_ATOMIC_SUB(pfp->ref, 1) == 1) { SCFree(pfp); } } diff --git a/src/runmode-netmap.c b/src/runmode-netmap.c index 9280764a81..6962a33c89 100644 --- a/src/runmode-netmap.c +++ b/src/runmode-netmap.c @@ -84,7 +84,7 @@ static void NetmapDerefConfig(void *conf) { NetmapIfaceConfig *pfp = (NetmapIfaceConfig *)conf; /* config is used only once but cost of this low. */ - if (SC_ATOMIC_SUB(pfp->ref, 1) == 0) { + if (SC_ATOMIC_SUB(pfp->ref, 1) == 1) { SCFree(pfp); } } diff --git a/src/runmode-pcap.c b/src/runmode-pcap.c index db22d2f0a1..76c25e258f 100644 --- a/src/runmode-pcap.c +++ b/src/runmode-pcap.c @@ -63,7 +63,7 @@ static void PcapDerefConfig(void *conf) { PcapIfaceConfig *pfp = (PcapIfaceConfig *)conf; /* Pcap config is used only once but cost of this low. */ - if (SC_ATOMIC_SUB(pfp->ref, 1) == 0) { + if (SC_ATOMIC_SUB(pfp->ref, 1) == 1) { SCFree(pfp); } } diff --git a/src/runmode-pfring.c b/src/runmode-pfring.c index 0c17a0533a..1ade202427 100644 --- a/src/runmode-pfring.c +++ b/src/runmode-pfring.c @@ -70,7 +70,7 @@ void RunModeIdsPfringRegister(void) static void PfringDerefConfig(void *conf) { PfringIfaceConfig *pfp = (PfringIfaceConfig *)conf; - if (SC_ATOMIC_SUB(pfp->ref, 1) == 0) { + if (SC_ATOMIC_SUB(pfp->ref, 1) == 1) { if (pfp->bpf_filter) { SCFree(pfp->bpf_filter); } diff --git a/src/source-af-packet.c b/src/source-af-packet.c index c380661558..f3cb43b2b7 100644 --- a/src/source-af-packet.c +++ b/src/source-af-packet.c @@ -507,7 +507,7 @@ static void AFPPeersListReachedInc(void) if (peerslist.turn == 0) return; - if (SC_ATOMIC_ADD(peerslist.reached, 1) == peerslist.turn) { + if ((SC_ATOMIC_ADD(peerslist.reached, 1) + 1) == peerslist.turn) { SCLogInfo("All AFP capture threads are running."); (void)SC_ATOMIC_SET(peerslist.reached, 0); /* Set turn to 0 to skip syncrhonization when ReceiveAFPLoop is @@ -1228,7 +1228,7 @@ static int AFPDerefSocket(AFPPeer* peer) if (peer == NULL) return 1; - if (SC_ATOMIC_SUB(peer->sock_usage, 1) == 0) { + if (SC_ATOMIC_SUB(peer->sock_usage, 1) == 1) { if (SC_ATOMIC_GET(peer->state) == AFP_STATE_DOWN) { SCLogInfo("Cleaning socket connected to '%s'", peer->iface); close(SC_ATOMIC_GET(peer->socket)); @@ -1265,7 +1265,7 @@ static void AFPSwitchState(AFPThreadVars *ptv, int state) #endif if (ptv->socket != -1) { /* we need to wait for all packets to return data */ - if (SC_ATOMIC_SUB(ptv->mpeer->sock_usage, 1) == 0) { + if (SC_ATOMIC_SUB(ptv->mpeer->sock_usage, 1) == 1) { SCLogDebug("Cleaning socket connected to '%s'", ptv->iface); munmap(ptv->ring_buf, ptv->ring_buflen); close(ptv->socket); diff --git a/src/util-atomic.h b/src/util-atomic.h index ba38ffc1b1..85d0a02935 100644 --- a/src/util-atomic.h +++ b/src/util-atomic.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2013 Open Information Security Foundation +/* Copyright (C) 2007-2020 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -185,7 +185,7 @@ * \param val the value to add to the variable */ #define SC_ATOMIC_ADD(name, val) \ - SCAtomicAddAndFetch(&(name ## _sc_atomic__), (val)) + SCAtomicFetchAndAdd(&(name ## _sc_atomic__), (val)) /** * \brief sub a value from our atomic variable @@ -194,7 +194,7 @@ * \param val the value to sub from the variable */ #define SC_ATOMIC_SUB(name, val) \ - SCAtomicSubAndFetch(&(name ## _sc_atomic__), (val)) + SCAtomicFetchAndSub(&(name ## _sc_atomic__), (val)) /** * \brief Bitwise OR a value to our atomic variable