On 2016-02-12 01:12, Ahmed S. Darwish wrote:
> 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 private malloc()s.
> The choice depends on system configuration and abilities.
>
> This will not scale when a third pool memory allocation mechanism
> is added (memfds) . Thus, split pa_shm responsibility into:
>
> - pa_shm: responsible for all *shared* memory, posix and memfds
> - pa_privatemem: only responsible for private memory allocations
>
> This way, memfd support can easily be added later.
So, this part I'm still quite hesitant about. It would be easier just to
change pa_shm to look like this, which is similar to I suggested earlier
[1]:
struct pa_shm {
void *ptr;
size_t size;
pa_mem_type_t type;
/* The three fields below are unused if type is PA_MEM_TYPE_PRIVATE */
int fd;
unsigned id;
bool do_unlink; /* Used for PA_MEM_TYPE_SHARED_POSIX only */
}
Instead you did a massive refactoring, add unions inside other unions,
and a privatemem type which contains just one other field, and even has
its own files? Sorry, but I don't see the point in doing all of that.
(Maybe it would make sense in an OOP language, but I don't think it does
in C.)
[1]
https://lists.freedesktop.org/archives/pulseaudio-discuss/2015-September/024482.html
>
> Signed-off-by: Ahmed S. Darwish <darwish.07 at gmail.com>
> ---
> src/Makefile.am | 1 +
> src/pulsecore/mem.h | 9 +++
> src/pulsecore/memblock.c | 68 ++++++++++++---------
> src/pulsecore/privatemem.c | 78 ++++++++++++++++++++++++
> src/pulsecore/privatemem.h | 35 +++++++++++
> src/pulsecore/shm.c | 148 ++++++++++++++++-----------------------------
> src/pulsecore/shm.h | 11 ++--
> 7 files changed, 220 insertions(+), 130 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 6cb5bd7..f3a62fb 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -701,6 +701,7 @@ libpulsecommon_ at PA_MAJORMINOR@_la_SOURCES = \
> pulsecore/sample-util.c pulsecore/sample-util.h \
> pulsecore/mem.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/mem.h b/src/pulsecore/mem.h
> index b6fdba6..d0a063c 100644
> --- a/src/pulsecore/mem.h
> +++ b/src/pulsecore/mem.h
> @@ -24,6 +24,15 @@
>
> #include <pulsecore/macro.h>
>
> +/* Generic interface representing a plain memory area and its size.
> + *
> + * Different memory backends (posix SHM, private, etc.) embed this
> + * structure as their first element for polymorphic access. */
> +typedef struct pa_mem {
> + void *ptr;
> + size_t size;
> +} pa_mem;
> +
> typedef enum pa_mem_type {
> PA_MEM_TYPE_SHARED_POSIX, /* Data is shared and created using POSIX shm_open() */
> PA_MEM_TYPE_SHARED_MEMFD, /* Data is shared and created using Linux memfd_create() */
> diff --git a/src/pulsecore/memblock.c b/src/pulsecore/memblock.c
> index 3a33a6f..154bd67 100644
> --- a/src/pulsecore/memblock.c
> +++ b/src/pulsecore/memblock.c
> @@ -36,7 +36,9 @@
> #include <pulse/xmalloc.h>
> #include <pulse/def.h>
>
> +#include <pulsecore/mem.h>
> #include <pulsecore/shm.h>
> +#include <pulsecore/privatemem.h>
> #include <pulsecore/log.h>
> #include <pulsecore/hashmap.h>
> #include <pulsecore/semaphore.h>
> @@ -145,8 +147,14 @@ struct pa_mempool {
> pa_semaphore *semaphore;
> pa_mutex *mutex;
>
> - pa_mem_type_t type;
> - pa_shm memory;
> + bool shared;
> + union {
> + pa_mem memory;
> + union {
> + pa_shm shm;
> + pa_privatemem privatemem;
> + } per_type;
> + };
>
> size_t block_size;
> unsigned n_blocks;
> @@ -748,11 +756,9 @@ static void memblock_replace_import(pa_memblock *b) {
>
> pa_mempool *pa_mempool_new(pa_mem_type_t type, size_t size) {
> pa_mempool *p;
> - bool shared;
> char t1[PA_BYTES_SNPRINT_MAX], t2[PA_BYTES_SNPRINT_MAX];
>
> p = pa_xnew0(pa_mempool, 1);
> - p->type = type;
>
> p->block_size = PA_PAGE_ALIGN(PA_MEMPOOL_SLOT_SIZE);
> if (p->block_size < PA_PAGE_SIZE)
> @@ -767,10 +773,13 @@ pa_mempool *pa_mempool_new(pa_mem_type_t type, size_t size) {
> p->n_blocks = 2;
> }
>
> - shared = pa_mem_type_is_shared(type);
> - if (pa_shm_create_rw(&p->memory, p->n_blocks * p->block_size, shared, 0700) < 0) {
> - pa_xfree(p);
> - return NULL;
> + if (pa_mem_type_is_shared(type)) {
> + p->shared = true;
> + if (pa_shm_create(&p->per_type.shm, type, p->n_blocks * p->block_size, 0700) < 0)
> + goto mem_create_fail;
> + } else {
> + if (pa_privatemem_create(&p->per_type.privatemem, p->n_blocks * p->block_size) < 0)
> + goto mem_create_fail;
> }
>
> pa_log_debug("Using %s memory pool with %u slots of size %s each, total size is %s, maximum usable slot size is %lu",
> @@ -791,6 +800,10 @@ pa_mempool *pa_mempool_new(pa_mem_type_t type, size_t size) {
> p->free_slots = pa_flist_new(p->n_blocks);
>
> return p;
> +
> +mem_create_fail:
> + pa_xfree(p);
> + return NULL;
> }
>
> void pa_mempool_free(pa_mempool *p) {
> @@ -852,15 +865,10 @@ void pa_mempool_free(pa_mempool *p) {
> /* PA_DEBUG_TRAP; */
> }
>
> - switch (p->type) {
> - case PA_MEM_TYPE_SHARED_POSIX:
> - case PA_MEM_TYPE_PRIVATE:
> - pa_shm_free(&p->memory);
> - break;
> - case PA_MEM_TYPE_SHARED_MEMFD:
> - pa_assert_not_reached();
> - break;
> - }
> + if (pa_mempool_is_shared(p))
> + pa_shm_free(&p->per_type.shm);
> + else
> + pa_privatemem_free(&p->per_type.privatemem);
>
> pa_mutex_free(p->mutex);
> pa_semaphore_free(p->semaphore);
> @@ -896,7 +904,7 @@ void pa_mempool_vacuum(pa_mempool *p) {
> ;
>
> while ((slot = pa_flist_pop(list))) {
> - pa_shm_punch(&p->memory, (size_t) ((uint8_t*) slot - (uint8_t*) p->memory.ptr), p->block_size);
> + pa_shm_punch(&p->per_type.shm, (size_t) ((uint8_t*) slot - (uint8_t*) p->memory.ptr), p->block_size);
>
> while (pa_flist_push(p->free_slots, slot))
> ;
> @@ -909,10 +917,10 @@ void pa_mempool_vacuum(pa_mempool *p) {
> int pa_mempool_get_shm_id(pa_mempool *p, uint32_t *id) {
> pa_assert(p);
>
> - if (!p->memory.shared)
> + if (!pa_mempool_is_shared(p))
> return -1;
>
> - *id = p->memory.id;
> + *id = p->per_type.shm.id;
>
> return 0;
> }
> @@ -921,7 +929,7 @@ int pa_mempool_get_shm_id(pa_mempool *p, uint32_t *id) {
> bool pa_mempool_is_shared(pa_mempool *p) {
> pa_assert(p);
>
> - return p->memory.shared;
> + return p->shared;
> }
>
> /* For receiving blocks from other nodes */
> @@ -964,7 +972,7 @@ static pa_memimport_segment* segment_attach(pa_memimport *i, uint32_t shm_id, bo
>
> seg->writable = writable;
> seg->import = i;
> - seg->trap = pa_memtrap_add(seg->memory.ptr, seg->memory.size);
> + seg->trap = pa_memtrap_add(seg->memory.mem.ptr, seg->memory.mem.size);
>
> pa_hashmap_put(i->segments, PA_UINT32_TO_PTR(seg->memory.id), seg);
> return seg;
> @@ -1044,7 +1052,7 @@ pa_memblock* pa_memimport_get(pa_memimport *i, uint32_t block_id, uint32_t shm_i
> goto finish;
> }
>
> - if (offset+size > seg->memory.size)
> + if (offset+size > seg->memory.mem.size)
> goto finish;
>
> if (!(b = pa_flist_pop(PA_STATIC_FLIST_GET(unused_memblocks))))
> @@ -1055,7 +1063,7 @@ pa_memblock* pa_memimport_get(pa_memimport *i, uint32_t block_id, uint32_t shm_i
> b->type = PA_MEMBLOCK_IMPORTED;
> b->read_only = !writable;
> b->is_silence = false;
> - pa_atomic_ptr_store(&b->data, (uint8_t*) seg->memory.ptr + offset);
> + pa_atomic_ptr_store(&b->data, (uint8_t*) seg->memory.mem.ptr + offset);
> b->length = size;
> pa_atomic_store(&b->n_acquired, 0);
> pa_atomic_store(&b->please_signal, 0);
> @@ -1103,7 +1111,7 @@ pa_memexport* pa_memexport_new(pa_mempool *p, pa_memexport_revoke_cb_t cb, void
> pa_assert(p);
> pa_assert(cb);
>
> - if (!p->memory.shared)
> + if (!pa_mempool_is_shared(p))
> return NULL;
>
> e = pa_xnew(pa_memexport, 1);
> @@ -1231,7 +1239,7 @@ static pa_memblock *memblock_shared_copy(pa_mempool *p, pa_memblock *b) {
>
> /* Self-locked */
> int pa_memexport_put(pa_memexport *e, pa_memblock *b, uint32_t *block_id, uint32_t *shm_id, size_t *offset, size_t * size) {
> - pa_shm *memory;
> + pa_shm *memory;
> struct memexport_slot *slot;
> void *data;
>
> @@ -1274,14 +1282,14 @@ int pa_memexport_put(pa_memexport *e, pa_memblock *b, uint32_t *block_id, uint32
> } else {
> pa_assert(b->type == PA_MEMBLOCK_POOL || b->type == PA_MEMBLOCK_POOL_EXTERNAL);
> pa_assert(b->pool);
> - memory = &b->pool->memory;
> + memory = &b->pool->per_type.shm;
> }
>
> - pa_assert(data >= memory->ptr);
> - pa_assert((uint8_t*) data + b->length <= (uint8_t*) memory->ptr + memory->size);
> + pa_assert(data >= memory->mem.ptr);
> + pa_assert((uint8_t*) data + b->length <= (uint8_t*) memory->mem.ptr + memory->mem.size);
>
> *shm_id = memory->id;
> - *offset = (size_t) ((uint8_t*) data - (uint8_t*) memory->ptr);
> + *offset = (size_t) ((uint8_t*) data - (uint8_t*) memory->mem.ptr);
> *size = b->length;
>
> pa_memblock_release(b);
> diff --git a/src/pulsecore/privatemem.c b/src/pulsecore/privatemem.c
> new file mode 100644
> index 0000000..99df8c7
> --- /dev/null
> +++ b/src/pulsecore/privatemem.c
> @@ -0,0 +1,78 @@
> +/***
> + 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 PA_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);
> +
> + /* Round up to make it page aligned */
> + size = PA_PAGE_ALIGN(size);
> +
> + m->mem.size = size;
> +
> +#ifdef MAP_ANONYMOUS
> + if ((m->mem.ptr = mmap(NULL, m->mem.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->mem.ptr, PA_PAGE_SIZE, size)) < 0) {
> + pa_log("posix_memalign() failed: %s", pa_cstrerror(r));
> + goto fail;
> + }
> + }
> +#else
> + m->mem.ptr = pa_xmalloc(m->mem.size);
> +#endif
> +
> + return 0;
> +fail:
> + return -1;
> +}
> +
> +void pa_privatemem_free(pa_privatemem *m) {
> + pa_assert(m);
> + pa_assert(m->mem.ptr);
> + pa_assert(m->mem.size > 0);
> +
> +#ifdef MAP_ANONYMOUS
> + if (munmap(m->mem.ptr, m->mem.size) < 0)
> + pa_log("munmap() failed: %s", pa_cstrerror(errno));
> +#elif defined(HAVE_POSIX_MEMALIGN)
> + free(m->mem.ptr);
> +#else
> + pa_xfree(m->mem.ptr);
> +#endif
> +}
> diff --git a/src/pulsecore/privatemem.h b/src/pulsecore/privatemem.h
> new file mode 100644
> index 0000000..9240ce0
> --- /dev/null
> +++ b/src/pulsecore/privatemem.h
> @@ -0,0 +1,35 @@
> +#ifndef foopulseprivatememhfoo
> +#define foopulseprivatememhfoo
> +
> +/***
> + 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/>.
> +***/
> +
> +#include <pulsecore/mem.h>
> +
> +/* 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 {
> + pa_mem mem; /* Parent; must be first */
> +} pa_privatemem;
> +
> +int pa_privatemem_create(pa_privatemem *m, size_t size);
> +void pa_privatemem_free(pa_privatemem *m);
> +
> +#endif
> diff --git a/src/pulsecore/shm.c b/src/pulsecore/shm.c
> index d613168..4c0ecf1 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,15 +97,17 @@ 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, pa_mem_type_t type, 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(pa_mem_type_is_shared(type));
> pa_assert(size > 0);
> - pa_assert(size <= MAX_SHM_SIZE);
> + pa_assert(size <= PA_MAX_SHM_SIZE);
> pa_assert(!(mode & ~0777));
> pa_assert(mode >= 0600);
>
> @@ -119,72 +118,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->mem.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->mem.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;
> - }
> -
> - /* 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;
> -#else
> + if ((m->mem.ptr = mmap(NULL, PA_PAGE_ALIGN(m->mem.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;
> -#endif
> }
>
> - m->shared = shared;
> + /* 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->mem.ptr + m->mem.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;
> +#else
> + goto fail;
> +#endif
>
> return 0;
>
> @@ -202,40 +171,28 @@ fail:
>
> void pa_shm_free(pa_shm *m) {
> pa_assert(m);
> - pa_assert(m->ptr);
> - pa_assert(m->size > 0);
> + pa_assert(m->mem.ptr);
> + pa_assert(m->mem.size > 0);
>
> #ifdef MAP_FAILED
> - pa_assert(m->ptr != MAP_FAILED);
> + pa_assert(m->mem.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 (munmap(m->mem.ptr, PA_PAGE_ALIGN(m->mem.size)) < 0)
> + pa_log("munmap() failed: %s", pa_cstrerror(errno));
>
> - if (m->do_unlink) {
> - char fn[32];
> + if (m->do_unlink) {
> + char fn[32];
>
> - 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();
> -#endif
> + 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();
> +#endif
>
> pa_zero(*m);
> }
> @@ -245,19 +202,19 @@ void pa_shm_punch(pa_shm *m, size_t offset, size_t size) {
> size_t o;
>
> pa_assert(m);
> - pa_assert(m->ptr);
> - pa_assert(m->size > 0);
> - pa_assert(offset+size <= m->size);
> + pa_assert(m->mem.ptr);
> + pa_assert(m->mem.size > 0);
> + pa_assert(offset+size <= m->mem.size);
>
> #ifdef MAP_FAILED
> - pa_assert(m->ptr != MAP_FAILED);
> + pa_assert(m->mem.ptr != MAP_FAILED);
> #endif
>
> /* You're welcome to implement this as NOOP on systems that don't
> * support it */
>
> /* Align the pointer up to multiples of the page size */
> - ptr = (uint8_t*) m->ptr + offset;
> + ptr = (uint8_t*) m->mem.ptr + offset;
> o = (size_t) ((uint8_t*) ptr - (uint8_t*) PA_PAGE_ALIGN_PTR(ptr));
>
> if (o > 0) {
> @@ -310,22 +267,21 @@ static int shm_attach(pa_shm *m, unsigned id, bool writable, bool for_cleanup) {
> }
>
> if (st.st_size <= 0 ||
> - st.st_size > (off_t) (MAX_SHM_SIZE+SHM_MARKER_SIZE) ||
> + st.st_size > (off_t) (PA_MAX_SHM_SIZE+SHM_MARKER_SIZE) ||
> PA_ALIGN((size_t) st.st_size) != (size_t) st.st_size) {
> pa_log("Invalid shared memory segment size");
> goto fail;
> }
>
> - m->size = (size_t) st.st_size;
> + m->mem.size = (size_t) st.st_size;
>
> prot = writable ? PROT_READ | PROT_WRITE : PROT_READ;
> - if ((m->ptr = mmap(NULL, PA_PAGE_ALIGN(m->size), prot, MAP_SHARED, fd, (off_t) 0)) == MAP_FAILED) {
> + if ((m->mem.ptr = mmap(NULL, PA_PAGE_ALIGN(m->mem.size), prot, MAP_SHARED, fd, (off_t) 0)) == MAP_FAILED) {
> pa_log("mmap() failed: %s", pa_cstrerror(errno));
> goto fail;
> }
>
> m->do_unlink = false;
> - m->shared = true;
>
> pa_assert_se(pa_close(fd) == 0);
>
> @@ -382,12 +338,12 @@ int pa_shm_cleanup(void) {
> if (shm_attach(&seg, id, false, true) < 0)
> continue;
>
> - if (seg.size < SHM_MARKER_SIZE) {
> + if (seg.mem.size < SHM_MARKER_SIZE) {
> pa_shm_free(&seg);
> continue;
> }
>
> - m = (struct shm_marker*) ((uint8_t*) seg.ptr + seg.size - SHM_MARKER_SIZE);
> + m = (struct shm_marker*) ((uint8_t*) seg.mem.ptr + seg.mem.size - SHM_MARKER_SIZE);
>
> if (pa_atomic_load(&m->marker) != SHM_MARKER) {
> pa_shm_free(&seg);
> diff --git a/src/pulsecore/shm.h b/src/pulsecore/shm.h
> index d438961..887f516 100644
> --- a/src/pulsecore/shm.h
> +++ b/src/pulsecore/shm.h
> @@ -23,16 +23,19 @@
> #include <sys/types.h>
>
> #include <pulsecore/macro.h>
> +#include <pulsecore/mem.h>
> +
> +/* 1 GiB at max */
> +#define PA_MAX_SHM_SIZE (PA_ALIGN(1024*1024*1024))
>
> typedef struct pa_shm {
> + pa_mem mem; /* Parent; must be first */
> + pa_mem_type_t type;
> unsigned id;
> - 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);
> +int pa_shm_create(pa_shm *m, pa_mem_type_t type, size_t size, mode_t mode);
> int pa_shm_attach(pa_shm *m, unsigned id, bool writable);
>
> void pa_shm_punch(pa_shm *m, size_t offset, size_t size);
>
> Regards,
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
