From 1718cf5f8a39b922eba3ad1b3c9a9570188362b1 Mon Sep 17 00:00:00 2001 From: Aidan MacDonald Date: Sun, 3 Apr 2022 11:16:39 +0100 Subject: Convert a number of allocations to use buflib pinning Several places in the codebase implemented an ad-hoc form of pinning; they can be converted to use buflib pinning instead. Change-Id: I4450be007e80f6c9cc9f56c2929fa4b9b85ebff3 --- apps/gui/icon.c | 8 ++--- apps/gui/skin_engine/skin_backdrops.c | 14 ++++---- apps/gui/skin_engine/skin_parser.c | 16 ++------- apps/tagcache.c | 13 +++---- apps/tagtree.c | 66 +++++++++++++---------------------- apps/tree.c | 15 ++++++-- apps/tree.h | 13 +++---- 7 files changed, 55 insertions(+), 90 deletions(-) (limited to 'apps') diff --git a/apps/gui/icon.c b/apps/gui/icon.c index 9fe7090f4a..e78aa6841c 100644 --- a/apps/gui/icon.c +++ b/apps/gui/icon.c @@ -63,7 +63,6 @@ static struct iconset { struct bitmap bmp; bool loaded; int handle; - int handle_locked; } iconsets[Iconset_Count][NB_SCREENS]; #define ICON_HEIGHT(screen) (!iconsets[Iconset_user][screen].loaded ? \ @@ -160,8 +159,6 @@ static int buflib_move_callback(int handle, void* current, void* new) struct iconset *set = &iconsets[i][j]; if (set->bmp.data == current) { - if (set->handle_locked > 0) - return BUFLIB_CB_CANNOT_MOVE; set->bmp.data = new; return BUFLIB_CB_OK; } @@ -201,11 +198,10 @@ static void load_icons(const char* filename, enum Iconset iconset, goto finished; } lseek(fd, 0, SEEK_SET); + core_pin(ic->handle); ic->bmp.data = core_get_data(ic->handle); - - ic->handle_locked = 1; size_read = read_bmp_fd(fd, &ic->bmp, buf_size, bmpformat, NULL); - ic->handle_locked = 0; + core_unpin(ic->handle); if (size_read < 0) { diff --git a/apps/gui/skin_engine/skin_backdrops.c b/apps/gui/skin_engine/skin_backdrops.c index 1def0812da..215667d585 100644 --- a/apps/gui/skin_engine/skin_backdrops.c +++ b/apps/gui/skin_engine/skin_backdrops.c @@ -41,7 +41,6 @@ static struct skin_backdrop { } backdrops[NB_BDROPS]; #define NB_BDROPS SKINNABLE_SCREENS_COUNT*NB_SCREENS -static int handle_being_loaded; static int current_lcd_backdrop[NB_SCREENS]; bool skin_backdrop_get_debug(int index, char **path, int *ref_count, size_t *size) @@ -65,8 +64,8 @@ bool skin_backdrop_get_debug(int index, char **path, int *ref_count, size_t *siz static int buflib_move_callback(int handle, void* current, void* new) { - if (handle == handle_being_loaded) - return BUFLIB_CB_CANNOT_MOVE; + (void)handle; + for (int i=0; ibuflib_handles++; _stats->images_size += buf_size; lseek(fd, 0, SEEK_SET); - lock_handle(handle); + core_pin(handle); bitmap->data = core_get_data(handle); int ret = read_bmp_fd(fd, bitmap, buf_size, format, NULL); bitmap->data = NULL; /* do this to force a crash later if the caller doesnt call core_get_data() */ - unlock_handle(); + core_unpin(handle); close(fd); if (ret > 0) { diff --git a/apps/tagcache.c b/apps/tagcache.c index 8db1569379..b6c86d7ddb 100644 --- a/apps/tagcache.c +++ b/apps/tagcache.c @@ -277,17 +277,16 @@ static struct tcramcache { struct ramcache_header *hdr; /* allocated ramcache_header */ int handle; /* buffer handle */ - int move_lock; } tcramcache; static inline void tcrc_buffer_lock(void) { - tcramcache.move_lock++; + core_pin(tcramcache.handle); } static inline void tcrc_buffer_unlock(void) { - tcramcache.move_lock--; + core_unpin(tcramcache.handle); } #else /* ndef HAVE_TC_RAMCACHE */ @@ -3861,10 +3860,9 @@ static bool delete_entry(long idx_id) struct tagfile_entry *tfe; int32_t *seek = &tcramcache.hdr->indices[idx_id].tag_seek[tag]; + /* crc_32 is assumed not to yield (why would it...?) */ tfe = (struct tagfile_entry *)&tcramcache.hdr->tags[tag][*seek]; - tcrc_buffer_lock(); /* protect tfe and seek if crc_32() yield()s */ *seek = crc_32(tfe->tag_data, strlen(tfe->tag_data), 0xffffffff); - tcrc_buffer_unlock(); myidx.tag_seek[tag] = *seek; } else @@ -4005,9 +4003,6 @@ static void fix_ramcache(void* old_addr, void* new_addr) static int move_cb(int handle, void* current, void* new) { (void)handle; - if (tcramcache.move_lock > 0) - return BUFLIB_CB_CANNOT_MOVE; - fix_ramcache(current, new); tcramcache.hdr = new; return BUFLIB_CB_OK; @@ -4092,8 +4087,8 @@ static bool tagcache_dumpload(void) return false; } - tcrc_buffer_lock(); tcramcache.handle = handle; + tcrc_buffer_lock(); tcramcache.hdr = core_get_data(handle); rc = read(fd, tcramcache.hdr, shdr.tc_stat.ramcache_allocated); tcrc_buffer_unlock(); diff --git a/apps/tagtree.c b/apps/tagtree.c index 39bc0ab37c..2ebab7949c 100644 --- a/apps/tagtree.c +++ b/apps/tagtree.c @@ -192,7 +192,7 @@ static int current_entry_count; static struct tree_context *tc; /* a few memory alloc helper */ -static int tagtree_handle, lock_count; +static int tagtree_handle; static size_t tagtree_bufsize, tagtree_buf_used; #define UPDATE(x, y) { x = (typeof(x))((char*)(x) + (y)); } @@ -201,9 +201,6 @@ static int move_callback(int handle, void* current, void* new) (void)handle; (void)current; (void)new; ptrdiff_t diff = new - current; - if (lock_count > 0) - return BUFLIB_CB_CANNOT_MOVE; - if (menu) UPDATE(menu, diff); @@ -251,16 +248,6 @@ static int move_callback(int handle, void* current, void* new) } #undef UPDATE -static inline void tagtree_lock(void) -{ - lock_count++; -} - -static inline void tagtree_unlock(void) -{ - lock_count--; -} - static struct buflib_callbacks ops = { .move_callback = move_callback, .shrink_callback = NULL, @@ -623,7 +610,7 @@ static int add_format(const char *buf) int clause_count = 0; strp++; - tagtree_lock(); + core_pin(tagtree_handle); while (1) { struct tagcache_search_clause *new_clause; @@ -646,7 +633,7 @@ static int add_format(const char *buf) clause_count++; } - tagtree_unlock(); + core_unpin(tagtree_handle); formats[format_count]->clause_count = clause_count; } @@ -719,9 +706,9 @@ static int get_condition(struct search_instruction *inst) } else { - tagtree_lock(); + core_pin(tagtree_handle); bool ret = read_clause(new_clause); - tagtree_unlock(); + core_unpin(tagtree_handle); if (!ret) return -1; } @@ -812,9 +799,9 @@ static bool parse_search(struct menu_entry *entry, const char *str) logf("tag: %d", inst->tagorder[inst->tagorder_count]); - tagtree_lock(); + core_pin(tagtree_handle); while ( (ret = get_condition(inst)) > 0 ) ; - tagtree_unlock(); + core_unpin(tagtree_handle); if (ret < 0) return false; @@ -1176,10 +1163,10 @@ static int parse_line(int n, char *buf, void *parameters) logf("tagtree failed to allocate %s", "menu items"); return -2; } - tagtree_lock(); + core_pin(tagtree_handle); if (parse_search(menu->items[menu->itemcount], buf)) menu->itemcount++; - tagtree_unlock(); + core_unpin(tagtree_handle); return 0; } @@ -1212,8 +1199,8 @@ static bool parse_menu(const char *filename) static void tagtree_unload(struct tree_context *c) { - int i; - tagtree_lock(); + /* may be spurious... */ + core_pin(tagtree_handle); remove_event(PLAYBACK_EVENT_TRACK_BUFFER, tagtree_buffer_event); remove_event(PLAYBACK_EVENT_TRACK_FINISH, tagtree_track_finish_event); @@ -1229,7 +1216,7 @@ static void tagtree_unload(struct tree_context *c) return; } - for (i = 0; i < menu->itemcount; i++) + for (int i = 0; i < menu->itemcount; i++) { dptr->name = NULL; dptr->newtable = 0; @@ -1238,11 +1225,11 @@ static void tagtree_unload(struct tree_context *c) } } - for (i = 0; i < menu_count; i++) + for (int i = 0; i < menu_count; i++) menus[i] = NULL; menu_count = 0; - for (i = 0; i < format_count; i++) + for (int i = 0; i < format_count; i++) formats[i] = NULL; format_count = 0; @@ -1253,9 +1240,6 @@ static void tagtree_unload(struct tree_context *c) if (c) tree_unlock_cache(c); - tagtree_unlock(); - if (lock_count > 0) - tagtree_unlock();/* second unlock to enable re-init */ } void tagtree_init(void) @@ -1285,8 +1269,6 @@ void tagtree_init(void) if (sizeof(struct tagentry) != sizeof(struct entry)) panicf("tagentry(%zu) and entry mismatch(%zu)", sizeof(struct tagentry), sizeof(struct entry)); - if (lock_count > 0) - panicf("tagtree locked after parsing"); /* If no root menu is set, assume it's the first single menu * we have. That shouldn't normally happen. */ @@ -1502,7 +1484,7 @@ static int retrieve_entries(struct tree_context *c, int offset, bool init) /* because tagcache saves the clauses, we need to lock the buffer * for the entire duration of the search */ - tagtree_lock(); + core_pin(tagtree_handle); for (i = 0; i <= level; i++) { int j; @@ -1579,7 +1561,7 @@ static int retrieve_entries(struct tree_context *c, int offset, bool init) fmt = NULL; /* Check the format */ - tagtree_lock(); + core_pin(tagtree_handle); for (i = 0; i < format_count; i++) { if (formats[i]->group_id != csi->format_id[level]) @@ -1592,7 +1574,7 @@ static int retrieve_entries(struct tree_context *c, int offset, bool init) break; } } - tagtree_unlock(); + core_unpin(tagtree_handle); if (strcmp(tcs.result, UNTAGGED) == 0) { @@ -1634,7 +1616,7 @@ static int retrieve_entries(struct tree_context *c, int offset, bool init) logf("format_str() failed"); tagcache_search_finish(&tcs); tree_unlock_cache(c); - tagtree_unlock(); + core_unpin(tagtree_handle); return 0; } else @@ -1675,7 +1657,7 @@ entry_skip_formatter: { /* user aborted */ tagcache_search_finish(&tcs); tree_unlock_cache(c); - tagtree_unlock(); + core_unpin(tagtree_handle); return current_entry_count; } } @@ -1694,7 +1676,7 @@ entry_skip_formatter: { tagcache_search_finish(&tcs); tree_unlock_cache(c); - tagtree_unlock(); + core_unpin(tagtree_handle); return current_entry_count; } @@ -1710,7 +1692,7 @@ entry_skip_formatter: tagcache_search_finish(&tcs); tree_unlock_cache(c); - tagtree_unlock(); + core_unpin(tagtree_handle); if (!sort && (sort_inverse || sort_limit)) { @@ -1870,7 +1852,7 @@ int tagtree_enter(struct tree_context* c) /* lock buflib for possible I/O to protect dptr */ tree_lock_cache(c); - tagtree_lock(); + core_pin(tagtree_handle); bool reset_selection = true; @@ -1940,7 +1922,7 @@ int tagtree_enter(struct tree_context* c) { tagtree_exit(c); tree_unlock_cache(c); - tagtree_unlock(); + core_unpin(tagtree_handle); return 0; } if (csi->clause[i][j]->numeric) @@ -1999,7 +1981,7 @@ int tagtree_enter(struct tree_context* c) } tree_unlock_cache(c); - tagtree_unlock(); + core_unpin(tagtree_handle); return rc; } diff --git a/apps/tree.c b/apps/tree.c index a034fd0545..6622e1c4c2 100644 --- a/apps/tree.c +++ b/apps/tree.c @@ -314,6 +314,18 @@ struct tree_context* tree_get_context(void) return &tc; } +void tree_lock_cache(struct tree_context *t) +{ + core_pin(t->cache.name_buffer_handle); + core_pin(t->cache.entries_handle); +} + +void tree_unlock_cache(struct tree_context *t) +{ + core_unpin(t->cache.name_buffer_handle); + core_unpin(t->cache.entries_handle); +} + /* * Returns the position of a given file in the current directory * returns -1 if not found @@ -1020,9 +1032,6 @@ int rockbox_browse(struct browse_context *browse) static int move_callback(int handle, void* current, void* new) { struct tree_cache* cache = &tc.cache; - if (cache->lock_count > 0) - return BUFLIB_CB_CANNOT_MOVE; - ptrdiff_t diff = new - current; /* FIX_PTR makes sure to not accidentally update static allocations */ #define FIX_PTR(x) \ diff --git a/apps/tree.h b/apps/tree.h index c70ae8dac1..a75e8d2260 100644 --- a/apps/tree.h +++ b/apps/tree.h @@ -52,7 +52,6 @@ struct tree_cache { int name_buffer_handle; /* handle to the name cache */ int max_entries; /* Max entries in the cache */ int name_buffer_size; /* in bytes */ - volatile int lock_count; /* non-0 if buffers may not move */ }; struct browse_context { @@ -120,14 +119,10 @@ void browse_context_init(struct browse_context *browse, int rockbox_browse(struct browse_context *browse); int create_playlist(void); void resume_directory(const char *dir); -static inline void tree_lock_cache(struct tree_context *t) -{ - t->cache.lock_count++; -} -static inline void tree_unlock_cache(struct tree_context *t) -{ - t->cache.lock_count--; -} + +void tree_lock_cache(struct tree_context *t); +void tree_unlock_cache(struct tree_context *t); + #ifdef WIN32 /* it takes an int on windows */ #define getcwd_size_t int -- cgit v1.2.3