util: replace atomic ops impls with g_atomic_int*

Libvirt's original atomic ops impls were largely copied
from GLib's code at the time. The only API difference
was that libvirt's virAtomicIntInc() would return a
value, but g_atomic_int_inc was void. We thus use
g_atomic_int_add(v, 1) instead, though this means
virAtomicIntInc() now returns the original value,
instead of the new value.

This rewrites libvirt's impl in terms of g_atomic_int*
as a short term conversion. The key motivation was to
quickly eliminate use of GNULIB's verify_expr() macro
which is not a direct match for G_STATIC_ASSERT_EXPR.
Long term all the callers should be updated to use
g_atomic_int* directly.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2020-01-09 12:54:51 +00:00
parent 4f128bbbfb
commit 7b9645a7d1
8 changed files with 26 additions and 347 deletions

View File

@ -1473,7 +1473,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
goto destroy_dom;
if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
/* finally we can call the 'started' hook script if any */

View File

@ -446,7 +446,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_UNKNOWN);
if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
/* Enable domain death events */

View File

@ -1468,7 +1468,7 @@ int virLXCProcessStart(virConnectPtr conn,
if (virCommandHandshakeNotify(cmd) < 0)
goto cleanup;
if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
if (lxcContainerWaitForContinue(handshakefds[0]) < 0) {
@ -1670,7 +1670,7 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_UNKNOWN);
if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm)))

View File

@ -888,7 +888,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
skip_instantiate:
VIR_FREE(ipl);
virAtomicIntDecAndTest(&virNWFilterSnoopState.nLeases);
ignore_value(virAtomicIntDecAndTest(&virNWFilterSnoopState.nLeases));
lease_not_found:
VIR_FREE(ipstr);
@ -1167,7 +1167,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque)
_("Instantiation of rules failed on "
"interface '%s'"), req->binding->portdevname);
}
virAtomicIntDecAndTest(job->qCtr);
ignore_value(virAtomicIntDecAndTest(job->qCtr));
VIR_FREE(job);
}
@ -1568,7 +1568,7 @@ virNWFilterDHCPSnoopThread(void *req0)
pcap_close(pcapConf[i].handle);
}
virAtomicIntDecAndTest(&virNWFilterSnoopState.nThreads);
ignore_value(virAtomicIntDecAndTest(&virNWFilterSnoopState.nThreads));
return;
}

View File

@ -5571,7 +5571,7 @@ qemuProcessInit(virQEMUDriverPtr driver,
qemuDomainSetFakeReboot(driver, vm, false);
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
/* Run an early hook to set-up missing devices */
@ -8139,7 +8139,7 @@ qemuProcessReconnect(void *opaque)
goto error;
}
if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
cleanup:

View File

@ -1,11 +1,7 @@
/*
* viratomic.h: atomic integer operations
*
* Copyright (C) 2012 Red Hat, Inc.
*
* Based on code taken from GLib 2.32, under the LGPLv2+
*
* Copyright (C) 2011 Ryan Lortie
* Copyright (C) 2012-2020 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@ -21,18 +17,15 @@
* License along with this library. If not, see
* <http://www.gnu.org/licenses/>.
*
* APIs in this header should no longer be used. Direct
* use of the g_atomic APIs is preferred & existing code
* should be converted as needed.
*/
#pragma once
#include "internal.h"
#ifdef VIR_ATOMIC_OPS_GCC
# define VIR_STATIC /* Nothing; we just never define the functions */
#else
# define VIR_STATIC static
#endif
/**
* virAtomicIntGet:
* Gets the current value of atomic.
@ -40,8 +33,7 @@
* This call acts as a full compiler and hardware memory barrier
* (before the get)
*/
VIR_STATIC int virAtomicIntGet(volatile int *atomic)
ATTRIBUTE_NONNULL(1);
#define virAtomicIntGet(v) g_atomic_int_get(v)
/**
* virAtomicIntSet:
@ -50,21 +42,18 @@ VIR_STATIC int virAtomicIntGet(volatile int *atomic)
* This call acts as a full compiler and hardware memory barrier
* (after the set)
*/
VIR_STATIC void virAtomicIntSet(volatile int *atomic,
int newval)
ATTRIBUTE_NONNULL(1);
#define virAtomicIntSet(i, newv) g_atomic_int_set(i, newv)
/**
* virAtomicIntInc:
* Increments the value of atomic by 1.
*
* Think of this operation as an atomic version of
* { *atomic += 1; return *atomic; }
* { tmp = *atomic; *atomic += 1; return tmp; }
*
* This call acts as a full compiler and hardware memory barrier.
*/
VIR_STATIC int virAtomicIntInc(volatile int *atomic)
ATTRIBUTE_NONNULL(1);
#define virAtomicIntInc(i) g_atomic_int_add(i, 1)
/**
* virAtomicIntDecAndTest:
@ -75,8 +64,7 @@ VIR_STATIC int virAtomicIntInc(volatile int *atomic)
*
* This call acts as a full compiler and hardware memory barrier.
*/
VIR_STATIC bool virAtomicIntDecAndTest(volatile int *atomic)
ATTRIBUTE_NONNULL(1);
#define virAtomicIntDecAndTest(i) (!!g_atomic_int_dec_and_test(i))
/**
* virAtomicIntCompareExchange:
@ -91,10 +79,8 @@ VIR_STATIC bool virAtomicIntDecAndTest(volatile int *atomic)
*
* This call acts as a full compiler and hardware memory barrier.
*/
VIR_STATIC bool virAtomicIntCompareExchange(volatile int *atomic,
int oldval,
int newval)
ATTRIBUTE_NONNULL(1);
#define virAtomicIntCompareExchange(i, oldi, newi) \
(!!g_atomic_int_compare_and_exchange(i, oldi, newi))
/**
* virAtomicIntAdd:
@ -105,9 +91,7 @@ VIR_STATIC bool virAtomicIntCompareExchange(volatile int *atomic,
*
* This call acts as a full compiler and hardware memory barrier.
*/
VIR_STATIC int virAtomicIntAdd(volatile int *atomic,
int val)
ATTRIBUTE_NONNULL(1);
#define virAtomicIntAdd(i, v) g_atomic_int_add(i, v)
/**
* virAtomicIntAnd:
@ -119,9 +103,7 @@ VIR_STATIC int virAtomicIntAdd(volatile int *atomic,
* Think of this operation as an atomic version of
* { tmp = *atomic; *atomic &= val; return tmp; }
*/
VIR_STATIC unsigned int virAtomicIntAnd(volatile unsigned int *atomic,
unsigned int val)
ATTRIBUTE_NONNULL(1);
#define virAtomicIntAnd(i, v) g_atomic_int_and(i, v)
/**
* virAtomicIntOr:
@ -133,9 +115,7 @@ VIR_STATIC unsigned int virAtomicIntAnd(volatile unsigned int *atomic,
*
* This call acts as a full compiler and hardware memory barrier.
*/
VIR_STATIC unsigned int virAtomicIntOr(volatile unsigned int *atomic,
unsigned int val)
ATTRIBUTE_NONNULL(1);
#define virAtomicIntOr(i, v) g_atomic_int_or(i, v)
/**
* virAtomicIntXor:
@ -147,305 +127,4 @@ VIR_STATIC unsigned int virAtomicIntOr(volatile unsigned int *atomic,
*
* This call acts as a full compiler and hardware memory barrier.
*/
VIR_STATIC unsigned int virAtomicIntXor(volatile unsigned int *atomic,
unsigned int val)
ATTRIBUTE_NONNULL(1);
#undef VIR_STATIC
#ifdef VIR_ATOMIC_OPS_GCC
# define virAtomicIntGet(atomic) \
(__extension__ ({ \
(void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \
(void)(0 ? *(atomic) ^ *(atomic) : 0); \
__sync_synchronize(); \
(int)*(atomic); \
}))
# define virAtomicIntSet(atomic, newval) \
(__extension__ ({ \
(void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \
(void)(0 ? *(atomic) ^ (newval) : 0); \
*(atomic) = (newval); \
__sync_synchronize(); \
}))
# define virAtomicIntInc(atomic) \
(__extension__ ({ \
(void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \
(void)(0 ? *(atomic) ^ *(atomic) : 0); \
__sync_add_and_fetch((atomic), 1); \
}))
# define virAtomicIntDecAndTest(atomic) \
(__extension__ ({ \
(void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \
(void)(0 ? *(atomic) ^ *(atomic) : 0); \
__sync_fetch_and_sub((atomic), 1) == 1; \
}))
# define virAtomicIntCompareExchange(atomic, oldval, newval) \
(__extension__ ({ \
(void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \
(void)(0 ? *(atomic) ^ (newval) ^ (oldval) : 0); \
(bool)__sync_bool_compare_and_swap((atomic), \
(oldval), (newval)); \
}))
# define virAtomicIntAdd(atomic, val) \
(__extension__ ({ \
(void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \
(void)(0 ? *(atomic) ^ (val) : 0); \
(int) __sync_fetch_and_add((atomic), (val)); \
}))
# define virAtomicIntAnd(atomic, val) \
(__extension__ ({ \
(void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \
(void) (0 ? *(atomic) ^ (val) : 0); \
(unsigned int) __sync_fetch_and_and((atomic), (val)); \
}))
# define virAtomicIntOr(atomic, val) \
(__extension__ ({ \
(void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \
(void) (0 ? *(atomic) ^ (val) : 0); \
(unsigned int) __sync_fetch_and_or((atomic), (val)); \
}))
# define virAtomicIntXor(atomic, val) \
(__extension__ ({ \
(void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \
(void) (0 ? *(atomic) ^ (val) : 0); \
(unsigned int) __sync_fetch_and_xor((atomic), (val)); \
}))
#else
# ifdef VIR_ATOMIC_OPS_WIN32
# include <winsock2.h>
# include <windows.h>
# include <intrin.h>
# if !defined(_M_AMD64) && !defined (_M_IA64) && !defined(_M_X64)
# define InterlockedAnd _InterlockedAnd
# define InterlockedOr _InterlockedOr
# define InterlockedXor _InterlockedXor
# endif
/*
* http://msdn.microsoft.com/en-us/library/ms684122(v=vs.85).aspx
*/
static inline int
virAtomicIntGet(volatile int *atomic)
{
MemoryBarrier();
return *atomic;
}
static inline void
virAtomicIntSet(volatile int *atomic,
int newval)
{
*atomic = newval;
MemoryBarrier();
}
static inline int
virAtomicIntInc(volatile int *atomic)
{
return InterlockedIncrement((volatile LONG *)atomic);
}
static inline bool
virAtomicIntDecAndTest(volatile int *atomic)
{
return InterlockedDecrement((volatile LONG *)atomic) == 0;
}
static inline bool
virAtomicIntCompareExchange(volatile int *atomic,
int oldval,
int newval)
{
return InterlockedCompareExchange((volatile LONG *)atomic, newval, oldval) == oldval;
}
static inline int
virAtomicIntAdd(volatile int *atomic,
int val)
{
return InterlockedExchangeAdd((volatile LONG *)atomic, val);
}
static inline unsigned int
virAtomicIntAnd(volatile unsigned int *atomic,
unsigned int val)
{
return InterlockedAnd((volatile LONG *)atomic, val);
}
static inline unsigned int
virAtomicIntOr(volatile unsigned int *atomic,
unsigned int val)
{
return InterlockedOr((volatile LONG *)atomic, val);
}
static inline unsigned int
virAtomicIntXor(volatile unsigned int *atomic,
unsigned int val)
{
return InterlockedXor((volatile LONG *)atomic, val);
}
# else
# ifdef VIR_ATOMIC_OPS_PTHREAD
# include <pthread.h>
extern pthread_mutex_t virAtomicLock;
static inline int
virAtomicIntGet(volatile int *atomic)
{
int value;
pthread_mutex_lock(&virAtomicLock);
value = *atomic;
pthread_mutex_unlock(&virAtomicLock);
return value;
}
static inline void
virAtomicIntSet(volatile int *atomic,
int value)
{
pthread_mutex_lock(&virAtomicLock);
*atomic = value;
pthread_mutex_unlock(&virAtomicLock);
}
static inline int
virAtomicIntInc(volatile int *atomic)
{
int value;
pthread_mutex_lock(&virAtomicLock);
value = ++(*atomic);
pthread_mutex_unlock(&virAtomicLock);
return value;
}
static inline bool
virAtomicIntDecAndTest(volatile int *atomic)
{
bool is_zero;
pthread_mutex_lock(&virAtomicLock);
is_zero = --(*atomic) == 0;
pthread_mutex_unlock(&virAtomicLock);
return is_zero;
}
static inline bool
virAtomicIntCompareExchange(volatile int *atomic,
int oldval,
int newval)
{
bool success;
pthread_mutex_lock(&virAtomicLock);
if ((success = (*atomic == oldval)))
*atomic = newval;
pthread_mutex_unlock(&virAtomicLock);
return success;
}
static inline int
virAtomicIntAdd(volatile int *atomic,
int val)
{
int oldval;
pthread_mutex_lock(&virAtomicLock);
oldval = *atomic;
*atomic = oldval + val;
pthread_mutex_unlock(&virAtomicLock);
return oldval;
}
static inline unsigned int
virAtomicIntAnd(volatile unsigned int *atomic,
unsigned int val)
{
unsigned int oldval;
pthread_mutex_lock(&virAtomicLock);
oldval = *atomic;
*atomic = oldval & val;
pthread_mutex_unlock(&virAtomicLock);
return oldval;
}
static inline unsigned int
virAtomicIntOr(volatile unsigned int *atomic,
unsigned int val)
{
unsigned int oldval;
pthread_mutex_lock(&virAtomicLock);
oldval = *atomic;
*atomic = oldval | val;
pthread_mutex_unlock(&virAtomicLock);
return oldval;
}
static inline unsigned int
virAtomicIntXor(volatile unsigned int *atomic,
unsigned int val)
{
unsigned int oldval;
pthread_mutex_lock(&virAtomicLock);
oldval = *atomic;
*atomic = oldval ^ val;
pthread_mutex_unlock(&virAtomicLock);
return oldval;
}
# else
# error "No atomic integer impl for this platform"
# endif
# endif
/* The int/unsigned int casts here ensure that you can
* pass either an int or unsigned int to all atomic op
* functions, in the same way that we can with GCC
* atomic op helpers.
*/
# define virAtomicIntGet(atomic) \
virAtomicIntGet((int *)atomic)
# define virAtomicIntSet(atomic, val) \
virAtomicIntSet((int *)atomic, val)
# define virAtomicIntInc(atomic) \
virAtomicIntInc((int *)atomic)
# define virAtomicIntDecAndTest(atomic) \
virAtomicIntDecAndTest((int *)atomic)
# define virAtomicIntCompareExchange(atomic, oldval, newval) \
virAtomicIntCompareExchange((int *)atomic, oldval, newval)
# define virAtomicIntAdd(atomic, val) \
virAtomicIntAdd((int *)atomic, val)
# define virAtomicIntAnd(atomic, val) \
virAtomicIntAnd((unsigned int *)atomic, val)
# define virAtomicIntOr(atomic, val) \
virAtomicIntOr((unsigned int *)atomic, val)
# define virAtomicIntXor(atomic, val) \
virAtomicIntXor((unsigned int *)atomic, val)
#endif
#define virAtomicIntXor(i, v) g_atomic_int_xor(i, v)

View File

@ -1021,7 +1021,7 @@ int virProcessGetStartTime(pid_t pid,
unsigned long long *timestamp)
{
static int warned;
if (virAtomicIntInc(&warned) == 1) {
if (virAtomicIntInc(&warned) == 0) {
VIR_WARN("Process start time of pid %lld not available on this platform",
(long long) pid);
}

View File

@ -50,7 +50,7 @@ testTypes(const void *data G_GNUC_UNUSED)
testAssertEq(virAtomicIntAdd(&u, 1), 5);
testAssertEq(u, 6);
testAssertEq(virAtomicIntInc(&u), 7);
testAssertEq(virAtomicIntInc(&u), 6);
testAssertEq(u, 7);
res = virAtomicIntDecAndTest(&u);