Soon we're going to have three types of memory pools: POSIX shm_open() pools, memfd memfd_create() pools, and privately malloc()-ed pools. Due to such diversity of mempool types, transform pa_mempool_new() into a factory method that returns the required type of memory pool according to given parameters. Signed-off-by: Ahmed S. Darwish <darwish.07 at gmail.com> --- src/pulse/context.c | 10 ++-- src/pulsecore/core.c | 4 +- src/pulsecore/memblock.c | 102 +++++++++++++++++++++++++++++++--------- src/pulsecore/memblock.h | 8 +++- src/pulsecore/protocol-native.c | 2 +- src/pulsecore/shm.c | 25 +++++----- src/pulsecore/shm.h | 2 +- src/tests/cpu-mix-test.c | 2 +- src/tests/lfe-filter-test.c | 2 +- src/tests/mcalign-test.c | 2 +- src/tests/memblock-test.c | 6 +-- src/tests/memblockq-test.c | 2 +- src/tests/mix-test.c | 2 +- src/tests/remix-test.c | 2 +- src/tests/resampler-test.c | 2 +- src/tests/srbchannel-test.c | 2 +- 16 files changed, 120 insertions(+), 55 deletions(-) diff --git a/src/pulse/context.c b/src/pulse/context.c index 738ea84..d3ea1d4 100644 --- a/src/pulse/context.c +++ b/src/pulse/context.c @@ -125,6 +125,7 @@ static void reset_callbacks(pa_context *c) { pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char *name, pa_proplist *p) { pa_context *c; + pa_mempool_type_t type; pa_assert(mainloop); @@ -170,10 +171,13 @@ pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char * c->srb_template.readfd = -1; c->srb_template.writefd = -1; - if (!(c->mempool = pa_mempool_new(!c->conf->disable_shm, c->conf->shm_size))) { + type = !c->conf->disable_shm ? PA_MEMPOOL_SHARED_POSIX : PA_MEMPOOL_PRIVATE; + if (!(c->mempool = pa_mempool_new(type, c->conf->shm_size))) { - if (!c->conf->disable_shm) - c->mempool = pa_mempool_new(false, c->conf->shm_size); + if (!c->conf->disable_shm) { + pa_log_warn("Failed to allocate shared memory pool. Falling back to a normal private one."); + c->mempool = pa_mempool_new(PA_MEMPOOL_PRIVATE, c->conf->shm_size); + } if (!c->mempool) { context_free(c); diff --git a/src/pulsecore/core.c b/src/pulsecore/core.c index 546df44..bf2448e 100644 --- a/src/pulsecore/core.c +++ b/src/pulsecore/core.c @@ -69,14 +69,14 @@ pa_core* pa_core_new(pa_mainloop_api *m, bool shared, size_t shm_size) { pa_assert(m); if (shared) { - if (!(pool = pa_mempool_new(shared, shm_size))) { + if (!(pool = pa_mempool_new(PA_MEMPOOL_SHARED_POSIX, shm_size))) { pa_log_warn("Failed to allocate shared memory pool. Falling back to a normal memory pool."); shared = false; } } if (!shared) { - if (!(pool = pa_mempool_new(shared, shm_size))) { + if (!(pool = pa_mempool_new(PA_MEMPOOL_PRIVATE, shm_size))) { pa_log("pa_mempool_new() failed."); return NULL; } diff --git a/src/pulsecore/memblock.c b/src/pulsecore/memblock.c index 9b6810d..aa64d01 100644 --- a/src/pulsecore/memblock.c +++ b/src/pulsecore/memblock.c @@ -145,7 +145,14 @@ struct pa_mempool { pa_semaphore *semaphore; pa_mutex *mutex; - pa_shm memory; + pa_mempool_type_t type; + union { + pa_shm shm; + } per_type; + + void *ptr; + size_t size; + size_t block_size; unsigned n_blocks; bool is_remote_writable; @@ -261,7 +268,7 @@ static struct mempool_slot* mempool_allocate_slot(pa_mempool *p) { if ((unsigned) (idx = pa_atomic_inc(&p->n_init)) >= p->n_blocks) pa_atomic_dec(&p->n_init); else - slot = (struct mempool_slot*) ((uint8_t*) p->memory.ptr + (p->block_size * (size_t) idx)); + slot = (struct mempool_slot*) ((uint8_t*) p->ptr + (p->block_size * (size_t) idx)); if (!slot) { if (pa_log_ratelimit(PA_LOG_DEBUG)) @@ -289,10 +296,10 @@ static inline void* mempool_slot_data(struct mempool_slot *slot) { static unsigned mempool_slot_idx(pa_mempool *p, void *ptr) { pa_assert(p); - pa_assert((uint8_t*) ptr >= (uint8_t*) p->memory.ptr); - pa_assert((uint8_t*) ptr < (uint8_t*) p->memory.ptr + p->memory.size); + pa_assert((uint8_t*) ptr >= (uint8_t*) p->ptr); + pa_assert((uint8_t*) ptr < (uint8_t*) p->ptr + p->size); - return (unsigned) ((size_t) ((uint8_t*) ptr - (uint8_t*) p->memory.ptr) / p->block_size); + return (unsigned) ((size_t) ((uint8_t*) ptr - (uint8_t*) p->ptr) / p->block_size); } /* No lock necessary */ @@ -302,7 +309,7 @@ static struct mempool_slot* mempool_slot_by_ptr(pa_mempool *p, void *ptr) { if ((idx = mempool_slot_idx(p, ptr)) == (unsigned) -1) return NULL; - return (struct mempool_slot*) ((uint8_t*) p->memory.ptr + (idx * p->block_size)); + return (struct mempool_slot*) ((uint8_t*) p->ptr + (idx * p->block_size)); } /* No lock necessary */ @@ -744,12 +751,28 @@ static void memblock_replace_import(pa_memblock *b) { pa_mutex_unlock(import->mutex); } -pa_mempool* pa_mempool_new(bool shared, size_t size) { +static const char *pa_mempool_type_tostr(pa_mempool_type_t type) { + switch (type) { + case PA_MEMPOOL_SHARED_POSIX: + return "shared posix-shm"; + case PA_MEMPOOL_SHARED_MEMFD: + return "shared memfd"; + case PA_MEMPOOL_PRIVATE: + return "private"; + default: + pa_assert_not_reached(); + } +} + +pa_mempool *pa_mempool_new(pa_mempool_type_t type, size_t size) { pa_mempool *p; + size_t pa_mem_size; 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) p->block_size = PA_PAGE_SIZE; @@ -762,17 +785,36 @@ pa_mempool* pa_mempool_new(bool shared, size_t size) { if (p->n_blocks < 2) p->n_blocks = 2; } - - if (pa_shm_create_rw(&p->memory, p->n_blocks * p->block_size, shared, 0700) < 0) { - pa_xfree(p); - return NULL; + pa_mem_size = p->n_blocks * p->block_size; + + switch (type) { + case PA_MEMPOOL_SHARED_POSIX: + if (pa_shm_create_rw(&p->per_type.shm, pa_mem_size, true, 0700) < 0) + goto fail; + + p->ptr = p->per_type.shm.ptr; + p->size = p->per_type.shm.size; + break; + case PA_MEMPOOL_SHARED_MEMFD: + pa_assert_not_reached(); + break; + case PA_MEMPOOL_PRIVATE: + if (pa_shm_create_rw(&p->per_type.shm, pa_mem_size, false, 0700) < 0) + goto fail; + + p->ptr = p->per_type.shm.ptr; + p->size = p->per_type.shm.size; + break; + default: + pa_assert_not_reached(); + break; } pa_log_debug("Using %s memory pool with %u slots of size %s each, total size is %s, maximum usable slot size is %lu", - p->memory.shared ? "shared" : "private", + pa_mempool_type_tostr(type), p->n_blocks, pa_bytes_snprint(t1, sizeof(t1), (unsigned) p->block_size), - pa_bytes_snprint(t2, sizeof(t2), (unsigned) (p->n_blocks * p->block_size)), + pa_bytes_snprint(t2, sizeof(t2), (unsigned) (pa_mem_size)), (unsigned long) pa_mempool_block_size_max(p)); pa_atomic_store(&p->n_init, 0); @@ -786,6 +828,10 @@ pa_mempool* pa_mempool_new(bool shared, size_t size) { p->free_slots = pa_flist_new(p->n_blocks); return p; + +fail: + pa_xfree(p); + return NULL; } void pa_mempool_free(pa_mempool *p) { @@ -819,7 +865,7 @@ void pa_mempool_free(pa_mempool *p) { struct mempool_slot *slot; pa_memblock *b, *k; - slot = (struct mempool_slot*) ((uint8_t*) p->memory.ptr + (p->block_size * (size_t) i)); + slot = (struct mempool_slot*) ((uint8_t*) p->ptr + (p->block_size * (size_t) i)); b = mempool_slot_data(slot); while ((k = pa_flist_pop(p->free_slots))) { @@ -847,7 +893,19 @@ void pa_mempool_free(pa_mempool *p) { /* PA_DEBUG_TRAP; */ } - pa_shm_free(&p->memory); + switch (p->type) { + case PA_MEMPOOL_SHARED_POSIX: + pa_shm_free(&p->per_type.shm); + break; + case PA_MEMPOOL_SHARED_MEMFD: + pa_assert_not_reached(); + break; + case PA_MEMPOOL_PRIVATE: + pa_shm_free(&p->per_type.shm); + break; + default: + pa_assert_not_reached(); + } pa_mutex_free(p->mutex); pa_semaphore_free(p->semaphore); @@ -883,12 +941,11 @@ 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->ptr, p->size, (size_t) ((uint8_t*) slot - (uint8_t*) p->ptr), p->block_size); while (pa_flist_push(p->free_slots, slot)) ; } - pa_flist_free(list, NULL); } @@ -896,10 +953,9 @@ 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) - return -1; + pa_assert(p->type == PA_MEMPOOL_SHARED_POSIX); - *id = p->memory.id; + *id = p->per_type.shm.id; return 0; } @@ -908,7 +964,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->type == PA_MEMPOOL_SHARED_POSIX || p->type == PA_MEMPOOL_SHARED_MEMFD; } /* For receiving blocks from other nodes */ @@ -1090,7 +1146,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); @@ -1261,7 +1317,7 @@ 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); diff --git a/src/pulsecore/memblock.h b/src/pulsecore/memblock.h index 4faef75..a3b8a07 100644 --- a/src/pulsecore/memblock.h +++ b/src/pulsecore/memblock.h @@ -47,6 +47,12 @@ typedef enum pa_memblock_type { PA_MEMBLOCK_TYPE_MAX } pa_memblock_type_t; +typedef enum pa_mempool_type { + PA_MEMPOOL_SHARED_POSIX, /* Mempool data is shared and created using POSIX shm_open() */ + PA_MEMPOOL_SHARED_MEMFD, /* Mempool data is shared and created using Linux kernel memfd_create() */ + PA_MEMPOOL_PRIVATE, /* Mempool data is private and created using classic memory allocation (malloc, etc.) */ +} pa_mempool_type_t; + typedef struct pa_mempool pa_mempool; typedef struct pa_mempool_stat pa_mempool_stat; typedef struct pa_memimport_segment pa_memimport_segment; @@ -121,7 +127,7 @@ pa_mempool * pa_memblock_get_pool(pa_memblock *b); pa_memblock *pa_memblock_will_need(pa_memblock *b); /* The memory block manager */ -pa_mempool* pa_mempool_new(bool shared, size_t size); +pa_mempool *pa_mempool_new(pa_mempool_type_t type, size_t size); void pa_mempool_free(pa_mempool *p); const pa_mempool_stat* pa_mempool_get_stat(pa_mempool *p); void pa_mempool_vacuum(pa_mempool *p); diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c index 5a062a6..ae0c974 100644 --- a/src/pulsecore/protocol-native.c +++ b/src/pulsecore/protocol-native.c @@ -2607,7 +2607,7 @@ static void setup_srbchannel(pa_native_connection *c) { return; } - if (!(rw_mempool = pa_mempool_new(true, c->protocol->core->shm_size))) { + if (!(rw_mempool = pa_mempool_new(PA_MEMPOOL_SHARED_POSIX, c->protocol->core->shm_size))) { pa_log_warn("Disabling srbchannel, reason: Failed to allocate shared " "writable memory pool."); return; diff --git a/src/pulsecore/shm.c b/src/pulsecore/shm.c index eefd7ba..0c14e70 100644 --- a/src/pulsecore/shm.c +++ b/src/pulsecore/shm.c @@ -240,49 +240,48 @@ void pa_shm_free(pa_shm *m) { pa_zero(*m); } -void pa_shm_punch(pa_shm *m, size_t offset, size_t size) { +void pa_shm_punch(void *shm_start, size_t total_shm_size, size_t punch_offset, size_t punch_size) { void *ptr; size_t o; - pa_assert(m); - pa_assert(m->ptr); - pa_assert(m->size > 0); - pa_assert(offset+size <= m->size); + pa_assert(shm_start); + pa_assert(total_shm_size > 0); + pa_assert(punch_offset+punch_size <= total_shm_size); #ifdef MAP_FAILED - pa_assert(m->ptr != MAP_FAILED); + pa_assert(shm_start != 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*) shm_start + punch_offset; o = (size_t) ((uint8_t*) ptr - (uint8_t*) PA_PAGE_ALIGN_PTR(ptr)); if (o > 0) { size_t delta = PA_PAGE_SIZE - o; ptr = (uint8_t*) ptr + delta; - size -= delta; + punch_size -= delta; } /* Align the size down to multiples of page size */ - size = (size / PA_PAGE_SIZE) * PA_PAGE_SIZE; + punch_size = (punch_size / PA_PAGE_SIZE) * PA_PAGE_SIZE; #ifdef MADV_REMOVE - if (madvise(ptr, size, MADV_REMOVE) >= 0) + if (madvise(ptr, punch_size, MADV_REMOVE) >= 0) return; #endif #ifdef MADV_FREE - if (madvise(ptr, size, MADV_FREE) >= 0) + if (madvise(ptr, punch_size, MADV_FREE) >= 0) return; #endif #ifdef MADV_DONTNEED - madvise(ptr, size, MADV_DONTNEED); + madvise(ptr, punch_size, MADV_DONTNEED); #elif defined(POSIX_MADV_DONTNEED) - posix_madvise(ptr, size, POSIX_MADV_DONTNEED); + posix_madvise(ptr, punch_size, POSIX_MADV_DONTNEED); #endif } diff --git a/src/pulsecore/shm.h b/src/pulsecore/shm.h index d438961..9fe8e2f 100644 --- a/src/pulsecore/shm.h +++ b/src/pulsecore/shm.h @@ -35,7 +35,7 @@ typedef struct pa_shm { int pa_shm_create_rw(pa_shm *m, size_t size, bool shared, 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); +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); diff --git a/src/tests/cpu-mix-test.c b/src/tests/cpu-mix-test.c index f3bc0cc..ff5dda8 100644 --- a/src/tests/cpu-mix-test.c +++ b/src/tests/cpu-mix-test.c @@ -76,7 +76,7 @@ static void run_mix_test( samples_ref = out_ref + (8 - align); nsamples = channels * (SAMPLES - (8 - align)); - fail_unless((pool = pa_mempool_new(false, 0)) != NULL, NULL); + fail_unless((pool = pa_mempool_new(PA_MEMPOOL_PRIVATE, 0)) != NULL, NULL); pa_random(samples0, nsamples * sizeof(int16_t)); c0.memblock = pa_memblock_new_fixed(pool, samples0, nsamples * sizeof(int16_t), false); diff --git a/src/tests/lfe-filter-test.c b/src/tests/lfe-filter-test.c index 389a2b9..7e68243 100644 --- a/src/tests/lfe-filter-test.c +++ b/src/tests/lfe-filter-test.c @@ -136,7 +136,7 @@ START_TEST (lfe_filter_test) { a.format = PA_SAMPLE_S16NE; lft.ss = &a; - pa_assert_se(lft.pool = pa_mempool_new(false, 0)); + pa_assert_se(lft.pool = pa_mempool_new(PA_MEMPOOL_PRIVATE, 0)); /* We prepare pseudo-random input audio samples for lfe-filter rewind testing*/ ori_sample_ptr = pa_xmalloc(pa_frame_size(lft.ss) * TOTAL_SAMPLES); diff --git a/src/tests/mcalign-test.c b/src/tests/mcalign-test.c index 0d27dfd..6a371f5 100644 --- a/src/tests/mcalign-test.c +++ b/src/tests/mcalign-test.c @@ -36,7 +36,7 @@ int main(int argc, char *argv[]) { pa_mcalign *a; pa_memchunk c; - p = pa_mempool_new(false, 0); + p = pa_mempool_new(PA_MEMPOOL_PRIVATE, 0); a = pa_mcalign_new(11); diff --git a/src/tests/memblock-test.c b/src/tests/memblock-test.c index 2b51108..fb52705 100644 --- a/src/tests/memblock-test.c +++ b/src/tests/memblock-test.c @@ -80,11 +80,11 @@ START_TEST (memblock_test) { const char txt[] = "This is a test!"; - pool_a = pa_mempool_new(true, 0); + pool_a = pa_mempool_new(PA_MEMPOOL_SHARED_POSIX, 0); fail_unless(pool_a != NULL); - pool_b = pa_mempool_new(true, 0); + pool_b = pa_mempool_new(PA_MEMPOOL_SHARED_POSIX, 0); fail_unless(pool_b != NULL); - pool_c = pa_mempool_new(true, 0); + pool_c = pa_mempool_new(PA_MEMPOOL_SHARED_POSIX, 0); fail_unless(pool_c != NULL); pa_mempool_get_shm_id(pool_a, &id_a); diff --git a/src/tests/memblockq-test.c b/src/tests/memblockq-test.c index eea6cfa..96fac9f 100644 --- a/src/tests/memblockq-test.c +++ b/src/tests/memblockq-test.c @@ -108,7 +108,7 @@ START_TEST (memblockq_test) { pa_log_set_level(PA_LOG_DEBUG); - p = pa_mempool_new(false, 0); + p = pa_mempool_new(PA_MEMPOOL_PRIVATE, 0); silence.memblock = pa_memblock_new_fixed(p, (char*) "__", 2, 1); fail_unless(silence.memblock != NULL); diff --git a/src/tests/mix-test.c b/src/tests/mix-test.c index c8af600..2074695 100644 --- a/src/tests/mix-test.c +++ b/src/tests/mix-test.c @@ -286,7 +286,7 @@ START_TEST (mix_test) { if (!getenv("MAKE_CHECK")) pa_log_set_level(PA_LOG_DEBUG); - fail_unless((pool = pa_mempool_new(false, 0)) != NULL, NULL); + fail_unless((pool = pa_mempool_new(PA_MEMPOOL_PRIVATE, 0)) != NULL, NULL); a.channels = 1; a.rate = 44100; diff --git a/src/tests/remix-test.c b/src/tests/remix-test.c index 6feb8dc..70fcd18 100644 --- a/src/tests/remix-test.c +++ b/src/tests/remix-test.c @@ -51,7 +51,7 @@ int main(int argc, char *argv[]) { pa_log_set_level(PA_LOG_DEBUG); - pa_assert_se(pool = pa_mempool_new(false, 0)); + pa_assert_se(pool = pa_mempool_new(PA_MEMPOOL_PRIVATE, 0)); for (i = 0; maps[i].channels > 0; i++) for (j = 0; maps[j].channels > 0; j++) { diff --git a/src/tests/resampler-test.c b/src/tests/resampler-test.c index 9832a31..b7d7729 100644 --- a/src/tests/resampler-test.c +++ b/src/tests/resampler-test.c @@ -404,7 +404,7 @@ int main(int argc, char *argv[]) { } ret = 0; - pa_assert_se(pool = pa_mempool_new(false, 0)); + pa_assert_se(pool = pa_mempool_new(PA_MEMPOOL_PRIVATE, 0)); if (!all_formats) { diff --git a/src/tests/srbchannel-test.c b/src/tests/srbchannel-test.c index cd4d397..cf8f281 100644 --- a/src/tests/srbchannel-test.c +++ b/src/tests/srbchannel-test.c @@ -85,7 +85,7 @@ START_TEST (srbchannel_test) { int pipefd[4]; pa_mainloop *ml = pa_mainloop_new(); - pa_mempool *mp = pa_mempool_new(true, 0); + pa_mempool *mp = pa_mempool_new(PA_MEMPOOL_SHARED_POSIX, 0); pa_iochannel *io1, *io2; pa_pstream *p1, *p2; pa_srbchannel *sr1, *sr2; -- Darwish http://darwish.chasingpointers.com