diff options
author | Aidan MacDonald <amachronic@protonmail.com> | 2022-04-03 11:16:39 +0100 |
---|---|---|
committer | Aidan MacDonald <amachronic@protonmail.com> | 2022-10-16 14:50:39 +0100 |
commit | 1718cf5f8a39b922eba3ad1b3c9a9570188362b1 (patch) | |
tree | 4f6bf81cb4f382ca04856b98492289825133c5ae /firmware | |
parent | b16bae6fe624d30631bf83290e204197ab136c12 (diff) | |
download | rockbox-1718cf5f8a39b922eba3ad1b3c9a9570188362b1.tar.gz rockbox-1718cf5f8a39b922eba3ad1b3c9a9570188362b1.zip |
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
Diffstat (limited to 'firmware')
-rw-r--r-- | firmware/common/dircache.c | 60 | ||||
-rw-r--r-- | firmware/common/unicode.c | 29 | ||||
-rw-r--r-- | firmware/font.c | 16 |
3 files changed, 38 insertions, 67 deletions
diff --git a/firmware/common/dircache.c b/firmware/common/dircache.c index 8917b3348e..1d6371a6b5 100644 --- a/firmware/common/dircache.c +++ b/firmware/common/dircache.c | |||
@@ -249,7 +249,6 @@ static struct dircache_runinfo | |||
249 | /* cache buffer info */ | 249 | /* cache buffer info */ |
250 | int handle; /* buflib buffer handle */ | 250 | int handle; /* buflib buffer handle */ |
251 | size_t bufsize; /* size of buflib allocation - 1 */ | 251 | size_t bufsize; /* size of buflib allocation - 1 */ |
252 | int buflocked; /* don't move due to other allocs */ | ||
253 | union { | 252 | union { |
254 | void *p; /* address of buffer - ENTRYSIZE */ | 253 | void *p; /* address of buffer - ENTRYSIZE */ |
255 | struct dircache_entry *pentry; /* alias of .p to assist entry resolution */ | 254 | struct dircache_entry *pentry; /* alias of .p to assist entry resolution */ |
@@ -329,29 +328,9 @@ static inline void dumpster_clean_buffer(void *p, size_t size) | |||
329 | */ | 328 | */ |
330 | static int move_callback(int handle, void *current, void *new) | 329 | static int move_callback(int handle, void *current, void *new) |
331 | { | 330 | { |
332 | if (dircache_runinfo.buflocked) | 331 | (void)handle; (void)current; |
333 | return BUFLIB_CB_CANNOT_MOVE; | ||
334 | |||
335 | dircache_runinfo.p = new - ENTRYSIZE; | 332 | dircache_runinfo.p = new - ENTRYSIZE; |
336 | |||
337 | return BUFLIB_CB_OK; | 333 | return BUFLIB_CB_OK; |
338 | (void)handle; (void)current; | ||
339 | } | ||
340 | |||
341 | /** | ||
342 | * add a "don't move" lock count | ||
343 | */ | ||
344 | static inline void buffer_lock(void) | ||
345 | { | ||
346 | dircache_runinfo.buflocked++; | ||
347 | } | ||
348 | |||
349 | /** | ||
350 | * remove a "don't move" lock count | ||
351 | */ | ||
352 | static inline void buffer_unlock(void) | ||
353 | { | ||
354 | dircache_runinfo.buflocked--; | ||
355 | } | 334 | } |
356 | 335 | ||
357 | 336 | ||
@@ -530,14 +509,14 @@ static void set_buffer(int handle, size_t size) | |||
530 | 509 | ||
531 | /** | 510 | /** |
532 | * remove the allocation from dircache control and return the handle | 511 | * remove the allocation from dircache control and return the handle |
512 | * Note that dircache must not be using the buffer! | ||
533 | */ | 513 | */ |
534 | static int reset_buffer(void) | 514 | static int reset_buffer(void) |
535 | { | 515 | { |
536 | int handle = dircache_runinfo.handle; | 516 | int handle = dircache_runinfo.handle; |
537 | if (handle > 0) | 517 | if (handle > 0) |
538 | { | 518 | { |
539 | /* don't mind .p; it might get changed by the callback even after | 519 | /* don't mind .p; buffer presence is determined by the following: */ |
540 | this call; buffer presence is determined by the following: */ | ||
541 | dircache_runinfo.handle = 0; | 520 | dircache_runinfo.handle = 0; |
542 | dircache_runinfo.bufsize = 0; | 521 | dircache_runinfo.bufsize = 0; |
543 | } | 522 | } |
@@ -1857,7 +1836,7 @@ static void reset_cache(void) | |||
1857 | */ | 1836 | */ |
1858 | static void build_volumes(void) | 1837 | static void build_volumes(void) |
1859 | { | 1838 | { |
1860 | buffer_lock(); | 1839 | core_pin(dircache_runinfo.handle); |
1861 | 1840 | ||
1862 | for (int i = 0; i < NUM_VOLUMES; i++) | 1841 | for (int i = 0; i < NUM_VOLUMES; i++) |
1863 | { | 1842 | { |
@@ -1903,12 +1882,14 @@ static void build_volumes(void) | |||
1903 | 1882 | ||
1904 | logf("Done, %ld KiB used", dircache.size / 1024); | 1883 | logf("Done, %ld KiB used", dircache.size / 1024); |
1905 | 1884 | ||
1906 | buffer_unlock(); | 1885 | core_unpin(dircache_runinfo.handle); |
1907 | } | 1886 | } |
1908 | 1887 | ||
1909 | /** | 1888 | /** |
1910 | * allocate buffer and return whether or not a synchronous build should take | 1889 | * allocate buffer and return whether or not a synchronous build should take |
1911 | * place; if 'realloced' is NULL, it's just a query about what will happen | 1890 | * place; if 'realloced' is NULL, it's just a query about what will happen |
1891 | * | ||
1892 | * Note this must be called with the dircache_lock() active. | ||
1912 | */ | 1893 | */ |
1913 | static int prepare_build(bool *realloced) | 1894 | static int prepare_build(bool *realloced) |
1914 | { | 1895 | { |
@@ -1958,16 +1939,14 @@ static int prepare_build(bool *realloced) | |||
1958 | *realloced = true; | 1939 | *realloced = true; |
1959 | reset_cache(); | 1940 | reset_cache(); |
1960 | 1941 | ||
1961 | buffer_lock(); | ||
1962 | |||
1963 | int handle = reset_buffer(); | 1942 | int handle = reset_buffer(); |
1964 | dircache_unlock(); | 1943 | dircache_unlock(); /* release lock held by caller */ |
1965 | 1944 | ||
1966 | core_free(handle); | 1945 | core_free(handle); |
1967 | 1946 | ||
1968 | handle = alloc_cache(size); | 1947 | handle = alloc_cache(size); |
1969 | 1948 | ||
1970 | dircache_lock(); | 1949 | dircache_lock(); /* reacquire lock */ |
1971 | 1950 | ||
1972 | if (dircache_runinfo.suspended && handle > 0) | 1951 | if (dircache_runinfo.suspended && handle > 0) |
1973 | { | 1952 | { |
@@ -1979,13 +1958,9 @@ static int prepare_build(bool *realloced) | |||
1979 | } | 1958 | } |
1980 | 1959 | ||
1981 | if (handle <= 0) | 1960 | if (handle <= 0) |
1982 | { | ||
1983 | buffer_unlock(); | ||
1984 | return -1; | 1961 | return -1; |
1985 | } | ||
1986 | 1962 | ||
1987 | set_buffer(handle, size); | 1963 | set_buffer(handle, size); |
1988 | buffer_unlock(); | ||
1989 | 1964 | ||
1990 | return syncbuild; | 1965 | return syncbuild; |
1991 | } | 1966 | } |
@@ -2384,9 +2359,9 @@ void dircache_fileop_create(struct file_base_info *dirinfop, | |||
2384 | if ((dinp->attr & ATTR_DIRECTORY) && !is_dotdir_name(basename)) | 2359 | if ((dinp->attr & ATTR_DIRECTORY) && !is_dotdir_name(basename)) |
2385 | { | 2360 | { |
2386 | /* scan-in the contents of the new directory at this level only */ | 2361 | /* scan-in the contents of the new directory at this level only */ |
2387 | buffer_lock(); | 2362 | core_pin(dircache_runinfo.handle); |
2388 | sab_process_dir(infop, false); | 2363 | sab_process_dir(infop, false); |
2389 | buffer_unlock(); | 2364 | core_unpin(dircache_runinfo.handle); |
2390 | } | 2365 | } |
2391 | } | 2366 | } |
2392 | 2367 | ||
@@ -2915,7 +2890,7 @@ void dircache_dump(void) | |||
2915 | 2890 | ||
2916 | if (dircache_runinfo.handle) | 2891 | if (dircache_runinfo.handle) |
2917 | { | 2892 | { |
2918 | buffer_lock(); | 2893 | core_pin(dircache_runinfo.handle); |
2919 | 2894 | ||
2920 | /* bin */ | 2895 | /* bin */ |
2921 | write(fdbin, dircache_runinfo.p + ENTRYSIZE, | 2896 | write(fdbin, dircache_runinfo.p + ENTRYSIZE, |
@@ -2985,7 +2960,7 @@ void dircache_dump(void) | |||
2985 | tm.tm_hour, tm.tm_min, tm.tm_sec); | 2960 | tm.tm_hour, tm.tm_min, tm.tm_sec); |
2986 | } | 2961 | } |
2987 | 2962 | ||
2988 | buffer_unlock(); | 2963 | core_unpin(dircache_runinfo.handle); |
2989 | } | 2964 | } |
2990 | 2965 | ||
2991 | dircache_unlock(); | 2966 | dircache_unlock(); |
@@ -3104,7 +3079,6 @@ int dircache_load(void) | |||
3104 | } | 3079 | } |
3105 | 3080 | ||
3106 | dircache_lock(); | 3081 | dircache_lock(); |
3107 | buffer_lock(); | ||
3108 | 3082 | ||
3109 | if (!dircache_is_clean(false)) | 3083 | if (!dircache_is_clean(false)) |
3110 | goto error; | 3084 | goto error; |
@@ -3113,6 +3087,7 @@ int dircache_load(void) | |||
3113 | dircache = maindata.dircache; | 3087 | dircache = maindata.dircache; |
3114 | 3088 | ||
3115 | set_buffer(handle, bufsize); | 3089 | set_buffer(handle, bufsize); |
3090 | core_pin(handle); | ||
3116 | hasbuffer = true; | 3091 | hasbuffer = true; |
3117 | 3092 | ||
3118 | /* convert back to in-RAM representation */ | 3093 | /* convert back to in-RAM representation */ |
@@ -3167,13 +3142,13 @@ int dircache_load(void) | |||
3167 | dircache_enable_internal(false); | 3142 | dircache_enable_internal(false); |
3168 | 3143 | ||
3169 | /* cache successfully loaded */ | 3144 | /* cache successfully loaded */ |
3145 | core_unpin(handle); | ||
3170 | logf("Done, %ld KiB used", dircache.size / 1024); | 3146 | logf("Done, %ld KiB used", dircache.size / 1024); |
3171 | rc = 0; | 3147 | rc = 0; |
3172 | error: | 3148 | error: |
3173 | if (rc < 0 && hasbuffer) | 3149 | if (rc < 0 && hasbuffer) |
3174 | reset_buffer(); | 3150 | reset_buffer(); |
3175 | 3151 | ||
3176 | buffer_unlock(); | ||
3177 | dircache_unlock(); | 3152 | dircache_unlock(); |
3178 | 3153 | ||
3179 | error_nolock: | 3154 | error_nolock: |
@@ -3199,8 +3174,9 @@ int dircache_save(void) | |||
3199 | if (fd < 0) | 3174 | if (fd < 0) |
3200 | return -1; | 3175 | return -1; |
3201 | 3176 | ||
3177 | /* it seems the handle *must* exist if this function is called */ | ||
3202 | dircache_lock(); | 3178 | dircache_lock(); |
3203 | buffer_lock(); | 3179 | core_pin(dircache_runinfo.handle); |
3204 | 3180 | ||
3205 | int rc = -1; | 3181 | int rc = -1; |
3206 | 3182 | ||
@@ -3269,7 +3245,7 @@ int dircache_save(void) | |||
3269 | that makes what was saved completely invalid */ | 3245 | that makes what was saved completely invalid */ |
3270 | rc = 0; | 3246 | rc = 0; |
3271 | error: | 3247 | error: |
3272 | buffer_unlock(); | 3248 | core_unpin(dircache_runinfo.handle); |
3273 | dircache_unlock(); | 3249 | dircache_unlock(); |
3274 | 3250 | ||
3275 | if (rc < 0) | 3251 | if (rc < 0) |
diff --git a/firmware/common/unicode.c b/firmware/common/unicode.c index f0f663f712..e53ad6bcb0 100644 --- a/firmware/common/unicode.c +++ b/firmware/common/unicode.c | |||
@@ -168,6 +168,10 @@ static unsigned short default_cp_table_buf[MAX_CP_TABLE_SIZE+1]; | |||
168 | do {} while (0) | 168 | do {} while (0) |
169 | #define cp_table_alloc(filename, size, opsp) \ | 169 | #define cp_table_alloc(filename, size, opsp) \ |
170 | ({ (void)(opsp); 1; }) | 170 | ({ (void)(opsp); 1; }) |
171 | #define cp_table_pin(handle) \ | ||
172 | do { (void)handle; } while(0) | ||
173 | #define cp_table_unpin(handle) \ | ||
174 | do { (void)handle; } while(0) | ||
171 | #else | 175 | #else |
172 | #define cp_table_alloc(filename, size, opsp) \ | 176 | #define cp_table_alloc(filename, size, opsp) \ |
173 | core_alloc_ex((filename), (size), (opsp)) | 177 | core_alloc_ex((filename), (size), (opsp)) |
@@ -175,6 +179,10 @@ static unsigned short default_cp_table_buf[MAX_CP_TABLE_SIZE+1]; | |||
175 | core_free(handle) | 179 | core_free(handle) |
176 | #define cp_table_get_data(handle) \ | 180 | #define cp_table_get_data(handle) \ |
177 | core_get_data(handle) | 181 | core_get_data(handle) |
182 | #define cp_table_pin(handle) \ | ||
183 | core_pin(handle) | ||
184 | #define cp_table_unpin(handle) \ | ||
185 | core_unpin(handle) | ||
178 | #endif | 186 | #endif |
179 | 187 | ||
180 | static const unsigned char utf8comp[6] = | 188 | static const unsigned char utf8comp[6] = |
@@ -191,21 +199,8 @@ static inline void cptable_tohw16(uint16_t *buf, unsigned int count) | |||
191 | (void)buf; (void)count; | 199 | (void)buf; (void)count; |
192 | } | 200 | } |
193 | 201 | ||
194 | static int move_callback(int handle, void *current, void *new) | ||
195 | { | ||
196 | /* we don't keep a pointer but we have to stop it if this applies to a | ||
197 | buffer not yet swapped-in since it will likely be in use in an I/O | ||
198 | call */ | ||
199 | return (handle != default_cp_handle || default_cp_table_ref != 0) ? | ||
200 | BUFLIB_CB_CANNOT_MOVE : BUFLIB_CB_OK; | ||
201 | (void)current; (void)new; | ||
202 | } | ||
203 | |||
204 | static int alloc_and_load_cp_table(int cp, void *buf) | 202 | static int alloc_and_load_cp_table(int cp, void *buf) |
205 | { | 203 | { |
206 | static struct buflib_callbacks ops = | ||
207 | { .move_callback = move_callback }; | ||
208 | |||
209 | /* alloc and read only if there is an associated file */ | 204 | /* alloc and read only if there is an associated file */ |
210 | const char *filename = cp_info[cp].filename; | 205 | const char *filename = cp_info[cp].filename; |
211 | if (!filename) | 206 | if (!filename) |
@@ -228,13 +223,17 @@ static int alloc_and_load_cp_table(int cp, void *buf) | |||
228 | !(size % (off_t)sizeof (uint16_t))) { | 223 | !(size % (off_t)sizeof (uint16_t))) { |
229 | 224 | ||
230 | /* if the buffer is provided, use that but don't alloc */ | 225 | /* if the buffer is provided, use that but don't alloc */ |
231 | int handle = buf ? 0 : cp_table_alloc(filename, size, &ops); | 226 | int handle = buf ? 0 : cp_table_alloc(filename, size, NULL); |
232 | if (handle > 0) | 227 | if (handle > 0) { |
228 | cp_table_pin(handle); | ||
233 | buf = cp_table_get_data(handle); | 229 | buf = cp_table_get_data(handle); |
230 | } | ||
234 | 231 | ||
235 | if (buf && read(fd, buf, size) == size) { | 232 | if (buf && read(fd, buf, size) == size) { |
236 | close(fd); | 233 | close(fd); |
237 | cptable_tohw16(buf, size / sizeof (uint16_t)); | 234 | cptable_tohw16(buf, size / sizeof (uint16_t)); |
235 | if (handle > 0) | ||
236 | cp_table_unpin(handle); | ||
238 | return handle; | 237 | return handle; |
239 | } | 238 | } |
240 | 239 | ||
diff --git a/firmware/font.c b/firmware/font.c index 724cb84d57..7ce64ed47d 100644 --- a/firmware/font.c +++ b/firmware/font.c | |||
@@ -90,7 +90,6 @@ extern struct font sysfont; | |||
90 | 90 | ||
91 | struct buflib_alloc_data { | 91 | struct buflib_alloc_data { |
92 | struct font font; /* must be the first member! */ | 92 | struct font font; /* must be the first member! */ |
93 | int handle_locks; /* is the buflib handle currently locked? */ | ||
94 | int refcount; /* how many times has this font been loaded? */ | 93 | int refcount; /* how many times has this font been loaded? */ |
95 | unsigned char buffer[]; | 94 | unsigned char buffer[]; |
96 | }; | 95 | }; |
@@ -107,9 +106,6 @@ static int buflibmove_callback(int handle, void* current, void* new) | |||
107 | struct buflib_alloc_data *alloc = (struct buflib_alloc_data*)current; | 106 | struct buflib_alloc_data *alloc = (struct buflib_alloc_data*)current; |
108 | ptrdiff_t diff = new - current; | 107 | ptrdiff_t diff = new - current; |
109 | 108 | ||
110 | if (alloc->handle_locks > 0) | ||
111 | return BUFLIB_CB_CANNOT_MOVE; | ||
112 | |||
113 | #define UPDATE(x) if (x) { x = PTR_ADD(x, diff); } | 109 | #define UPDATE(x) if (x) { x = PTR_ADD(x, diff); } |
114 | 110 | ||
115 | UPDATE(alloc->font.bits); | 111 | UPDATE(alloc->font.bits); |
@@ -129,18 +125,18 @@ static void lock_font_handle(int handle, bool lock) | |||
129 | { | 125 | { |
130 | if ( handle < 0 ) | 126 | if ( handle < 0 ) |
131 | return; | 127 | return; |
132 | struct buflib_alloc_data *alloc = core_get_data(handle); | 128 | |
133 | if ( lock ) | 129 | if (lock) |
134 | alloc->handle_locks++; | 130 | core_pin(handle); |
135 | else | 131 | else |
136 | alloc->handle_locks--; | 132 | core_unpin(handle); |
137 | } | 133 | } |
138 | 134 | ||
139 | void font_lock(int font_id, bool lock) | 135 | void font_lock(int font_id, bool lock) |
140 | { | 136 | { |
141 | if( font_id < 0 || font_id >= MAXFONTS ) | 137 | if( font_id < 0 || font_id >= MAXFONTS ) |
142 | return; | 138 | return; |
143 | if( buflib_allocations[font_id] >= 0 ) | 139 | if( buflib_allocations[font_id] > 0 ) |
144 | lock_font_handle(buflib_allocations[font_id], lock); | 140 | lock_font_handle(buflib_allocations[font_id], lock); |
145 | } | 141 | } |
146 | 142 | ||
@@ -522,8 +518,8 @@ int font_load_ex( const char *path, size_t buf_size, int glyphs ) | |||
522 | return -1; | 518 | return -1; |
523 | } | 519 | } |
524 | struct buflib_alloc_data *pdata; | 520 | struct buflib_alloc_data *pdata; |
521 | core_pin(handle); | ||
525 | pdata = core_get_data(handle); | 522 | pdata = core_get_data(handle); |
526 | pdata->handle_locks = 1; | ||
527 | pdata->refcount = 1; | 523 | pdata->refcount = 1; |
528 | 524 | ||
529 | /* load and init */ | 525 | /* load and init */ |