Messages in this thread | | | Subject | Re: [PATCH 1/2] perf vendor events: Add stepping in CPUID string for x86 | From | "Liang, Kan" <> | Date | Thu, 15 Nov 2018 10:18:27 -0500 |
| |
On 11/15/2018 9:26 AM, Liang, Kan wrote: > > > On 11/15/2018 8:53 AM, Jiri Olsa wrote: >> On Wed, Nov 14, 2018 at 01:24:15PM -0800, kan.liang@linux.intel.com >> wrote: >> >> SNIP >> >>> diff --git a/tools/perf/arch/x86/util/header.c >>> b/tools/perf/arch/x86/util/header.c >>> index fb0d71afee8b..b428a4b00bf7 100644 >>> --- a/tools/perf/arch/x86/util/header.c >>> +++ b/tools/perf/arch/x86/util/header.c >>> @@ -4,6 +4,7 @@ >>> #include <stdio.h> >>> #include <stdlib.h> >>> #include <string.h> >>> +#include <regex.h> >>> #include "../../util/header.h" >>> @@ -70,9 +71,73 @@ get_cpuid_str(struct perf_pmu *pmu __maybe_unused) >>> { >>> char *buf = malloc(128); >>> - if (buf && __get_cpuid(buf, 128, "%s-%u-%X$") < 0) { >>> + if (buf && __get_cpuid(buf, 128, "%s-%u-%X-%X$") < 0) { >>> free(buf); >>> return NULL; >>> } >>> return buf; >>> } >>> + >>> +/* Full CPUID format for x86 is vendor-family-model-stepping */ >>> +static bool is_full_cpuid(const char *cpuid) >>> +{ >>> + const char *tmp = cpuid; >>> + int count = 0; >>> + >>> + while ((tmp = strchr(tmp, '-')) != NULL) { >>> + count++; >>> + tmp++; >>> + } >>> + >>> + if (count == 3) >>> + return true; >>> + >>> + return false; >>> +} >>> + >>> +int strcmp_cpuid_str(const char *mapcpuid, const char *cpuid) >>> +{ >>> + regex_t re; >>> + regmatch_t pmatch[1]; >>> + int match; >>> + bool full_mapcpuid = is_full_cpuid(mapcpuid); >>> + bool full_cpuid = is_full_cpuid(cpuid); >> >> cpuid will be always full from now right? why do we need to check it? >> > > User may set cpuid by environment string "PERF_CPUID", which may not be > full format. > >> also please move this to arch/x86/util/pmu.c >> so it matches the weak function object > > Sure. >
It looks like both pmu.c and header.c have cpuid related functions. For example, get_cpuid_str() has weak function in pmu.c. It's declaration in header.h. It's x86 specific function in x86/util/header.c get_cpuid() has weak function in header.c. It's declaration in header.h. It's x86 specific function in x86/util/header.c
It looks like most of the cpuid related functions are in header.c/h. I think it may be better to move all the cpuid related functions to header.c/h.
If it's OK for you, I will send a clean up patch later to move the weak functions strcmp_cpuid_str() and get_cpuid_str() to header.c
What do you think?
Thanks, Kan
| |