Messages in this thread Patch in this message | | | From | Jarmo Tiitto <> | Subject | [PATCH v3 1/1] pgo: Fix allocate_node() v2 | Date | Fri, 4 Jun 2021 19:58:20 +0300 |
| |
When clang instrumentation eventually calls allocate_node() the struct llvm_prf_data *p argument tells us from what section we should reserve the vnode: It either points into vmlinux's core __llvm_prf_data section or some loaded module's __llvm_prf_data section.
But since we don't have access to corresponding __llvm_prf_vnds section(s) for any module, the function should return just NULL and ignore any profiling attempts from modules for now.
Signed-off-by: Jarmo Tiitto <jarmo.tiitto@gmail.com> --- Based on Kees and others feedback here is v3 patch that clarifies why the current checks in allocate_node() are flawed. I did fair amount of KGDB time on it.
The commit is based on kees/for-next/clang/features tree, hopefully this is ok. Should I have based it on linux-next instead?
I grep -R'd where the memory_contains() can be found and it is only found in #include <asm-generic/sections.h>
I cross my fingers and await if this is my first accepted patch. :-) --- kernel/pgo/instrument.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/kernel/pgo/instrument.c b/kernel/pgo/instrument.c index 0e07ee1b17d9..c69b4f7ebaad 100644 --- a/kernel/pgo/instrument.c +++ b/kernel/pgo/instrument.c @@ -18,6 +18,7 @@ #define pr_fmt(fmt) "pgo: " fmt +#include <asm-generic/sections.h> #include <linux/bitops.h> #include <linux/kernel.h> #include <linux/export.h> @@ -55,17 +56,21 @@ void prf_unlock(unsigned long flags) static struct llvm_prf_value_node *allocate_node(struct llvm_prf_data *p, u32 index, u64 value) { - if (&__llvm_prf_vnds_start[current_node + 1] >= __llvm_prf_vnds_end) - return NULL; /* Out of nodes */ - - current_node++; - - /* Make sure the node is entirely within the section */ - if (&__llvm_prf_vnds_start[current_node] >= __llvm_prf_vnds_end || - &__llvm_prf_vnds_start[current_node + 1] > __llvm_prf_vnds_end) + const int max_vnds = prf_vnds_count(); + + /* + * Check that p is within vmlinux __llvm_prf_data section. + * If not, don't allocate since we can't handle modules yet. + */ + if (!memory_contains(__llvm_prf_data_start, + __llvm_prf_data_end, p, sizeof(*p))) return NULL; - return &__llvm_prf_vnds_start[current_node]; + if (WARN_ON_ONCE(current_node >= max_vnds)) + return NULL; /* Out of nodes */ + + /* reserve vnode for vmlinux */ + return &__llvm_prf_vnds_start[current_node++]; } /* base-commit: 5d0cda65918279ada060417c5fecb7e86ccb3def -- 2.31.1
| |