forked from len0rd/rockbox
Shortcuts: Add move callback for buflib allocations
If we don't provide a callback to buflib_alloc(), the buffer is always movable (to reduce fragmentation). We were passing around buffers to multiple functions that call yield() and might trigger buflib compaction. -> add locking while we are working on the buffers. Also added source code comments that explain why we added the locking in that particular section. Change-Id: Ie32867b0b735ddb2905fd4bd51342f61035f836f
This commit is contained in:
parent
9076b433d1
commit
7265375087
1 changed files with 48 additions and 6 deletions
|
@ -83,6 +83,22 @@ struct shortcut_handle {
|
||||||
static int first_handle = 0;
|
static int first_handle = 0;
|
||||||
static int shortcut_count = 0;
|
static int shortcut_count = 0;
|
||||||
|
|
||||||
|
static int buflib_move_lock;
|
||||||
|
static int move_callback(int handle, void *current, void *new)
|
||||||
|
{
|
||||||
|
(void)handle;
|
||||||
|
(void)current;
|
||||||
|
(void)new;
|
||||||
|
|
||||||
|
if (buflib_move_lock > 0)
|
||||||
|
return BUFLIB_CB_CANNOT_MOVE;
|
||||||
|
|
||||||
|
return BUFLIB_CB_OK;
|
||||||
|
}
|
||||||
|
static struct buflib_callbacks shortcut_ops = {
|
||||||
|
.move_callback = move_callback,
|
||||||
|
};
|
||||||
|
|
||||||
static void reset_shortcuts(void)
|
static void reset_shortcuts(void)
|
||||||
{
|
{
|
||||||
int current_handle = first_handle;
|
int current_handle = first_handle;
|
||||||
|
@ -97,17 +113,18 @@ static void reset_shortcuts(void)
|
||||||
}
|
}
|
||||||
first_handle = 0;
|
first_handle = 0;
|
||||||
shortcut_count = 0;
|
shortcut_count = 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
static struct shortcut* get_shortcut(int index)
|
static struct shortcut* get_shortcut(int index)
|
||||||
{
|
{
|
||||||
int handle_count, handle_index;
|
int handle_count, handle_index;
|
||||||
int current_handle = first_handle;
|
int current_handle = first_handle;
|
||||||
struct shortcut_handle *h = NULL;
|
struct shortcut_handle *h = NULL;
|
||||||
|
|
||||||
if (first_handle == 0)
|
if (first_handle == 0)
|
||||||
{
|
{
|
||||||
first_handle = core_alloc("shortcuts_head", sizeof(struct shortcut_handle));
|
first_handle = core_alloc_ex("shortcuts_head",
|
||||||
|
sizeof(struct shortcut_handle), &shortcut_ops);
|
||||||
if (first_handle <= 0)
|
if (first_handle <= 0)
|
||||||
return NULL;
|
return NULL;
|
||||||
h = core_get_data(first_handle);
|
h = core_get_data(first_handle);
|
||||||
|
@ -126,7 +143,10 @@ static struct shortcut* get_shortcut(int index)
|
||||||
{
|
{
|
||||||
char buf[32];
|
char buf[32];
|
||||||
snprintf(buf, sizeof buf, "shortcuts_%d", index/SHORTCUTS_PER_HANDLE);
|
snprintf(buf, sizeof buf, "shortcuts_%d", index/SHORTCUTS_PER_HANDLE);
|
||||||
h->next_handle = core_alloc(buf, sizeof(struct shortcut_handle));
|
/* prevent invalidation of 'h' during compaction */
|
||||||
|
++buflib_move_lock;
|
||||||
|
h->next_handle = core_alloc_ex(buf, sizeof(struct shortcut_handle), &shortcut_ops);
|
||||||
|
--buflib_move_lock;
|
||||||
if (h->next_handle <= 0)
|
if (h->next_handle <= 0)
|
||||||
return NULL;
|
return NULL;
|
||||||
h = core_get_data(h->next_handle);
|
h = core_get_data(h->next_handle);
|
||||||
|
@ -190,12 +210,17 @@ static void shortcuts_ata_idle_callback(void)
|
||||||
char buf[MAX_PATH];
|
char buf[MAX_PATH];
|
||||||
int current_idx = first_idx_to_writeback;
|
int current_idx = first_idx_to_writeback;
|
||||||
int append = overwrite_shortcuts ? O_TRUNC : O_APPEND;
|
int append = overwrite_shortcuts ? O_TRUNC : O_APPEND;
|
||||||
|
|
||||||
if (first_idx_to_writeback < 0)
|
if (first_idx_to_writeback < 0)
|
||||||
return;
|
return;
|
||||||
fd = open(SHORTCUTS_FILENAME, append|O_RDWR|O_CREAT, 0644);
|
fd = open(SHORTCUTS_FILENAME, append|O_RDWR|O_CREAT, 0644);
|
||||||
if (fd < 0)
|
if (fd < 0)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
|
/* ensure pointer returned by get_shortcut()
|
||||||
|
survives the yield() of write() */
|
||||||
|
++buflib_move_lock;
|
||||||
|
|
||||||
while (current_idx < shortcut_count)
|
while (current_idx < shortcut_count)
|
||||||
{
|
{
|
||||||
struct shortcut* sc = get_shortcut(current_idx++);
|
struct shortcut* sc = get_shortcut(current_idx++);
|
||||||
|
@ -221,6 +246,7 @@ static void shortcuts_ata_idle_callback(void)
|
||||||
reset_shortcuts();
|
reset_shortcuts();
|
||||||
shortcuts_init();
|
shortcuts_init();
|
||||||
}
|
}
|
||||||
|
--buflib_move_lock;
|
||||||
first_idx_to_writeback = -1;
|
first_idx_to_writeback = -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -336,15 +362,25 @@ void shortcuts_init(void)
|
||||||
fd = open_utf8(SHORTCUTS_FILENAME, O_RDONLY);
|
fd = open_utf8(SHORTCUTS_FILENAME, O_RDONLY);
|
||||||
if (fd < 0)
|
if (fd < 0)
|
||||||
return;
|
return;
|
||||||
first_handle = core_alloc("shortcuts_head", sizeof(struct shortcut_handle));
|
first_handle = core_alloc_ex("shortcuts_head", sizeof(struct shortcut_handle), &shortcut_ops);
|
||||||
if (first_handle <= 0)
|
if (first_handle <= 0)
|
||||||
return;
|
return;
|
||||||
h = core_get_data(first_handle);
|
h = core_get_data(first_handle);
|
||||||
h->next_handle = 0;
|
h->next_handle = 0;
|
||||||
|
|
||||||
|
/* we enter readline_cb() multiple times with a buffer
|
||||||
|
obtained when we encounter a "[shortcut]" section.
|
||||||
|
fast_readline() might yield() -> protect buffer */
|
||||||
|
++buflib_move_lock;
|
||||||
|
|
||||||
fast_readline(fd, buf, sizeof buf, ¶m, readline_cb);
|
fast_readline(fd, buf, sizeof buf, ¶m, readline_cb);
|
||||||
close(fd);
|
close(fd);
|
||||||
if (param && verify_shortcut(param))
|
if (param && verify_shortcut(param))
|
||||||
shortcut_count++;
|
shortcut_count++;
|
||||||
|
|
||||||
|
/* invalidate at scope end since "param" contains
|
||||||
|
the shortcut pointer */
|
||||||
|
--buflib_move_lock;
|
||||||
}
|
}
|
||||||
|
|
||||||
static const char * shortcut_menu_get_name(int selected_item, void * data,
|
static const char * shortcut_menu_get_name(int selected_item, void * data,
|
||||||
|
@ -462,6 +498,10 @@ int do_shortcut_menu(void *ignored)
|
||||||
}
|
}
|
||||||
push_current_activity(ACTIVITY_SHORTCUTSMENU);
|
push_current_activity(ACTIVITY_SHORTCUTSMENU);
|
||||||
|
|
||||||
|
/* prevent buffer moves while the menu is active.
|
||||||
|
Playing talk files or other I/O actions call yield() */
|
||||||
|
++buflib_move_lock;
|
||||||
|
|
||||||
while (done == GO_TO_PREVIOUS)
|
while (done == GO_TO_PREVIOUS)
|
||||||
{
|
{
|
||||||
if (simplelist_show_list(&list))
|
if (simplelist_show_list(&list))
|
||||||
|
@ -540,5 +580,7 @@ int do_shortcut_menu(void *ignored)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
pop_current_activity();
|
pop_current_activity();
|
||||||
|
--buflib_move_lock;
|
||||||
|
|
||||||
return done;
|
return done;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue