From e05e655dd41185125d765e6a99eadb58cd95f6eb Mon Sep 17 00:00:00 2001 From: Vedashree Vidwans Date: Mon, 18 Mar 2019 09:48:34 -0700 Subject: [PATCH] gpu: nvgpu: Fix buddy_allocator mutex, logical bugs Bugs in current version are listed below: 1. Function alloc() or alloc_pte() allocate memory for len=0. 2. Function alloc() or alloc_pte() don't unlock nvgpu_allocator if pte_size is invalid. 3. Function alloc_pte() and alloc_fixed() set alloc_made flag unconditionally. 4. Function release_carveout() tries to acquire nvgpu_allocator lock twice causing unresponsive state. 5. If buddy allocation fails in balloc_do_alloc() or balloc_do_alloc_fixed() function, previously allocated buddies are not merged. This causes seg fault in ops->fini(). 6. With gva_space enabled and base=0, buddy_allocator updated base not checked for pde alignment. 7. In balloc_do_alloc_fixed(), align_order computed using __fls() results in one order higher than requested. 8. Initializing buddy allocator with size=0, initializes very large memory and will trigger seg fault with the changes in this patch. Setting size=1G so that further execution is successful. This patch fixes above listed bugs and updates following: 1. With gva_space enabled, BALLOC_PTE_SIZE_ANY is considered as BALLOC_PTE_SIZE_SMALL which allows alloc() to be used. 2. GPU_BALLOC_MAX_ORDER changed to 63U. Condition added to check that max_order is never greater than GPU_BALLOC_MAX_ORDER. 3. BUG() changed to nvgpu_do_assert(). JIRA NVGPU-3005 Change-Id: I20c28e20aa3404976d67f7884b4f8cbd5c908ba7 Signed-off-by: Vedashree Vidwans Reviewed-on: https://git-master.nvidia.com/r/2075646 Reviewed-by: svc-misra-checker GVS: Gerrit_Virtual_Submit Reviewed-by: Alex Waterman Reviewed-by: mobile promotions Tested-by: mobile promotions --- .../common/mm/allocators/buddy_allocator.c | 93 ++++++++++++------- drivers/gpu/nvgpu/include/nvgpu/allocator.h | 2 +- 2 files changed, 61 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/nvgpu/common/mm/allocators/buddy_allocator.c b/drivers/gpu/nvgpu/common/mm/allocators/buddy_allocator.c index d3ba81c70..fbcc6712d 100644 --- a/drivers/gpu/nvgpu/common/mm/allocators/buddy_allocator.c +++ b/drivers/gpu/nvgpu/common/mm/allocators/buddy_allocator.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2018, NVIDIA CORPORATION. All rights reserved. + * Copyright (c) 2016-2019, NVIDIA CORPORATION. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -78,6 +78,9 @@ static u32 nvgpu_balloc_page_size_to_pte_size(struct nvgpu_buddy_allocator *a, return BALLOC_PTE_SIZE_BIG; } else if (page_size == SZ_4K) { return BALLOC_PTE_SIZE_SMALL; + } else if (page_size == BALLOC_PTE_SIZE_ANY) { + /* With gva_space enabled, only 2 types of PTE sizes allowed */ + return BALLOC_PTE_SIZE_SMALL; } else { return BALLOC_PTE_SIZE_INVALID; } @@ -93,17 +96,15 @@ static void balloc_compute_max_order(struct nvgpu_buddy_allocator *a) { u64 true_max_order = ilog2(a->blks); - if (a->max_order == 0U) { - a->max_order = true_max_order; - return; + if (true_max_order > GPU_BALLOC_MAX_ORDER) { + alloc_dbg(balloc_owner(a), + "Oops: Can't manage more than 1 Exabyte memory"); + nvgpu_do_assert(); } - if (a->max_order > true_max_order) { + if ((a->max_order == 0ULL) || (a->max_order > true_max_order)) { a->max_order = true_max_order; } - if (a->max_order > GPU_BALLOC_MAX_ORDER) { - a->max_order = GPU_BALLOC_MAX_ORDER; - } } /* @@ -152,7 +153,7 @@ static void balloc_buddy_list_do_add(struct nvgpu_buddy_allocator *a, alloc_dbg(balloc_owner(a), "Oops: adding added buddy (%llu:0x%llx)", b->order, b->start); - BUG(); + nvgpu_do_assert(); } /* @@ -177,7 +178,7 @@ static void balloc_buddy_list_do_rem(struct nvgpu_buddy_allocator *a, alloc_dbg(balloc_owner(a), "Oops: removing removed buddy (%llu:0x%llx)", b->order, b->start); - BUG(); + nvgpu_do_assert(); } nvgpu_list_del(&b->buddy_entry); @@ -333,19 +334,19 @@ static void nvgpu_buddy_allocator_destroy(struct nvgpu_allocator *na) nvgpu_info(na->g, "Excess buddies!!! (%d: %llu)", i, a->buddy_list_len[i]); - BUG(); + nvgpu_do_assert(); } if (a->buddy_list_split[i] != 0U) { nvgpu_info(na->g, "Excess split nodes!!! (%d: %llu)", i, a->buddy_list_split[i]); - BUG(); + nvgpu_do_assert(); } if (a->buddy_list_alloced[i] != 0U) { nvgpu_info(na->g, "Excess alloced nodes!!! (%d: %llu)", i, a->buddy_list_alloced[i]); - BUG(); + nvgpu_do_assert(); } } @@ -565,6 +566,7 @@ static u64 balloc_do_alloc(struct nvgpu_buddy_allocator *a, while (bud->order != order) { if (balloc_split_buddy(a, bud, pte_size) != 0) { + balloc_coalesce(a, bud); return 0; /* No mem... */ } bud = bud->left; @@ -762,11 +764,11 @@ static u64 balloc_do_alloc_fixed(struct nvgpu_buddy_allocator *a, shifted_base = balloc_base_shift(a, base); if (shifted_base == 0U) { - align_order = __fls(len >> a->blk_shift); + align_order = __ffs(len >> a->blk_shift); } else { align_order = min_t(u64, __ffs(shifted_base >> a->blk_shift), - __fls(len >> a->blk_shift)); + __ffs(len >> a->blk_shift)); } if (align_order > a->max_order) { @@ -822,7 +824,11 @@ err_and_cleanup: balloc_buddy_list_do_rem(a, bud); (void) balloc_free_buddy(a, bud->start); - nvgpu_kmem_cache_free(a->buddy_cache, bud); + balloc_blist_add(a, bud); + /* + * Attemp to defrag the allocation. + */ + balloc_coalesce(a, bud); } return 0; @@ -862,6 +868,11 @@ static u64 nvgpu_buddy_balloc_pte(struct nvgpu_allocator *na, u64 len, u32 pte_size; struct nvgpu_buddy_allocator *a = na->priv; + if (len == 0ULL) { + alloc_dbg(balloc_owner(a), "Alloc fail"); + return 0; + } + alloc_lock(na); order = balloc_get_order(a, len); @@ -874,7 +885,8 @@ static u64 nvgpu_buddy_balloc_pte(struct nvgpu_allocator *na, u64 len, pte_size = nvgpu_balloc_page_size_to_pte_size(a, page_size); if (pte_size == BALLOC_PTE_SIZE_INVALID) { - return 0ULL; + alloc_unlock(na); + return 0; } addr = balloc_do_alloc(a, order, pte_size); @@ -888,12 +900,11 @@ static u64 nvgpu_buddy_balloc_pte(struct nvgpu_allocator *na, u64 len, pte_size == BALLOC_PTE_SIZE_BIG ? "big" : pte_size == BALLOC_PTE_SIZE_SMALL ? "small" : "NA/any"); + a->alloc_made = true; } else { alloc_dbg(balloc_owner(a), "Alloc failed: no mem!"); } - a->alloc_made = true; - alloc_unlock(na); return addr; @@ -989,27 +1000,25 @@ static u64 nvgpu_balloc_fixed_buddy(struct nvgpu_allocator *na, alloc_lock(na); alloc = nvgpu_balloc_fixed_buddy_locked(na, base, len, page_size); - a->alloc_made = true; + + if (alloc != 0ULL) { + a->alloc_made = true; + } + alloc_unlock(na); return alloc; } /* - * Free the passed allocation. + * @a must be locked. */ -static void nvgpu_buddy_bfree(struct nvgpu_allocator *na, u64 addr) +static void nvgpu_buddy_bfree_locked(struct nvgpu_allocator *na, u64 addr) { struct nvgpu_buddy *bud; struct nvgpu_fixed_alloc *falloc; struct nvgpu_buddy_allocator *a = na->priv; - if (addr == 0ULL) { - return; - } - - alloc_lock(na); - /* * First see if this is a fixed alloc. If not fall back to a regular * buddy. @@ -1034,9 +1043,21 @@ static void nvgpu_buddy_bfree(struct nvgpu_allocator *na, u64 addr) balloc_coalesce(a, bud); done: - alloc_unlock(na); alloc_dbg(balloc_owner(a), "Free 0x%llx", addr); - return; +} + +/* + * Free the passed allocation. + */ +static void nvgpu_buddy_bfree(struct nvgpu_allocator *na, u64 addr) +{ + if (addr == 0ULL) { + return; + } + + alloc_lock(na); + nvgpu_buddy_bfree_locked(na, addr); + alloc_unlock(na); } static bool nvgpu_buddy_reserve_is_possible(struct nvgpu_buddy_allocator *a, @@ -1115,7 +1136,7 @@ static void nvgpu_buddy_release_co(struct nvgpu_allocator *na, alloc_lock(na); nvgpu_list_del(&co->co_entry); - nvgpu_free(na, co->base); + nvgpu_buddy_bfree_locked(na, co->base); alloc_unlock(na); } @@ -1309,6 +1330,12 @@ int nvgpu_buddy_allocator_init(struct gk20a *g, struct nvgpu_allocator *na, return -EINVAL; } + /* Needs to be fixed, return -EINVAL*/ + if (size == 0U) { + /* Setting to fixed size 1G to avoid further issues */ + size = 0x40000000; + } + /* If this is to manage a GVA space we need a VM. */ if (is_gva_space && vm == NULL) { return -EINVAL; @@ -1351,9 +1378,9 @@ int nvgpu_buddy_allocator_init(struct gk20a *g, struct nvgpu_allocator *na, * requirement is not necessary. */ if (is_gva_space) { - base_big_page = base & + base_big_page = a->base & ((U64(vm->big_page_size) << U64(10)) - U64(1)); - size_big_page = size & + size_big_page = a->length & ((U64(vm->big_page_size) << U64(10)) - U64(1)); if (vm->big_pages && (base_big_page != 0ULL || size_big_page != 0ULL)) { diff --git a/drivers/gpu/nvgpu/include/nvgpu/allocator.h b/drivers/gpu/nvgpu/include/nvgpu/allocator.h index be7e12fba..27c4ee754 100644 --- a/drivers/gpu/nvgpu/include/nvgpu/allocator.h +++ b/drivers/gpu/nvgpu/include/nvgpu/allocator.h @@ -235,7 +235,7 @@ int nvgpu_lockless_allocator_init(struct gk20a *g, struct nvgpu_allocator *na, const char *name, u64 base, u64 length, u64 blk_size, u64 flags); -#define GPU_BALLOC_MAX_ORDER 31U +#define GPU_BALLOC_MAX_ORDER 63U /* * Allocator APIs.