fifo: isolate atomic and futex operations
Now all atomic and futex operations occur in fifo_index.cpp.
This implements these TODOs:
- Abstract out atomic operations to audio_utils_fifo_index
- Replace friend by setter and getter, and abstract the futex
This is one of a series of CLs to isolate the dependencies.
Test: builds OK on Android, host Linux, and host macOS
Change-Id: Ic4ec46d472c583dce8525f11ed8cb1db89928d30
diff --git a/audio_utils/Android.bp b/audio_utils/Android.bp
index fa8fc42..11ed72c 100644
--- a/audio_utils/Android.bp
+++ b/audio_utils/Android.bp
@@ -20,6 +20,7 @@
srcs: [
"channels.c",
"fifo.cpp",
+ "fifo_index.cpp",
"format.c",
"limiter.c",
"minifloat.c",
@@ -78,6 +79,7 @@
defaults: ["audio_utils_defaults"],
srcs: [
"fifo.cpp",
+ "fifo_index.cpp",
"primitives.c",
"roundup.c",
],
diff --git a/audio_utils/fifo.cpp b/audio_utils/fifo.cpp
index 45e6488..8f89318 100644
--- a/audio_utils/fifo.cpp
+++ b/audio_utils/fifo.cpp
@@ -194,7 +194,7 @@
int retries = kRetries;
uint32_t front;
for (;;) {
- front = atomic_load_explicit(&mFifo.mThrottleFront->mIndex, std::memory_order_acquire);
+ front = mFifo.mThrottleFront->loadAcquire();
// returns -EIO if mIsShutdown
int32_t filled = mFifo.diff(mLocalRear, front);
if (filled < 0) {
@@ -233,7 +233,7 @@
if (timeout->tv_sec == LONG_MAX) {
timeout = NULL;
}
- err = sys_futex(&mFifo.mThrottleFront->mIndex, op, front, timeout, NULL, 0);
+ err = mFifo.mThrottleFront->wait(op, front, timeout);
if (err < 0) {
switch (errno) {
case EWOULDBLOCK:
@@ -300,13 +300,11 @@
return;
}
if (mFifo.mThrottleFront != NULL) {
- uint32_t front = atomic_load_explicit(&mFifo.mThrottleFront->mIndex,
- std::memory_order_acquire);
+ uint32_t front = mFifo.mThrottleFront->loadAcquire();
// returns -EIO if mIsShutdown
int32_t filled = mFifo.diff(mLocalRear, front);
mLocalRear = mFifo.sum(mLocalRear, count);
- atomic_store_explicit(&mFifo.mWriterRear.mIndex, mLocalRear,
- std::memory_order_release);
+ mFifo.mWriterRear.storeRelease(mLocalRear);
// TODO add comments
int op = FUTEX_WAKE;
switch (mFifo.mWriterRearSync) {
@@ -321,8 +319,7 @@
mIsArmed = true;
}
if (mIsArmed && filled + count > mTriggerLevel) {
- int err = sys_futex(&mFifo.mWriterRear.mIndex,
- op, INT32_MAX /*waiters*/, NULL, NULL, 0);
+ int err = mFifo.mWriterRear.wake(op, INT32_MAX /*waiters*/);
// err is number of processes woken up
if (err < 0) {
LOG_ALWAYS_FATAL("%s: unexpected err=%d errno=%d",
@@ -338,8 +335,7 @@
}
} else {
mLocalRear = mFifo.sum(mLocalRear, count);
- atomic_store_explicit(&mFifo.mWriterRear.mIndex, mLocalRear,
- std::memory_order_release);
+ mFifo.mWriterRear.storeRelease(mLocalRear);
}
mObtained -= count;
mTotalReleased += count;
@@ -451,13 +447,11 @@
return;
}
if (mThrottleFront != NULL) {
- uint32_t rear = atomic_load_explicit(&mFifo.mWriterRear.mIndex,
- std::memory_order_acquire);
+ uint32_t rear = mFifo.mWriterRear.loadAcquire();
// returns -EIO if mIsShutdown
int32_t filled = mFifo.diff(rear, mLocalFront);
mLocalFront = mFifo.sum(mLocalFront, count);
- atomic_store_explicit(&mThrottleFront->mIndex, mLocalFront,
- std::memory_order_release);
+ mThrottleFront->storeRelease(mLocalFront);
// TODO add comments
int op = FUTEX_WAKE;
switch (mFifo.mThrottleFrontSync) {
@@ -472,8 +466,7 @@
mIsArmed = true;
}
if (mIsArmed && filled - count < mTriggerLevel) {
- int err = sys_futex(&mThrottleFront->mIndex,
- op, 1 /*waiters*/, NULL, NULL, 0);
+ int err = mThrottleFront->wake(op, 1 /*waiters*/);
// err is number of processes woken up
if (err < 0 || err > 1) {
LOG_ALWAYS_FATAL("%s: unexpected err=%d errno=%d",
@@ -504,8 +497,7 @@
int retries = kRetries;
uint32_t rear;
for (;;) {
- rear = atomic_load_explicit(&mFifo.mWriterRear.mIndex,
- std::memory_order_acquire);
+ rear = mFifo.mWriterRear.loadAcquire();
// TODO pull out "count == 0"
if (count == 0 || rear != mLocalFront || timeout == NULL ||
(timeout->tv_sec == 0 && timeout->tv_nsec == 0)) {
@@ -531,7 +523,7 @@
if (timeout->tv_sec == LONG_MAX) {
timeout = NULL;
}
- err = sys_futex(&mFifo.mWriterRear.mIndex, op, rear, timeout, NULL, 0);
+ err = mFifo.mWriterRear.wait(op, rear, timeout);
if (err < 0) {
switch (errno) {
case EWOULDBLOCK:
diff --git a/audio_utils/fifo_index.cpp b/audio_utils/fifo_index.cpp
new file mode 100644
index 0000000..b35319c
--- /dev/null
+++ b/audio_utils/fifo_index.cpp
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <audio_utils/fifo_index.h>
+#include <audio_utils/futex.h>
+
+// These are not implemented within <audio_utils/fifo_index.h>
+// so that we don't expose futex.
+
+uint32_t audio_utils_fifo_index::loadAcquire()
+{
+ return atomic_load_explicit(&mIndex, std::memory_order_acquire);
+}
+
+void audio_utils_fifo_index::storeRelease(uint32_t value)
+{
+ atomic_store_explicit(&mIndex, value, std::memory_order_release);
+}
+
+int audio_utils_fifo_index::wait(int op, uint32_t expected, const struct timespec *timeout)
+{
+ return sys_futex(&mIndex, op, expected, timeout, NULL, 0);
+}
+
+int audio_utils_fifo_index::wake(int op, int waiters)
+{
+ return sys_futex(&mIndex, op, waiters, NULL, NULL, 0);
+}
diff --git a/audio_utils/include/audio_utils/fifo.h b/audio_utils/include/audio_utils/fifo.h
index d7d8173..c0cde32 100644
--- a/audio_utils/include/audio_utils/fifo.h
+++ b/audio_utils/include/audio_utils/fifo.h
@@ -19,36 +19,12 @@
#include <atomic>
#include <stdlib.h>
+#include <audio_utils/fifo_index.h>
#ifndef __cplusplus
#error C API is no longer supported
#endif
-/**
- * An index that may optionally be placed in shared memory.
- * Must be Plain Old Data (POD), so no virtual methods are allowed.
- * If in shared memory, exactly one process must explicitly call the constructor via placement new.
- * \see #audio_utils_fifo_sync
- */
-struct audio_utils_fifo_index {
- friend class audio_utils_fifo_reader;
- friend class audio_utils_fifo_writer;
-
-public:
- audio_utils_fifo_index() : mIndex(0) { }
- ~audio_utils_fifo_index() { }
-
-private:
- // Linux futex is 32 bits regardless of platform.
- // It would make more sense to declare this as atomic_uint32_t, but there is no such type name.
- // TODO Support 64-bit index with 32-bit futex in low-order bits.
- std::atomic_uint_least32_t mIndex; // accessed by both sides using atomic operations
- static_assert(sizeof(mIndex) == sizeof(uint32_t), "mIndex must be 32 bits");
-
- // TODO Abstract out atomic operations to here
- // TODO Replace friend by setter and getter, and abstract the futex
-};
-
/** Indicates whether an index is also used for synchronization. */
enum audio_utils_fifo_sync {
/** Index is not also used for synchronization; timeouts are done via clock_nanosleep(). */
diff --git a/audio_utils/include/audio_utils/fifo_index.h b/audio_utils/include/audio_utils/fifo_index.h
new file mode 100644
index 0000000..55eb0b4
--- /dev/null
+++ b/audio_utils/include/audio_utils/fifo_index.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ANDROID_AUDIO_FIFO_INDEX_H
+#define ANDROID_AUDIO_FIFO_INDEX_H
+
+#include <atomic>
+#include <stdint.h>
+#include <time.h>
+
+/**
+ * An index that may optionally be placed in shared memory.
+ * Must be Plain Old Data (POD), so no virtual methods are allowed.
+ * If in shared memory, exactly one process must explicitly call the constructor via placement new.
+ * \see #audio_utils_fifo_sync
+ */
+struct audio_utils_fifo_index {
+
+public:
+ audio_utils_fifo_index() : mIndex(0) { }
+ ~audio_utils_fifo_index() { }
+
+ uint32_t loadAcquire();
+ void storeRelease(uint32_t value);
+ int wait(int op, uint32_t expected, const struct timespec *timeout);
+ int wake(int op, int waiters);
+
+private:
+ // Linux futex is 32 bits regardless of platform.
+ // It would make more sense to declare this as atomic_uint32_t, but there is no such type name.
+ // TODO Support 64-bit index with 32-bit futex in low-order bits.
+ std::atomic_uint_least32_t mIndex; // accessed by both sides using atomic operations
+ static_assert(sizeof(mIndex) == sizeof(uint32_t), "mIndex must be 32 bits");
+};
+
+#endif // !ANDROID_AUDIO_FIFO_INDEX_H