Basic Clang PGO support #1231
Basic Clang PGO support #1231
Conversation
|
Hi, thanks for the pullrequest. It seems it hasn't been merged to upstream kernel yet, so is this work based on "pgo: add clang's Profile Guided Optimization infrastructure" patch? If so I see documentation suggesting to use Also do you know if there is any way to tell if the original kernel was built with the same profile data kpatch-build gets? Maybe clang notes it down somewhere in resulting binaries and we can check it the same way we do with compiler version? Overall I think this looks good, but we'll have to wait until CONFIG_PGO_CLANG gets merged to the kernel before merging this. |
Yes, our version is based on this patch. We use CFLAGS_PGO_CLANG flag here to only pass
I am not aware of such information shipped with the kernel. |
liu-song-6
force-pushed
the
pgo
branch
2 times, most recently
from
d94bd1e to
184bc48
Compare
|
Hi @liu-song-6, I think your force pushes were just rebasing this development branch? Also, on the subject of the original commit msg that "fixes [the] issue that function section name in original and patched kernel has different prefixes (e.g., .text vs. .text.unlikely)"... I noticed similar behavior with gcc on arm64. Do you have any information on why or how the compiler decides that this can occur? And then, it looks like your fix was dropped from this PR. How was it fixed? Were such changes ignored (or always added) if it was determined that only section names changed? Thanks. |
Yes, this is to address some conflict.
In PGO case, the compiler compares the function signature of current version and the profiled version. If the signature changed (due to patch here), the compiler will ignore the profile data, and thus made different likely/unlikely decision. In none PGO case, I guess it is some other change in likely/unlikely decision?
It was a mistake. Thanks for catching it! I will add the fix back. |
|
Hi @liu-song-6 And then did upstream kernel progress on adding clang PGO stall? I see Kees note here: https://lore.kernel.org/lkml/202106140921.5E591BD@keescook/ but is this any closer to merging? Thanks. |
IIUC, if the function doesn't change, the profile data still works and thus the unlikely-or-not decision should be the same. We haven't got much experience on this yet, so there may still be some corner cases.
I am not sure about the upstream progress. We decided to use the latest version of the PGO in one of our release, which will service us for the next year or two. |
…_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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
|
This PR has been open for 60 days with no activity and no assignee. It will be closed in 7 days unless a comment is added. |
|
This PR was closed because it was inactive for 7 days after being marked stale. |
This series adds basic Clang PGO support.
The first commit fixes issue that function section name in original and patched kernel has different prefixes (e.g., .text vs. .text.unlikely). Such behavior is expected with PGO.
The secondd commit adds -p|--profile-data option to kpatch-build.