From 2bebc35bd6475a5612382fe7a698de7c57a5effd Mon Sep 17 00:00:00 2001 From: Sparchatus Date: Wed, 13 Apr 2022 18:32:00 +0000 Subject: [PATCH] Revert "fine grained paging locking to avoid deadlocks" This reverts commit fca2584d89a2660bf0653c853f3815053869071a. --- lib/aos/paging.c | 230 +++++++++++++++++++---------------------------- 1 file changed, 90 insertions(+), 140 deletions(-) diff --git a/lib/aos/paging.c b/lib/aos/paging.c index 65490fc..898cb44 100644 --- a/lib/aos/paging.c +++ b/lib/aos/paging.c @@ -21,8 +21,8 @@ #include #include -#define PT_PT_SLAB_MIN_SPACE 256 -#define PT_CHILDREN_SLAB_MIN_SPACE 256 +#define PT_PT_SLAB_MIN_SPACE 18 +#define PT_CHILDREN_SLAB_MIN_SPACE 12 #define PT_META_MAX_SIZE MAX(sizeof(struct pt_t), sizeof(struct pt_vaddr_reg_t)) #define PT_PT_SLAB_INITIAL_SPACE SLAB_STATIC_SIZE(PT_PT_SLAB_MIN_SPACE, PT_META_MAX_SIZE) @@ -30,24 +30,13 @@ #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) { @@ -125,7 +114,9 @@ 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(); - PAGING_LOCK + // 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"); struct pt_vaddr_reg_t *vaddr_reg = &st->vaddr_head; for (; vaddr_reg != NULL; vaddr_reg = vaddr_reg->next) { @@ -143,10 +134,8 @@ 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); @@ -154,18 +143,14 @@ 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)) { - 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); + USER_PANIC_ERR(err, "Failed to map frame in page fault handler"); } - debug_printf("[pt_exception_handler] done\n"); + // debug_printf("[pt_exception_handler] Releasing paging mutex.\n"); + thread_mutex_unlock(&st->lock); + // debug_printf("[pt_exception_handler] done\n"); } /** @@ -218,12 +203,12 @@ static errval_t pt_alloc_level(struct paging_state *st, struct capref *pt_cap, u } void pt_print_state(struct paging_state *st) { - PAGING_LOCK + thread_mutex_lock_nested(&st->lock); // iterates over all the page table entries and prints them debug_printf("L0\n"); - if(st->l0_pt.children == NULL){ - PAGING_UNLOCK + if(st->l0_pt.children == NULL) { + thread_mutex_unlock(&st->lock); return; } for(size_t i0 = 0; i0 < PTABLE_ENTRIES; ++i0) { @@ -257,7 +242,7 @@ void pt_print_state(struct paging_state *st) { reg = reg->next; } - PAGING_UNLOCK + thread_mutex_unlock(&st->lock); } void * paging_malloc(size_t bytes, size_t * retbytes, size_t guarded_region_size) { @@ -281,33 +266,6 @@ void * paging_malloc(size_t bytes, size_t * retbytes, size_t guarded_region_size // } -// 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 // slab refilling reserve cycles causing paging calls: // - this slab @@ -316,45 +274,39 @@ static errval_t pt_slab_refill_locked(struct paging_state *st, struct slab_alloc 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; - // 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 + err = slab_default_refill(&st->pt_slabs); + 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) { - debug_printf("DEBUG rueegges: pt_ensure_slabs - refilling pt_children_slabs\n"); + // ASSESSMENT M1: show refilling + // debug_printf("DEBUG rueegges: pt_ensure_slabs - refilling pt_children_slabs\n"); + st->refilling = 1; - // 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 + err = slab_refill_pages(&st->pt_children_slabs, LARGE_PAGE_SIZE); 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; } @@ -368,13 +320,10 @@ 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); @@ -384,7 +333,6 @@ 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); @@ -392,10 +340,8 @@ 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; } @@ -403,20 +349,16 @@ 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); } @@ -427,24 +369,20 @@ 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); @@ -459,8 +397,6 @@ 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; } @@ -608,7 +544,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; @@ -703,7 +639,7 @@ static errval_t paging_insert_vaddr_reg( return SYS_ERR_OK; } -errval_t paging_alloc_ext( +static errval_t _paging_alloc_ext( struct paging_state *st, void **buf, size_t bytes, size_t alignment, bool heap, size_t guarded_region_size ) { @@ -728,7 +664,6 @@ 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 @@ -740,22 +675,30 @@ 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, guarded_region_size); 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, size_t guarded_region_size) +{ + errval_t err; + + thread_mutex_lock_nested(&st->lock); + err = _paging_alloc_ext(st, buf, bytes, alignment, heap, 0); + 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'. @@ -772,19 +715,7 @@ errval_t paging_alloc(struct paging_state *st, void **buf, size_t bytes, size_t return paging_alloc_ext(st, buf, bytes, alignment, false, 0); } -/** - * \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, +static errval_t _paging_map_frame_attr(struct paging_state *st, void **buf, size_t bytes, struct capref frame, int flags) { errval_t err; @@ -810,18 +741,30 @@ errval_t paging_map_frame_attr(struct paging_state *st, void **buf, size_t bytes } /** - * @brief mapps the provided frame at the supplied address in the paging state + * \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[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 . + * @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 SYS_ERR_OK on success. + * @return Either SYS_ERR_OK if no error occured or an error indicating what went wrong otherwise. */ -errval_t paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, +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, struct capref frame, size_t bytes, int flags) { errval_t err; @@ -853,12 +796,10 @@ errval_t paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, } // 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); @@ -881,18 +822,15 @@ 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, 0); 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; } @@ -943,9 +881,7 @@ errval_t paging_map_fixed_attr(struct paging_state *st, lvaddr_t vaddr, } // 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; @@ -956,24 +892,17 @@ 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"); @@ -983,7 +912,6 @@ 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"); @@ -991,6 +919,30 @@ 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; @@ -1037,8 +989,6 @@ 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); @@ -1080,9 +1030,9 @@ errval_t paging_unmap(struct paging_state *st, const void *region) { errval_t err; - PAGING_LOCK + thread_mutex_lock_nested(&st->lock); err = _paging_unmap(st, region); - PAGING_UNLOCK + thread_mutex_unlock(&st->lock); return err; }