From a41d4e72dc915a8cb79aa8f1f2177def06d75844 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Sun, 13 Aug 2023 23:12:15 -0700 Subject: til_fb: track all pages and free them in til_fb_free() Pages that have been handed out from the inactive list didn't live on any lists, not until they were submitted. This commit keeps all pages on a separate list to make cleanup of outstanding pages in til_fb_free() trivial, which has also been introduced here. Just tidying up the shutdown to get rid of a bunch of harmless leaks, in an effort to make ASAN more useful in detecting real leaks on graceful exits. --- src/til_fb.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/til_fb.c b/src/til_fb.c index 5de7035..10a5931 100644 --- a/src/til_fb.c +++ b/src/til_fb.c @@ -114,6 +114,7 @@ struct _til_fb_page_t { til_fb_t *fb; void *fb_ops_page; + _til_fb_page_t *all_next, *all_previous; _til_fb_page_t *next, *previous; _til_fb_fragment_t fragment; }; @@ -138,6 +139,9 @@ typedef struct til_fb_t { _til_fb_page_t *inactive_pages_head; /* finished pages available for (re)use */ _til_fb_page_t *inactive_pages_tail; + _til_fb_page_t *all_pages_head; /* all pages allocated */ + _til_fb_page_t *all_pages_tail; + unsigned put_pages_count; } til_fb_t; @@ -226,6 +230,9 @@ static int til_fb_acquire(til_fb_t *fb, _til_fb_page_t *page) /* release the fb, making the visible page inactive */ static void til_fb_release(til_fb_t *fb) { + assert(fb); + assert(fb->active_page); + fb->ops->release(fb, fb->ops_context); /* XXX: this is getting silly, either add a doubly linked list header or @@ -245,6 +252,16 @@ static void til_fb_release(til_fb_t *fb) static void _til_fb_page_free(til_fb_t *fb, _til_fb_page_t *page) { + if (page->all_next) + page->all_next->all_previous = page->all_previous; + else + fb->all_pages_tail = page->all_previous; + + if (page->all_previous) + page->all_previous->all_next = page->all_next; + else + fb->all_pages_head = page->all_next; + fb->ops->page_free(fb, fb->ops_context, page->fb_ops_page); free(page); @@ -291,7 +308,7 @@ static void _til_fb_fragment_memcpy_buf(til_fb_fragment_t *dest, til_fb_fragment } -/* snapshot the contents of fragment */ +/* snapshot the contents of whole-page fragment */ static til_fb_fragment_t * _til_fb_page_snapshot(til_fb_fragment_t **fragment_ptr, int preserve_original) { _til_fb_page_t *page; @@ -299,6 +316,15 @@ static til_fb_fragment_t * _til_fb_page_snapshot(til_fb_fragment_t **fragment_pt assert(fragment_ptr && *fragment_ptr); + /* XXX: note that nothing serializes this _til_fb_page_alloc(), so if anything actually + * tried to do whole-page snapshotting in parallel things would likely explode. + * But as of now, all parallel snapshots of fragments occur on subfragments, not on the + * top-level page, so they don't enter this page allocation code path. + * + * If things started doing threaded page allocations/snapshots, it'd break some assumptions + * all the way down to the $foo_fb backends where they maintain spare pages lists without + * any locking. XXX + */ page = container_of(*fragment_ptr, _til_fb_page_t, fragment); new_page = _til_fb_page_alloc(page->fb); *fragment_ptr = &new_page->fragment.public; @@ -326,6 +352,13 @@ static _til_fb_page_t * _til_fb_page_alloc(til_fb_t *fb) page->fragment.ops.reclaim = _til_fb_page_reclaim; page->fragment.public.ops = &page->fragment.ops; + page->all_next = fb->all_pages_head; + fb->all_pages_head = page; + if (page->all_next) + page->all_next->all_previous = page; + else + fb->all_pages_tail = page; + return page; } @@ -465,10 +498,17 @@ void til_fb_get_put_pages_count(til_fb_t *fb, unsigned *count) til_fb_t * til_fb_free(til_fb_t *fb) { if (fb) { + int count = 0; + if (fb->active_page) til_fb_release(fb); - /* TODO: free all the pages */ + while (fb->all_pages_head) { + _til_fb_page_free(fb, fb->all_pages_head); + count++; + } + + assert(count == fb->n_pages); if (fb->ops->shutdown && fb->ops_context) fb->ops->shutdown(fb, fb->ops_context); -- cgit v1.2.1