OSDN Git Service

fifo: isolate atomic and futex operations
authorGlenn Kasten <gkasten@google.com>
Fri, 2 Dec 2016 00:12:59 +0000 (16:12 -0800)
committerGlenn Kasten <gkasten@google.com>
Fri, 2 Dec 2016 00:38:05 +0000 (16:38 -0800)
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

audio_utils/Android.bp
audio_utils/fifo.cpp
audio_utils/fifo_index.cpp [new file with mode: 0644]
audio_utils/include/audio_utils/fifo.h
audio_utils/include/audio_utils/fifo_index.h [new file with mode: 0644]

index fa8fc42..11ed72c 100644 (file)
@@ -20,6 +20,7 @@ cc_library {
     srcs: [
         "channels.c",
         "fifo.cpp",
+        "fifo_index.cpp",
         "format.c",
         "limiter.c",
         "minifloat.c",
@@ -78,6 +79,7 @@ cc_library_static {
     defaults: ["audio_utils_defaults"],
     srcs: [
         "fifo.cpp",
+        "fifo_index.cpp",
         "primitives.c",
         "roundup.c",
     ],
index 45e6488..8f89318 100644 (file)
@@ -194,7 +194,7 @@ ssize_t audio_utils_fifo_writer::obtain(audio_utils_iovec iovec[2], size_t count
         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 @@ ssize_t audio_utils_fifo_writer::obtain(audio_utils_iovec iovec[2], size_t count
                 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 @@ void audio_utils_fifo_writer::release(size_t count)
             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 @@ void audio_utils_fifo_writer::release(size_t count)
                         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 @@ void audio_utils_fifo_writer::release(size_t count)
             }
         } 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 @@ void audio_utils_fifo_reader::release(size_t count)
             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 @@ void audio_utils_fifo_reader::release(size_t count)
                         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 @@ ssize_t audio_utils_fifo_reader::obtain(audio_utils_iovec iovec[2], size_t count
     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 @@ ssize_t audio_utils_fifo_reader::obtain(audio_utils_iovec iovec[2], size_t count
             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 (file)
index 0000000..b35319c
--- /dev/null
@@ -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);
+}
index d7d8173..c0cde32 100644 (file)
 
 #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 (file)
index 0000000..55eb0b4
--- /dev/null
@@ -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