From c6881d0eea7af76dd584e5ed73bc7a5692e95c0e Mon Sep 17 00:00:00 2001 From: Song Liu Date: Wed, 13 Oct 2021 00:10:20 -0700 Subject: [PATCH 1/4] create-diff-object: compare section name with kpatch_section_function_name Profile-Guided Optimization (PGO) uses profiling data to help the compiler to evaluate the properties of a function, and thus adds different prefixes to the section/function. For example, with -ffunction-sections, the compiler will prefix some sectiones with .text.unlikely. However, if a function changes from the version in the profiling data, the compiler may ignore the profiling data. This often happens to the patched function when kpatch-build builds the patch. As a result, the original function and the patch function may have different prefix, i.e., one of them has .text, the other has .text.unlikely. To correlate these functions properly, use kpatch_section_function_name() in kpatch_correlate_sections() to find matching sections. Signed-off-by: Song Liu --- kpatch-build/create-diff-object.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 1967bd804..6f08c4d1e 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -986,6 +986,8 @@ static void kpatch_correlate_section(struct section *sec_orig, kpatch_correlate_symbol(sec_orig->sym, sec_patched->sym); } +static char *kpatch_section_function_name(struct section *sec); + static void kpatch_correlate_sections(struct list_head *seclist_orig, struct list_head *seclist_patched) { @@ -995,8 +997,9 @@ static void kpatch_correlate_sections(struct list_head *seclist_orig, if (sec_orig->twin) continue; list_for_each_entry(sec_patched, seclist_patched, list) { - if (kpatch_mangled_strcmp(sec_orig->name, sec_patched->name) || - sec_patched->twin) + if (sec_patched->twin || + kpatch_mangled_strcmp(kpatch_section_function_name(sec_orig), + kpatch_section_function_name(sec_patched))) continue; if (is_special_static(is_rela_section(sec_orig) ? From fc7c52f237e33eb9ecc669a0575e00c8c3864390 Mon Sep 17 00:00:00 2001 From: Song Liu Date: Wed, 13 Oct 2021 01:03:55 -0700 Subject: [PATCH 2/4] kpatch-build: add support for clang pgo For clang with Profile-Guided Optimization (PGO), profile data is needed to compile the livepatch properly. Add option -p|--profile-data, which specifies the profile data file. This option is only valid with CONFIG_CC_IS_CLANG and CONFIG_PGO_CLANG. Signed-off-by: Song Liu --- kpatch-build/kpatch-build | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/kpatch-build/kpatch-build b/kpatch-build/kpatch-build index bec80102c..b9325b172 100755 --- a/kpatch-build/kpatch-build +++ b/kpatch-build/kpatch-build @@ -677,9 +677,10 @@ usage() { echo " --skip-cleanup Skip post-build cleanup" >&2 echo " --skip-compiler-check Skip compiler version matching check" >&2 echo " (not recommended)" >&2 + echo " -p, --profile-data specify profile data for PGO (clang only)" >&2 } -options="$(getopt -o ha:r:s:c:v:j:t:n:o:dR -l "help,archversion:,sourcerpm:,sourcedir:,config:,vmlinux:,jobs:,target:,name:,output:,oot-module:,oot-module-src:,debug,skip-gcc-check,skip-compiler-check,skip-cleanup,non-replace" -- "$@")" || die "getopt failed" +options="$(getopt -o ha:r:s:c:v:j:t:n:o:dRp: -l "help,archversion:,sourcerpm:,sourcedir:,config:,vmlinux:,jobs:,target:,name:,output:,oot-module:,oot-module-src:,debug,skip-gcc-check,skip-compiler-check,skip-cleanup,non-replace,profile-data" -- "$@")" || die "getopt failed" eval set -- "$options" @@ -761,6 +762,10 @@ while [[ $# -gt 0 ]]; do echo "WARNING: Skipping compiler version matching check (not recommended)" SKIPCOMPILERCHECK=1 ;; + -p|--profile-data) + PROFILE_DATA="$(readlink -f "$2")" + shift + ;; *) [[ "$1" = "--" ]] && shift && continue [[ ! -f "$1" ]] && die "patch file '$1' not found" @@ -1056,6 +1061,16 @@ fi if [[ -n "$CONFIG_CC_IS_CLANG" ]]; then echo "WARNING: Clang support is experimental" + if [[ -z "$PROFILE_DATA" ]] && [[ -n "$CONFIG_PGO_CLANG" ]]; then + die "Please specify profile-data for CONFIG_PGO_CLANG" + fi + if [[ -n "$PROFILE_DATA" ]] && [[ -z "$CONFIG_PGO_CLANG" ]]; then + echo "WARNING profile-data specified w/o CONFIG_PGO_CLANG, ignore it" + fi +else + if [[ -n "$PROFILE_DATA" ]]; then + die "Only supports profile-data with Clang" + fi fi if [[ "$SKIPCOMPILERCHECK" -eq 0 ]]; then @@ -1107,6 +1122,9 @@ declare -a MAKEVARS if [[ -n "$CONFIG_CC_IS_CLANG" ]]; then MAKEVARS+=("CC=${KPATCH_CC_PREFIX}${CLANG}") MAKEVARS+=("HOSTCC=clang") + if [[ -n "$CONFIG_PGO_CLANG" ]]; then + MAKEVARS+=("LLVM=1 CFLAGS_PGO_CLANG=-fprofile-use=$PROFILE_DATA") + fi else MAKEVARS+=("CC=${KPATCH_CC_PREFIX}${GCC}") fi From b7fa713c9b3e79c1a16d2176fc3b2be79d51d733 Mon Sep 17 00:00:00 2001 From: Song Liu Date: Mon, 17 Oct 2022 15:11:50 -0700 Subject: [PATCH 3/4] kpatch-build: ignore changed __llvm_prf_cnts section clang GPO uses __llvm_prf_cnts session for performance counters. Changes in these sections could be counted as changed function. Ignore them to avoid these unnecessary changes. Signed-off-by: Song Liu --- kpatch-build/create-diff-object.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 6f08c4d1e..4e63d159c 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -454,6 +454,14 @@ static bool rela_equal(struct rela *rela1, struct rela *rela2) !strcmp(rela2->sym->name, ".altinstr_aux")) return true; + /* + * __llvm_prf_cnts is used by clang PGO to store counters. Ignore + * these to void unnecessary "changed function". + */ + if (!strcmp(rela1->sym->name, "__llvm_prf_cnts") && + !strcmp(rela2->sym->name, "__llvm_prf_cnts")) + return true; + /* * With -mcmodel=large on ppc64le, GCC might generate entries in the .toc * section for relocation symbol references. The .toc offsets may change From db68a17e90f4c67e68e9ee0296cd348ce2d373a6 Mon Sep 17 00:00:00 2001 From: Song Liu Date: Mon, 17 Oct 2022 15:14:42 -0700 Subject: [PATCH 4/4] kpatch-build: let locals_match to match 90% of local symbols With clang PGO is used, the compiler uses function signature to match functions to profile data. When the signature of a function changed as part of livepatch, the profile data is ignore. This may cause the compiler to make different inline decisions, and thus cause mismatch in local functions. To adapt to this case, let locals_match to count the number of match and mismatch symbols. If 90% of the symbols match, we consider the two files are the same. Signed-off-by: Song Liu --- kpatch-build/lookup.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/kpatch-build/lookup.c b/kpatch-build/lookup.c index f2596b15f..bf23c329f 100644 --- a/kpatch-build/lookup.c +++ b/kpatch-build/lookup.c @@ -91,11 +91,13 @@ static bool maybe_discarded_sym(const char *name) } static bool locals_match(struct lookup_table *table, int idx, - struct symbol *file_sym, struct list_head *sym_list) + struct symbol *file_sym, struct list_head *sym_list, + int match_pct) { struct symbol *sym; struct object_symbol *table_sym; int i, found; + int match_cnt = 0, mismatch_cnt = 0; i = idx + 1; for_each_obj_symbol_continue(i, table_sym, table) { @@ -121,10 +123,18 @@ static bool locals_match(struct lookup_table *table, int idx, } } - if (!found) - return false; + if (found) + match_cnt++; + else + mismatch_cnt++; } + if (match_cnt * 100 < (match_cnt + mismatch_cnt) * match_pct) + return false; + + match_cnt = 0; + mismatch_cnt = 0; + sym = file_sym; list_for_each_entry_continue(sym, sym_list, list) { if (sym->type == STT_FILE) @@ -157,11 +167,13 @@ static bool locals_match(struct lookup_table *table, int idx, } } - if (!found) - return false; + if (found) + match_cnt++; + else + mismatch_cnt++; } - return true; + return match_cnt * 100 >= (match_cnt + mismatch_cnt) * match_pct; } static void find_local_syms(struct lookup_table *table, struct symbol *file_sym, @@ -176,7 +188,7 @@ static void find_local_syms(struct lookup_table *table, struct symbol *file_sym, continue; if (strcmp(file_sym->name, sym->name)) continue; - if (!locals_match(table, i, file_sym, sym_list)) + if (!locals_match(table, i, file_sym, sym_list, 90 /* match_pct */)) continue; if (lookup_table_file_sym) ERROR("found duplicate matches for %s local symbols in %s symbol table",