From fca2584d89a2660bf0653c853f3815053869071a Mon Sep 17 00:00:00 2001 From: Sparchatus Date: Wed, 13 Apr 2022 12:55:56 +0000 Subject: [PATCH] fine grained paging locking to avoid deadlocks --- lib/aos/paging.c | 231 +++++++++++++++++++++++++++++------------------ 1 file changed, 142 insertions(+), 89 deletions(-) diff --git a/lib/aos/paging.c b/lib/aos/paging.c index b3e265e..d8a919c 100644 --- a/lib/aos/paging.c +++ b/lib/aos/paging.c @@ -30,13 +30,24 @@ #define PT_STATIC_EXCEPTION_STACK_SIZE (4 * BASE_PAGE_SIZE) +#define PAGING_LOCK {thread_mutex_lock(&st->lock);} +#define PAGING_UNLOCK {thread_mutex_unlock(&st->lock);} + +/* + * Paging locking strategy: + * - lock whenever modifying or reading the paging state + * - never hold a paging lock when calling code that might take another lock + * - there must never be a page fault while holding the lock + * + * For example: + * - we MUST NOT allocate a slot while holding the page lock + * - we MUST hold the lock when using slab_alloc and slab_free + */ + static struct paging_state current; // initial page fault handler stack space static char pt_static_exception_stack[PT_STATIC_EXCEPTION_STACK_SIZE]; -static errval_t _paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, - struct capref frame, size_t bytes, int flags); - __attribute__((__used__)) static char *pt_exception_type_to_string(enum exception_type type, int subtype) { @@ -114,9 +125,7 @@ static void pt_exception_handler(enum exception_type type, int subtype, lvaddr_t page_addr = ROUND_DOWN((lvaddr_t)addr, BASE_PAGE_SIZE); struct paging_state *st = get_current_paging_state(); - // debug_printf("[pt_exception_handler] Attempting to get paging mutex.\n"); - thread_mutex_lock_nested(&st->lock); - // debug_printf("[pt_exception_handler] Got paging mutex.\n"); + PAGING_LOCK struct pt_vaddr_reg_t *vaddr_reg = &st->vaddr_head; for (; vaddr_reg != NULL; vaddr_reg = vaddr_reg->next) { @@ -133,8 +142,10 @@ static void pt_exception_handler(enum exception_type type, int subtype, if (is_mapped(st, page_addr)) { // Page has already been mapped by another thread before we took the lock. + PAGING_UNLOCK return; } + PAGING_UNLOCK struct capref frame; err = frame_alloc(&frame, BASE_PAGE_SIZE, NULL); @@ -142,14 +153,18 @@ static void pt_exception_handler(enum exception_type type, int subtype, USER_PANIC_ERR(err, "Failed to allocate frame in page fault handler"); } - err = _paging_map_fixed_attr(st, page_addr, frame, BASE_PAGE_SIZE, VREGION_FLAGS_READ_WRITE); + err = paging_map_fixed_attr(st, page_addr, frame, BASE_PAGE_SIZE, VREGION_FLAGS_READ_WRITE); if (err_is_fail(err)) { - USER_PANIC_ERR(err, "Failed to map frame in page fault handler"); + if(err_no(err) != LIB_ERR_PMAP_EXISTING_MAPPING) { + USER_PANIC_ERR(err, "Failed to map frame in page fault handler"); + } + debug_printf("[pt_exception_handler] another thread was faster to create the mapping\n"); + // if the error is that the mapping already exists then we can ignore it and free the allocated frame. + // another thread was faster in mapping it than we were + cap_destroy(frame); } - // debug_printf("[pt_exception_handler] Releasing paging mutex.\n"); - thread_mutex_unlock(&st->lock); - // debug_printf("[pt_exception_handler] done\n"); + debug_printf("[pt_exception_handler] done\n"); } /** @@ -202,11 +217,14 @@ static errval_t pt_alloc_level(struct paging_state *st, struct capref *pt_cap, u } void pt_print_state(struct paging_state *st) { - thread_mutex_lock_nested(&st->lock); + PAGING_LOCK // iterates over all the page table entries and prints them debug_printf("L0\n"); - if(st->l0_pt.children == NULL) return; + if(st->l0_pt.children == NULL){ + PAGING_UNLOCK + return; + } for(size_t i0 = 0; i0 < PTABLE_ENTRIES; ++i0) { struct pt_t *l1_pt = st->l0_pt.children[i0]; if (l1_pt == NULL) continue; @@ -238,7 +256,34 @@ void pt_print_state(struct paging_state *st) { reg = reg->next; } - thread_mutex_unlock(&st->lock); + PAGING_UNLOCK +} + +// refill the slab and hold the paging lock for the slab growing but not before +static errval_t pt_slab_refill_locked(struct paging_state *st, struct slab_allocator *slabs, size_t size) { + errval_t err; + size_t alloc_bytes; + struct capref frame_slot; + err = frame_alloc(&frame_slot, size, &alloc_bytes); + if (err_is_fail(err)) return err_push(err, LIB_ERR_FRAME_ALLOC); + + void *vaddr = NULL; + err = paging_map_frame_attr( + get_current_paging_state(), &vaddr, + alloc_bytes, frame_slot, VREGION_FLAGS_READ_WRITE + ); + if (err_is_fail(err)) { + // TODO: free the RAM, but without freeing the frame slot + cap_delete(frame_slot); + return err; + } + debug_printf("DEBUG rueegges: pt_ensure_slabs - mapped\n"); + + PAGING_LOCK + slab_grow(slabs, vaddr, alloc_bytes); + PAGING_UNLOCK + + return SYS_ERR_OK; } // NOTE rueegges: each paging fixed call can use up to 4 pt slabs, up to 3 children slabs and up to 7 slots @@ -249,39 +294,45 @@ void pt_print_state(struct paging_state *st) { static errval_t pt_ensure_slabs(struct paging_state *st) { errval_t err; + PAGING_LOCK + // ensure there is enough space to refill the page tables at any time if(slab_freecount(&st->pt_slabs) <= PT_PT_SLAB_MIN_SPACE && !st->refilling) { - // ASSESSMENT M1: show refilling - // debug_printf("DEBUG rueegges: pt_ensure_slabs - refilling pt_slab\n"); st->refilling = 1; - err = slab_default_refill(&st->pt_slabs); - + // we need to unlock because refilling requires paging functionality + PAGING_UNLOCK + err = pt_slab_refill_locked(st, &st->pt_slabs, BASE_PAGE_SIZE); + PAGING_LOCK st->refilling = 0; - // debug_printf("DEBUG rueegges: pt_ensure_slabs - refilling pt_slab DONE\n"); if(err_is_fail(err)) { + PAGING_UNLOCK return err_push(err, LIB_ERR_SLAB_REFILL); } } if(slab_freecount(&st->pt_children_slabs) <= PT_CHILDREN_SLAB_MIN_SPACE && !st->refilling) { - // ASSESSMENT M1: show refilling - // debug_printf("DEBUG rueegges: pt_ensure_slabs - refilling pt_children_slabs\n"); - + debug_printf("DEBUG rueegges: pt_ensure_slabs - refilling pt_children_slabs\n"); st->refilling = 1; - err = slab_refill_pages(&st->pt_children_slabs, LARGE_PAGE_SIZE); + // we need to unlock because refilling requires paging functionality + PAGING_UNLOCK + err = pt_slab_refill_locked(st, &st->pt_children_slabs, LARGE_PAGE_SIZE); + PAGING_LOCK st->refilling = 0; - // debug_printf("DEBUG rueegges: pt_ensure_slabs - refilling pt_children_slabs DONE\n"); + debug_printf("DEBUG rueegges: pt_ensure_slabs - refilling pt_children_slabs DONE\n"); if(err_is_fail(err)) { + PAGING_UNLOCK return err_push(err, LIB_ERR_SLAB_REFILL); } } + PAGING_UNLOCK + return SYS_ERR_OK; } @@ -295,10 +346,13 @@ static errval_t pt_ensure(struct paging_state *st, struct pt_t *pt_parent, size_ assert(pt_parent->children != NULL); assert(level > 0 && level <= 3); + PAGING_LOCK // check if the page table already exists if (pt_parent->children[pt_index] != NULL) { + PAGING_UNLOCK return SYS_ERR_OK; } + PAGING_UNLOCK // make sure slot and slab refilling is performed in time err = pt_ensure_slabs(st); @@ -308,6 +362,7 @@ static errval_t pt_ensure(struct paging_state *st, struct pt_t *pt_parent, size_ // create the page table struct capref pt_cap; + // no locking needed. it does not alter the paging state err = pt_alloc_level(st, &pt_cap, level); if (err_is_fail(err)) { DEBUG_ERR(err, "Failed pt_alloc l%u", level); @@ -315,8 +370,10 @@ static errval_t pt_ensure(struct paging_state *st, struct pt_t *pt_parent, size_ } // allocate shadow page table space + PAGING_LOCK struct pt_t *pt_meta = (struct pt_t *) slab_alloc(&st->pt_slabs); if(pt_meta == NULL) { + PAGING_UNLOCK ram_free(pt_cap); return LIB_ERR_SLAB_ALLOC_FAIL; } @@ -324,16 +381,20 @@ static errval_t pt_ensure(struct paging_state *st, struct pt_t *pt_parent, size_ struct pt_t **pt_children = (struct pt_t **) slab_alloc(&st->pt_children_slabs); if(pt_children == NULL) { slab_free(&st->pt_slabs, pt_meta); + PAGING_UNLOCK ram_free(pt_cap); return LIB_ERR_SLAB_ALLOC_FAIL; } + PAGING_UNLOCK // create new mapping struct capref pt_mapping; err = st->slot_alloc->alloc(st->slot_alloc, &pt_mapping); if (err_is_fail(err)) { + PAGING_LOCK slab_free(&st->pt_slabs, pt_meta); slab_free(&st->pt_children_slabs, pt_children); + PAGING_UNLOCK ram_free(pt_cap); return err_push(err, LIB_ERR_SLOT_ALLOC); } @@ -344,20 +405,24 @@ static errval_t pt_ensure(struct paging_state *st, struct pt_t *pt_parent, size_ if (err_is_fail(err_err)) { DEBUG_ERR(err_err, "Failed to free capability slot during error handling"); } + PAGING_LOCK slab_free(&st->pt_slabs, pt_meta); slab_free(&st->pt_children_slabs, pt_children); + PAGING_UNLOCK ram_free(pt_cap); return SYS_ERR_OK; } + PAGING_LOCK err = vnode_map(pt_parent->cap_pt, pt_cap, pt_index, 0, 0, 1, pt_mapping); if (err_is_fail(err)) { + slab_free(&st->pt_slabs, pt_meta); + slab_free(&st->pt_children_slabs, pt_children); + PAGING_UNLOCK errval_t err_err = st->slot_alloc->free(st->slot_alloc, pt_mapping); if (err_is_fail(err_err)) { DEBUG_ERR(err_err, "Failed to free capability slot during error handling"); } - slab_free(&st->pt_slabs, pt_meta); - slab_free(&st->pt_children_slabs, pt_children); ram_free(pt_cap); DEBUG_ERR(err, "Failed vnode_map for pt l%u", level); return err_push(err, LIB_ERR_VNODE_MAP); @@ -372,6 +437,8 @@ static errval_t pt_ensure(struct paging_state *st, struct pt_t *pt_parent, size_ pt_parent->children[pt_index] = pt_meta; + PAGING_UNLOCK + return SYS_ERR_OK; } @@ -518,7 +585,7 @@ errval_t paging_init_onthread(struct thread *t) // - setup exception handler for thread `t'. errval_t err; - debug_printf("paging_init_onthread thread id: %lx\n", t->id); + debug_printf("[paging_init_onthread] thread id: %lx\n", t->id); // TODO rueegges: Is this how we are supposed to get the exception handler stack? size_t stack_size = PT_STATIC_EXCEPTION_STACK_SIZE; @@ -605,7 +672,7 @@ static errval_t paging_insert_vaddr_reg( return SYS_ERR_OK; } -static errval_t _paging_alloc_ext( +errval_t paging_alloc_ext( struct paging_state *st, void **buf, size_t bytes, size_t alignment, bool heap ) { @@ -630,6 +697,7 @@ static errval_t _paging_alloc_ext( return err; } + PAGING_LOCK struct pt_vaddr_reg_t *vaddr_reg = &st->vaddr_head; while(vaddr_reg != NULL) { // calculate the number of bytes to skip to achieve alignment @@ -641,30 +709,22 @@ static errval_t _paging_alloc_ext( // allocate the new region, potentially splitting off a prefix and postfix from the region err = paging_insert_vaddr_reg(st, vaddr_reg, prefix_size, bytes, heap); if (err_is_fail(err)) { + PAGING_UNLOCK return err; } *buf = (void *) (vaddr_reg->base + prefix_size); + PAGING_UNLOCK return SYS_ERR_OK; } vaddr_reg = vaddr_reg -> next; } + PAGING_UNLOCK return LIB_ERR_OUT_OF_VIRTUAL_ADDR; } -errval_t paging_alloc_ext(struct paging_state *st, void **buf, size_t bytes, size_t alignment, bool heap) -{ - errval_t err; - - thread_mutex_lock_nested(&st->lock); - err = _paging_alloc_ext(st, buf, bytes, alignment, heap); - thread_mutex_unlock(&st->lock); - - return err; -} - /** * @brief Find a free region of virtual address space that is large enough to accomodate a * buffer of size 'bytes'. @@ -681,7 +741,19 @@ errval_t paging_alloc(struct paging_state *st, void **buf, size_t bytes, size_t return paging_alloc_ext(st, buf, bytes, alignment, false); } -static errval_t _paging_map_frame_attr(struct paging_state *st, void **buf, size_t bytes, +/** + * \brief Finds a free virtual address and maps `bytes` of the supplied frame at that address + * + * @param[in] st the paging state to create the mapping in + * @param[out] buf returns the virtual address at which this frame has been mapped. + * @param[in] bytes the number of bytes to map. + * @param[in] frame the frame capability to be mapped + * @param[in] flags The flags that are to be set for the newly mapped region, + * see 'paging_flags_t' in paging_types.h . + * + * @return Either SYS_ERR_OK if no error occured or an error indicating what went wrong otherwise. + */ +errval_t paging_map_frame_attr(struct paging_state *st, void **buf, size_t bytes, struct capref frame, int flags) { errval_t err; @@ -707,30 +779,18 @@ static errval_t _paging_map_frame_attr(struct paging_state *st, void **buf, size } /** - * \brief Finds a free virtual address and maps `bytes` of the supplied frame at that address + * @brief mapps the provided frame at the supplied address in the paging state * - * @param[in] st the paging state to create the mapping in - * @param[out] buf returns the virtual address at which this frame has been mapped. - * @param[in] bytes the number of bytes to map. - * @param[in] frame the frame capability to be mapped - * @param[in] flags The flags that are to be set for the newly mapped region, - * see 'paging_flags_t' in paging_types.h . + * @param[in] st the paging state to create the mapping in + * @param[in] vaddr the virtual address to create the mapping at + * @param[in] frame the frame to map in + * @param[in] bytes the number of bytes that will be mapped. + * @param[in] flags The flags that are to be set for the newly mapped region, + * see 'paging_flags_t' in paging_types.h . * - * @return Either SYS_ERR_OK if no error occured or an error indicating what went wrong otherwise. + * @return SYS_ERR_OK on success. */ -errval_t paging_map_frame_attr(struct paging_state *st, void **buf, size_t bytes, - struct capref frame, int flags) -{ - errval_t err; - - thread_mutex_lock_nested(&st->lock); - err = _paging_map_frame_attr(st, buf, bytes, frame, flags); - thread_mutex_unlock(&st->lock); - - return err; -} - -static errval_t _paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, +errval_t paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, struct capref frame, size_t bytes, int flags) { errval_t err; @@ -758,14 +818,16 @@ static errval_t _paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, // make sure we have enough slot and slab space left err = pt_ensure_slabs(st); if(err_is_fail(err)) { - return err_push(err, LIB_ERR_SLAB_REFILL); + return err; } // get the l0 page table and make sure late init is completed + PAGING_LOCK struct pt_t *l0_pt = &st->l0_pt; if (l0_pt->children == NULL) { l0_pt->children = (struct pt_t **) slab_alloc(&st->pt_children_slabs); if(l0_pt->children == NULL) { + PAGING_UNLOCK return LIB_ERR_SLAB_ALLOC_FAIL; } memset(l0_pt->children, 0, st->pt_children_slabs.blocksize); @@ -782,15 +844,18 @@ static errval_t _paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, // make sure the virtual memory is not used by anyone else err = paging_insert_vaddr_reg(st, vaddr_reg, vaddr - vaddr_reg->base, bytes, false); if (err_is_fail(err)) { + PAGING_UNLOCK return err; } } else if (!(vaddr_reg->base == vaddr && end_vaddr == vaddr_reg->base + vaddr_reg->size) && !vaddr_reg->heap) { + PAGING_UNLOCK return LIB_ERR_PMAP_ADDR_NOT_FREE; } break; } } + PAGING_UNLOCK if (vaddr_reg == NULL) { return LIB_ERR_PMAP_ADDR_NOT_FREE; } @@ -837,11 +902,13 @@ static errval_t _paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, // make sure we have enough slot and slab space left err = pt_ensure_slabs(st); if(err_is_fail(err)) { - return err_push(err, LIB_ERR_SLAB_REFILL); + return err; } // create structures for the new metadata + PAGING_LOCK struct pt_t *pt_entry = (struct pt_t *) slab_alloc(&st->pt_slabs); + PAGING_UNLOCK if (pt_entry == NULL) { DEBUG_ERR(err, "Failed to refill slabs before adding page mapping."); return LIB_ERR_SLAB_ALLOC_FAIL; @@ -852,17 +919,24 @@ static errval_t _paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, // allocate the new mapping err = st->slot_alloc->alloc(st->slot_alloc, &pt_entry->cap_mapping); if (err_is_fail(err)) { + PAGING_LOCK slab_free(&st->pt_slabs, pt_entry); + PAGING_UNLOCK return err; } // debug_printf("DEBUG rueegges: paging_map_fixed_attr - add new mapping\n"); // create the new mapping // debug_printf("DEBUG rueegges: vnode_map(ll_pt, frame, %u, %d, %lu, %lu, cap_mapping)\n", l3_index, flags, 0, mapping_size); + PAGING_LOCK + if (l3_pt->children[l3_index] != NULL) { + return LIB_ERR_PMAP_EXISTING_MAPPING; + } err = vnode_map(l3_pt->cap_pt, frame, l3_index, flags, current_vaddr - vaddr, mapping_size, pt_entry->cap_mapping); if (err_is_fail(err)) { debug_printf("Failed to map vnode at vaddr 0x%lx\n", current_vaddr); slab_free(&st->pt_slabs, pt_entry); + PAGING_UNLOCK errval_t err_err = st->slot_alloc->free(st->slot_alloc, pt_entry->cap_mapping); if (err_is_fail(err_err)) { DEBUG_ERR(err, "Failed to free slot during error handling"); @@ -872,6 +946,7 @@ static errval_t _paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, // add the new page table metadata to the shadow tables l3_pt->children[l3_index] = pt_entry; + PAGING_UNLOCK } // debug_printf("DEBUG rueegges: paging_map_fixed_attr - success\n"); @@ -879,30 +954,6 @@ static errval_t _paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, return SYS_ERR_OK; } -/** - * @brief mapps the provided frame at the supplied address in the paging state - * - * @param[in] st the paging state to create the mapping in - * @param[in] vaddr the virtual address to create the mapping at - * @param[in] frame the frame to map in - * @param[in] bytes the number of bytes that will be mapped. - * @param[in] flags The flags that are to be set for the newly mapped region, - * see 'paging_flags_t' in paging_types.h . - * - * @return SYS_ERR_OK on success. - */ -errval_t paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, - struct capref frame, size_t bytes, int flags) -{ - errval_t err; - - thread_mutex_lock_nested(&st->lock); - err = _paging_map_fixed_attr(st, vaddr, frame, bytes, flags); - thread_mutex_unlock(&st->lock); - - return err; -} - static errval_t _paging_unmap(struct paging_state *st, const void *region) { errval_t err; @@ -949,6 +1000,8 @@ static errval_t _paging_unmap(struct paging_state *st, const void *region) err = cap_delete(pt_entry->cap_mapping); if (err_is_fail(err)) return err_push(err, LIB_ERR_VNODE_UNMAP); + // NOTE this calls slab_alloc but it is fine to do while holding paging lock since the + // single slot allocator slab allocator does not auto refill during free err = st->slot_alloc->free(st->slot_alloc, pt_entry->cap_mapping); if (err_is_fail(err)) return err_push(err, LIB_ERR_SLOT_FREE); @@ -990,9 +1043,9 @@ errval_t paging_unmap(struct paging_state *st, const void *region) { errval_t err; - thread_mutex_lock_nested(&st->lock); + PAGING_LOCK err = _paging_unmap(st, region); - thread_mutex_unlock(&st->lock); + PAGING_UNLOCK return err; }