From e8faf2f2adeb9066de3c968a57803bb262f61ee1 Mon Sep 17 00:00:00 2001 From: Aidan MacDonald Date: Tue, 18 Jan 2022 18:57:06 +0000 Subject: [PATCH] buflib: add a common dummy callbacks struct & use it There are various allocations that can't be moved or shrunk. Provide a global callback struct for this use case instead of making each caller declare its own dummy struct. Also fixed ROLO and x1000 installer code which incorrectly used movable allocations. Change-Id: I00088396b9826e02e69a4a33477fe1a7816374f1 --- apps/playlist.c | 10 ++-------- apps/plugin.c | 5 +---- apps/recorder/pcm_record.c | 7 ++----- apps/tagcache.c | 3 +-- firmware/buflib.c | 6 ++++++ firmware/common/zip.c | 4 +--- firmware/include/buflib.h | 6 ++++++ firmware/rolo.c | 2 +- firmware/target/mips/ingenic_x1000/installer-x1000.c | 2 +- firmware/usbstack/usb_storage.c | 7 +++---- lib/x1000-installer/src/xf_nandio.c | 2 +- lib/x1000-installer/src/xf_package.c | 2 +- lib/x1000-installer/test_lib/core_alloc.c | 7 +++++++ lib/x1000-installer/test_lib/core_alloc.h | 10 ++++++++++ 14 files changed, 43 insertions(+), 30 deletions(-) diff --git a/apps/playlist.c b/apps/playlist.c index e93feb6abb..a7b16d8b1b 100644 --- a/apps/playlist.c +++ b/apps/playlist.c @@ -2076,13 +2076,10 @@ int playlist_create(const char *dir, const char *file) if (file) { - /* dummy ops with no callbacks, needed because by - * default buflib buffers can be moved around which must be avoided */ - static struct buflib_callbacks dummy_ops; int handle; size_t buflen; /* use mp3 buffer for maximum load speed */ - handle = core_alloc_maximum("temp", &buflen, &dummy_ops); + handle = core_alloc_maximum("temp", &buflen, &buflib_ops_locked); if (handle > 0) { /* load the playlist file */ @@ -2119,13 +2116,10 @@ int playlist_resume(void) bool sorted = true; int result = -1; - /* dummy ops with no callbacks, needed because by - * default buflib buffers can be moved around which must be avoided */ - static struct buflib_callbacks dummy_ops; /* use mp3 buffer for maximum load speed */ if (core_allocatable() < (1 << 10)) talk_buffer_set_policy(TALK_BUFFER_LOOSE); /* back off voice buffer */ - handle = core_alloc_maximum("temp", &buflen, &dummy_ops); + handle = core_alloc_maximum("temp", &buflen, &buflib_ops_locked); if (handle < 0) { splashf(HZ * 2, "%s(): OOM", __func__); diff --git a/apps/plugin.c b/apps/plugin.c index aa69b9ca03..8434fea07d 100644 --- a/apps/plugin.c +++ b/apps/plugin.c @@ -990,14 +990,11 @@ void* plugin_get_buffer(size_t *buffer_size) */ static void* plugin_get_audio_buffer(size_t *buffer_size) { - /* dummy ops with no callbacks, needed because by - * default buflib buffers can be moved around which must be avoided */ - static struct buflib_callbacks dummy_ops; if (plugin_buffer_handle <= 0) { plugin_buffer_handle = core_alloc_maximum("plugin audio buf", &plugin_buffer_size, - &dummy_ops); + &buflib_ops_locked); } if (buffer_size) diff --git a/apps/recorder/pcm_record.c b/apps/recorder/pcm_record.c index ef10f8a433..0f6e8469ec 100644 --- a/apps/recorder/pcm_record.c +++ b/apps/recorder/pcm_record.c @@ -1406,12 +1406,9 @@ static int pcmrec_handle; static void on_init_recording(void) { send_event(RECORDING_EVENT_START, NULL); - /* dummy ops with no callbacks, needed because by - * default buflib buffers can be moved around which must be avoided - * FIXME: This buffer should play nicer and be shrinkable/movable */ - static struct buflib_callbacks dummy_ops; + /* FIXME: This buffer should play nicer and be shrinkable/movable */ talk_buffer_set_policy(TALK_BUFFER_LOOSE); - pcmrec_handle = core_alloc_maximum("pcmrec", &rec_buffer_size, &dummy_ops); + pcmrec_handle = core_alloc_maximum("pcmrec", &rec_buffer_size, &buflib_ops_locked); if (pcmrec_handle <= 0) /* someone is abusing core_alloc_maximum(). Fix this evil guy instead of * trying to handle OOM without hope */ diff --git a/apps/tagcache.c b/apps/tagcache.c index 37f443e036..fc06005c1d 100644 --- a/apps/tagcache.c +++ b/apps/tagcache.c @@ -328,8 +328,7 @@ static void allocate_tempbuf(void) #else /* !__PCTOOL__ */ /* Need to pass dummy ops to prevent the buffer being moved * out from under us, since we yield during the tagcache commit. */ - static struct buflib_callbacks dummy_ops; - tempbuf_handle = core_alloc_maximum("tc tempbuf", &size, &dummy_ops); + tempbuf_handle = core_alloc_maximum("tc tempbuf", &size, &buflib_ops_locked); if (tempbuf_handle > 0) { tempbuf = core_get_data(tempbuf_handle); diff --git a/firmware/buflib.c b/firmware/buflib.c index 0e90e7fe72..c6ec011653 100644 --- a/firmware/buflib.c +++ b/firmware/buflib.c @@ -97,6 +97,12 @@ #define BPANICF panicf +struct buflib_callbacks buflib_ops_locked = { + .move_callback = NULL, + .shrink_callback = NULL, + .sync_callback = NULL, +}; + #define IS_MOVABLE(a) (!a[2].ops || a[2].ops->move_callback) static union buflib_data* find_first_free(struct buflib_context *ctx); static union buflib_data* find_block_before(struct buflib_context *ctx, diff --git a/firmware/common/zip.c b/firmware/common/zip.c index 9512d6c239..36b90a9223 100644 --- a/firmware/common/zip.c +++ b/firmware/common/zip.c @@ -32,9 +32,7 @@ #include "crc32.h" #include "rbendian.h" -#define zip_core_alloc(N) core_alloc_ex("zip",(N),&dummy_ops) - -static struct buflib_callbacks dummy_ops; +#define zip_core_alloc(N) core_alloc_ex("zip",(N),&buflib_ops_locked) enum { ZIP_SIG_ED = 0x06054b50, diff --git a/firmware/include/buflib.h b/firmware/include/buflib.h index 7f534c6ce0..e805ebbf1b 100644 --- a/firmware/include/buflib.h +++ b/firmware/include/buflib.h @@ -129,6 +129,12 @@ struct buflib_callbacks { void (*sync_callback)(int handle, bool sync_on); }; +/** A set of all NULL callbacks for use with allocations that need to stay + * locked in RAM and not moved or shrunk. These type of allocations should + * be avoided as much as possible to avoid memory fragmentation but it can + * suitable for short-lived allocations. */ +extern struct buflib_callbacks buflib_ops_locked; + #define BUFLIB_SHRINK_SIZE_MASK (~BUFLIB_SHRINK_POS_MASK) #define BUFLIB_SHRINK_POS_FRONT (1u<<31) #define BUFLIB_SHRINK_POS_BACK (1u<<30) diff --git a/firmware/rolo.c b/firmware/rolo.c index 5f936c95f4..f95fc4bd4d 100644 --- a/firmware/rolo.c +++ b/firmware/rolo.c @@ -242,7 +242,7 @@ int rolo_load(const char* filename) /* get the system buffer. release only in case of error, otherwise * we don't return anyway */ - rolo_handle = core_alloc_maximum("rolo", &filebuf_size, NULL); + rolo_handle = core_alloc_maximum("rolo", &filebuf_size, &buflib_ops_locked); if (rolo_handle < 0) { rolo_error("OOM"); diff --git a/firmware/target/mips/ingenic_x1000/installer-x1000.c b/firmware/target/mips/ingenic_x1000/installer-x1000.c index 0a09ad0e91..66aa42d4a1 100644 --- a/firmware/target/mips/ingenic_x1000/installer-x1000.c +++ b/firmware/target/mips/ingenic_x1000/installer-x1000.c @@ -148,7 +148,7 @@ static int updater_init(struct updater* u) /* buf_len is a bit oversized here, but it's not really important */ u->buf_len = u->img_len + sizeof(mtar_t) + 2*CACHEALIGN_SIZE; - u->buf_hnd = core_alloc("boot_image", u->buf_len); + u->buf_hnd = core_alloc_ex("boot_image", u->buf_len, &buflib_ops_locked); if(u->buf_hnd < 0) { rc = IERR_OUT_OF_MEMORY; goto error; diff --git a/firmware/usbstack/usb_storage.c b/firmware/usbstack/usb_storage.c index a32cf185e7..eb82f72eae 100644 --- a/firmware/usbstack/usb_storage.c +++ b/firmware/usbstack/usb_storage.c @@ -449,12 +449,11 @@ void usb_storage_init_connection(void) #endif #else unsigned char * buffer; - /* dummy ops with no callbacks, needed because by - * default buflib buffers can be moved around which must be avoided */ - static struct buflib_callbacks dummy_ops; // Add 31 to handle worst-case misalignment - usb_handle = core_alloc_ex("usb storage", ALLOCATE_BUFFER_SIZE + MAX_CBW_SIZE + 31, &dummy_ops); + usb_handle = core_alloc_ex("usb storage", + ALLOCATE_BUFFER_SIZE + MAX_CBW_SIZE + 31, + &buflib_ops_locked); if (usb_handle < 0) panicf("%s(): OOM", __func__); diff --git a/lib/x1000-installer/src/xf_nandio.c b/lib/x1000-installer/src/xf_nandio.c index ba79cbbcbf..29ff9d9120 100644 --- a/lib/x1000-installer/src/xf_nandio.c +++ b/lib/x1000-installer/src/xf_nandio.c @@ -51,7 +51,7 @@ int xf_nandio_init(struct xf_nandio* nio) alloc_size += CACHEALIGN_SIZE - 1; alloc_size += nio->block_size * 2; - nio->alloc_handle = core_alloc("xf_nandio", alloc_size); + nio->alloc_handle = core_alloc_ex("xf_nandio", alloc_size, &buflib_ops_locked); if(nio->alloc_handle < 0) { rc = XF_E_OUT_OF_MEMORY; goto out_nclose; diff --git a/lib/x1000-installer/src/xf_package.c b/lib/x1000-installer/src/xf_package.c index 78bddded68..04b32cdcb0 100644 --- a/lib/x1000-installer/src/xf_package.c +++ b/lib/x1000-installer/src/xf_package.c @@ -49,7 +49,7 @@ static int pkg_alloc(struct xf_package* pkg) alloc_size += ALIGN_UP_P2(METADATA_SIZE, 3); alloc_size += 7; /* for alignment */ - pkg->alloc_handle = core_alloc("xf_package", alloc_size); + pkg->alloc_handle = core_alloc_ex("xf_package", alloc_size, &buflib_ops_locked); if(pkg->alloc_handle < 0) return XF_E_OUT_OF_MEMORY; diff --git a/lib/x1000-installer/test_lib/core_alloc.c b/lib/x1000-installer/test_lib/core_alloc.c index 5d4edb03f7..719670f8f2 100644 --- a/lib/x1000-installer/test_lib/core_alloc.c +++ b/lib/x1000-installer/test_lib/core_alloc.c @@ -25,6 +25,7 @@ #define N_POINTERS 100 static void* pointers[N_POINTERS]; +struct buflib_callbacks buflib_ops_locked = {NULL, NULL, NULL}; int core_alloc(const char* name, size_t size) { @@ -46,6 +47,12 @@ int core_alloc(const char* name, size_t size) return -1; } +int core_alloc_ex(const char* name, size_t size, struct buflib_callbacks* cb) +{ + (void)cb; + return core_alloc(name, size); +} + int core_free(int handle) { if(handle > 0) { diff --git a/lib/x1000-installer/test_lib/core_alloc.h b/lib/x1000-installer/test_lib/core_alloc.h index 6fb06649fb..2c77e3c274 100644 --- a/lib/x1000-installer/test_lib/core_alloc.h +++ b/lib/x1000-installer/test_lib/core_alloc.h @@ -25,8 +25,18 @@ #define CORE_ALLOC_H #include +#include + +struct buflib_callbacks { + int (*move_callback)(int handle, void* current, void* new); + int (*shrink_callback)(int handle, unsigned hints, void* start, size_t old_size); + void (*sync_callback)(int handle, bool sync_on); +}; + +extern struct buflib_callbacks buflib_ops_locked; int core_alloc(const char* name, size_t size); +int core_alloc_ex(const char* name, size_t size, struct buflib_callbacks* cb); int core_free(int handle); void* core_get_data(int handle);