[pulseaudio-discuss] [PATCH 04/11] pulsecore: Split pa_shm mempool backend into pa_shm and pa_privatemem

Ahmed S. Darwish darwish.07 at gmail.com
Sun Sep 20 14:30:44 PDT 2015


Mempools use "pa_shm" for their pools memory allocation. pa_shm is
then overloaded with two unrelated tasks: either allocating the pool
using posix SHM or allocating the same pool using basic malloc()s. The
choice depends on system configuration and abilities.

This will not scale when a third pool memory allocation mechanism
(memfds) is added. Thus, split pa_shm responsibility into:

  - pa_shm: only responsible for posix SHM allocations
  - pa_privatemem: only responsible for private memory allocations

This way, a pa_memfd mempool backend can be added easily in the
furture.

Signed-off-by: Ahmed S. Darwish <darwish.07 at gmail.com>
---
 src/Makefile.am            |   1 +
 src/pulsecore/memblock.c   |  12 +++--
 src/pulsecore/privatemem.c |  82 ++++++++++++++++++++++++++++++++
 src/pulsecore/privatemem.h |  34 ++++++++++++++
 src/pulsecore/shm.c        | 115 ++++++++++++++-------------------------------
 src/pulsecore/shm.h        |  10 ++--
 6 files changed, 164 insertions(+), 90 deletions(-)
 create mode 100644 src/pulsecore/privatemem.c
 create mode 100644 src/pulsecore/privatemem.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 839078a..19b335c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -698,6 +698,7 @@ libpulsecommon_ at PA_MAJORMINOR@_la_SOURCES = \
 		pulsecore/srbchannel.c pulsecore/srbchannel.h \
 		pulsecore/sample-util.c pulsecore/sample-util.h \
 		pulsecore/shm.c pulsecore/shm.h \
+		pulsecore/privatemem.c pulsecore/privatemem.h \
 		pulsecore/bitset.c pulsecore/bitset.h \
 		pulsecore/socket-client.c pulsecore/socket-client.h \
 		pulsecore/socket-server.c pulsecore/socket-server.h \
diff --git a/src/pulsecore/memblock.c b/src/pulsecore/memblock.c
index aa64d01..d66cd52 100644
--- a/src/pulsecore/memblock.c
+++ b/src/pulsecore/memblock.c
@@ -37,6 +37,7 @@
 #include <pulse/def.h>
 
 #include <pulsecore/shm.h>
+#include <pulsecore/privatemem.h>
 #include <pulsecore/log.h>
 #include <pulsecore/hashmap.h>
 #include <pulsecore/semaphore.h>
@@ -148,6 +149,7 @@ struct pa_mempool {
     pa_mempool_type_t type;
     union {
         pa_shm shm;
+        pa_privatemem privatemem;
     } per_type;
 
     void *ptr;
@@ -789,7 +791,7 @@ pa_mempool *pa_mempool_new(pa_mempool_type_t type, size_t size) {
 
     switch (type) {
     case PA_MEMPOOL_SHARED_POSIX:
-        if (pa_shm_create_rw(&p->per_type.shm, pa_mem_size, true, 0700) < 0)
+        if (pa_shm_create(&p->per_type.shm, pa_mem_size, 0700) < 0)
             goto fail;
 
         p->ptr = p->per_type.shm.ptr;
@@ -799,11 +801,11 @@ pa_mempool *pa_mempool_new(pa_mempool_type_t type, size_t size) {
         pa_assert_not_reached();
         break;
     case PA_MEMPOOL_PRIVATE:
-        if (pa_shm_create_rw(&p->per_type.shm, pa_mem_size, false, 0700) < 0)
+        if (pa_privatemem_create(&p->per_type.privatemem, pa_mem_size) < 0)
             goto fail;
 
-        p->ptr = p->per_type.shm.ptr;
-        p->size = p->per_type.shm.size;
+        p->ptr = p->per_type.privatemem.ptr;
+        p->size = p->per_type.privatemem.size;
         break;
     default:
         pa_assert_not_reached();
@@ -901,7 +903,7 @@ void pa_mempool_free(pa_mempool *p) {
         pa_assert_not_reached();
         break;
     case PA_MEMPOOL_PRIVATE:
-        pa_shm_free(&p->per_type.shm);
+        pa_privatemem_free(&p->per_type.privatemem);
         break;
     default:
         pa_assert_not_reached();
diff --git a/src/pulsecore/privatemem.c b/src/pulsecore/privatemem.c
new file mode 100644
index 0000000..7c098d6
--- /dev/null
+++ b/src/pulsecore/privatemem.c
@@ -0,0 +1,82 @@
+/***
+    This file is part of PulseAudio.
+
+    Copyright 2006 Lennart Poettering
+    Copyright 2006 Pierre Ossman <ossman at cendio.se> for Cendio AB
+
+    PulseAudio is free software; you can redistribute it and/or modify
+    it under the terms of the GNU Lesser General Public License as
+    published by the Free Software Foundation; either version 2.1 of the
+    License, or (at your option) any later version.
+
+    PulseAudio is distributed in the hope that it will be useful, but
+    WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+    Lesser General Public License for more details.
+
+    You should have received a copy of the GNU Lesser General Public
+    License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
+***/
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <pulsecore/shm.h>
+#include <pulsecore/privatemem.h>
+#include <pulsecore/core-error.h>
+#include <pulse/xmalloc.h>
+
+#define MAX_MEM_SIZE    MAX_SHM_SIZE
+
+int pa_privatemem_create(pa_privatemem *m, size_t size) {
+    pa_assert(m);
+    pa_assert(size > 0);
+    pa_assert(size <= MAX_MEM_SIZE);
+
+    /* Each time we create a new SHM area, let's first drop all stale
+     * ones */
+    pa_shm_cleanup();
+
+    /* Round up to make it page aligned */
+    size = PA_PAGE_ALIGN(size);
+
+    m->size = size;
+
+#ifdef MAP_ANONYMOUS
+    if ((m->ptr = mmap(NULL, m->size, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, -1, (off_t) 0)) == MAP_FAILED) {
+        pa_log("mmap() failed: %s", pa_cstrerror(errno));
+        goto fail;
+    }
+#elif defined(HAVE_POSIX_MEMALIGN)
+    {
+        int r;
+
+        if ((r = posix_memalign(&m->ptr, PA_PAGE_SIZE, size)) < 0) {
+            pa_log("posix_memalign() failed: %s", pa_cstrerror(r));
+            goto fail;
+        }
+    }
+#else
+    m->ptr = pa_xmalloc(m->size);
+#endif
+
+    return 0;
+fail:
+    return -1;
+}
+
+void pa_privatemem_free(pa_privatemem *m) {
+    pa_assert(m);
+    pa_assert(m->ptr);
+    pa_assert(m->size > 0);
+
+#ifdef MAP_ANONYMOUS
+    if (munmap(m->ptr, m->size) < 0)
+        pa_log("munmap() failed: %s", pa_cstrerror(errno));
+#elif defined(HAVE_POSIX_MEMALIGN)
+    free(m->ptr);
+#else
+    pa_xfree(m->ptr);
+#endif
+}
diff --git a/src/pulsecore/privatemem.h b/src/pulsecore/privatemem.h
new file mode 100644
index 0000000..a6ae710
--- /dev/null
+++ b/src/pulsecore/privatemem.h
@@ -0,0 +1,34 @@
+#ifndef SRC_PULSECORE_PRIVATEMEM_H_
+#define SRC_PULSECORE_PRIVATEMEM_H_
+
+/***
+  This file is part of PulseAudio.
+
+  Copyright 2015 Ahmed S. Darwish <darwish.07 at gmail.com>
+
+  PulseAudio is free software; you can redistribute it and/or modify
+  it under the terms of the GNU Lesser General Public License as
+  published by the Free Software Foundation; either version 2.1 of the
+  License, or (at your option) any later version.
+
+  PulseAudio is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public
+  License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
+***/
+
+/* Private memory for the mempools. This is usually used if posix SHM
+ * or Linux memfds are not available, or if we're explicitly asked
+ * not to use any form of shared memory. */
+typedef struct pa_privatemem {
+    void *ptr;
+    size_t size;
+} pa_privatemem;
+
+int pa_privatemem_create(pa_privatemem *m, size_t size);
+void pa_privatemem_free(pa_privatemem *m);
+
+#endif /* SRC_PULSECORE_PRIVATEMEM_H_ */
diff --git a/src/pulsecore/shm.c b/src/pulsecore/shm.c
index 0c14e70..79fde5b 100644
--- a/src/pulsecore/shm.c
+++ b/src/pulsecore/shm.c
@@ -58,9 +58,6 @@
 #define MADV_REMOVE 9
 #endif
 
-/* 1 GiB at max */
-#define MAX_SHM_SIZE (PA_ALIGN(1024*1024*1024))
-
 #ifdef __linux__
 /* On Linux we know that the shared memory blocks are files in
  * /dev/shm. We can use that information to list all blocks and
@@ -100,11 +97,12 @@ static char *segment_name(char *fn, size_t l, unsigned id) {
 }
 #endif
 
-int pa_shm_create_rw(pa_shm *m, size_t size, bool shared, mode_t mode) {
+int pa_shm_create(pa_shm *m, size_t size, mode_t mode) {
 #ifdef HAVE_SHM_OPEN
     char fn[32];
     int fd = -1;
 #endif
+    struct shm_marker *marker;
 
     pa_assert(m);
     pa_assert(size > 0);
@@ -119,72 +117,42 @@ int pa_shm_create_rw(pa_shm *m, size_t size, bool shared, mode_t mode) {
     /* Round up to make it page aligned */
     size = PA_PAGE_ALIGN(size);
 
-    if (!shared) {
-        m->id = 0;
-        m->size = size;
-
-#ifdef MAP_ANONYMOUS
-        if ((m->ptr = mmap(NULL, m->size, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, -1, (off_t) 0)) == MAP_FAILED) {
-            pa_log("mmap() failed: %s", pa_cstrerror(errno));
-            goto fail;
-        }
-#elif defined(HAVE_POSIX_MEMALIGN)
-        {
-            int r;
-
-            if ((r = posix_memalign(&m->ptr, PA_PAGE_SIZE, size)) < 0) {
-                pa_log("posix_memalign() failed: %s", pa_cstrerror(r));
-                goto fail;
-            }
-        }
-#else
-        m->ptr = pa_xmalloc(m->size);
-#endif
-
-        m->do_unlink = false;
-
-    } else {
 #ifdef HAVE_SHM_OPEN
-        struct shm_marker *marker;
+    pa_random(&m->id, sizeof(m->id));
+    segment_name(fn, sizeof(fn), m->id);
 
-        pa_random(&m->id, sizeof(m->id));
-        segment_name(fn, sizeof(fn), m->id);
-
-        if ((fd = shm_open(fn, O_RDWR|O_CREAT|O_EXCL, mode)) < 0) {
-            pa_log("shm_open() failed: %s", pa_cstrerror(errno));
-            goto fail;
-        }
+    if ((fd = shm_open(fn, O_RDWR|O_CREAT|O_EXCL, mode)) < 0) {
+        pa_log("shm_open() failed: %s", pa_cstrerror(errno));
+        goto fail;
+    }
 
-        m->size = size + SHM_MARKER_SIZE;
+    m->size = size + SHM_MARKER_SIZE;
 
-        if (ftruncate(fd, (off_t) m->size) < 0) {
-            pa_log("ftruncate() failed: %s", pa_cstrerror(errno));
-            goto fail;
-        }
+    if (ftruncate(fd, (off_t) m->size) < 0) {
+        pa_log("ftruncate() failed: %s", pa_cstrerror(errno));
+        goto fail;
+    }
 
 #ifndef MAP_NORESERVE
 #define MAP_NORESERVE 0
 #endif
 
-        if ((m->ptr = mmap(NULL, PA_PAGE_ALIGN(m->size), PROT_READ|PROT_WRITE, MAP_SHARED|MAP_NORESERVE, fd, (off_t) 0)) == MAP_FAILED) {
-            pa_log("mmap() failed: %s", pa_cstrerror(errno));
-            goto fail;
-        }
+    if ((m->ptr = mmap(NULL, PA_PAGE_ALIGN(m->size), PROT_READ|PROT_WRITE, MAP_SHARED|MAP_NORESERVE, fd, (off_t) 0)) == MAP_FAILED) {
+        pa_log("mmap() failed: %s", pa_cstrerror(errno));
+        goto fail;
+    }
 
-        /* We store our PID at the end of the shm block, so that we
-         * can check for dead shm segments later */
-        marker = (struct shm_marker*) ((uint8_t*) m->ptr + m->size - SHM_MARKER_SIZE);
-        pa_atomic_store(&marker->pid, (int) getpid());
-        pa_atomic_store(&marker->marker, SHM_MARKER);
+    /* We store our PID at the end of the shm block, so that we
+     * can check for dead shm segments later */
+    marker = (struct shm_marker*) ((uint8_t*) m->ptr + m->size - SHM_MARKER_SIZE);
+    pa_atomic_store(&marker->pid, (int) getpid());
+    pa_atomic_store(&marker->marker, SHM_MARKER);
 
-        pa_assert_se(pa_close(fd) == 0);
-        m->do_unlink = true;
+    pa_assert_se(pa_close(fd) == 0);
+    m->do_unlink = true;
 #else
-        goto fail;
+    goto fail;
 #endif
-    }
-
-    m->shared = shared;
 
     return 0;
 
@@ -209,33 +177,21 @@ void pa_shm_free(pa_shm *m) {
     pa_assert(m->ptr != MAP_FAILED);
 #endif
 
-    if (!m->shared) {
-#ifdef MAP_ANONYMOUS
-        if (munmap(m->ptr, m->size) < 0)
-            pa_log("munmap() failed: %s", pa_cstrerror(errno));
-#elif defined(HAVE_POSIX_MEMALIGN)
-        free(m->ptr);
-#else
-        pa_xfree(m->ptr);
-#endif
-    } else {
 #ifdef HAVE_SHM_OPEN
-        if (munmap(m->ptr, PA_PAGE_ALIGN(m->size)) < 0)
-            pa_log("munmap() failed: %s", pa_cstrerror(errno));
-
-        if (m->do_unlink) {
-            char fn[32];
+    if (munmap(m->ptr, PA_PAGE_ALIGN(m->size)) < 0)
+        pa_log("munmap() failed: %s", pa_cstrerror(errno));
 
-            segment_name(fn, sizeof(fn), m->id);
+    if (m->do_unlink) {
+        char fn[32];
 
-            if (shm_unlink(fn) < 0)
-                pa_log(" shm_unlink(%s) failed: %s", fn, pa_cstrerror(errno));
-        }
+        segment_name(fn, sizeof(fn), m->id);
+        if (shm_unlink(fn) < 0)
+            pa_log(" shm_unlink(%s) failed: %s", fn, pa_cstrerror(errno));
+    }
 #else
-        /* We shouldn't be here without shm support */
-        pa_assert_not_reached();
+    /* We shouldn't be here without shm support */
+    pa_assert_not_reached();
 #endif
-    }
 
     pa_zero(*m);
 }
@@ -324,7 +280,6 @@ int pa_shm_attach(pa_shm *m, unsigned id, bool writable) {
     }
 
     m->do_unlink = false;
-    m->shared = true;
 
     pa_assert_se(pa_close(fd) == 0);
 
diff --git a/src/pulsecore/shm.h b/src/pulsecore/shm.h
index 9fe8e2f..0e6e9b6 100644
--- a/src/pulsecore/shm.h
+++ b/src/pulsecore/shm.h
@@ -21,7 +21,6 @@
 ***/
 
 #include <sys/types.h>
-
 #include <pulsecore/macro.h>
 
 typedef struct pa_shm {
@@ -29,16 +28,17 @@ typedef struct pa_shm {
     void *ptr;
     size_t size;
     bool do_unlink:1;
-    bool shared:1;
 } pa_shm;
 
-int pa_shm_create_rw(pa_shm *m, size_t size, bool shared, mode_t mode);
+/* 1 GiB at max */
+#define MAX_SHM_SIZE (PA_ALIGN(1024*1024*1024))
+
+int pa_shm_create(pa_shm *m, size_t size, mode_t mode);
 int pa_shm_attach(pa_shm *m, unsigned id, bool writable);
+void pa_shm_free(pa_shm *m);
 
 void pa_shm_punch(void *shm_start, size_t total_shm_size, size_t punch_offset, size_t punch_size);
 
-void pa_shm_free(pa_shm *m);
-
 int pa_shm_cleanup(void);
 
 #endif

-- 
Darwish
http://darwish.chasingpointers.com


More information about the pulseaudio-discuss mailing list