Re: [PATCH] operf: prefer _stext over _text as the kernel address range start

On 03/31/2016 01:10 PM, Michael Petlan wrote:
> In newer kernels, _stext is the right pseudo-symbol for marking the
> start address of the kernel address range. The older _text symbol is
> rather architecture specific.

Hi Michaelm

There are kernel patches that are moving to using _stext, so operf preferring _stext makes sense.  Could you add some info on which particular situation not having this patch presented a problem?  Something that would allow testing to make sure that the patch works as expected.

How was the patch generated? "git am" didn't accept it when trying to apply it to the a branch of oprofile git repo.  Could you resent a patch that applies?

-Will
> 
> With this patch, the _stext symbol is preferred (if found). In case
> _stext is not there (an older kernel), _text is accepted like before.
> ---
>  pe_profiling/operf.cpp | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/pe_profiling/operf.cpp b/pe_profiling/operf.cpp
> index 5585b34..06a0ea3 100644
> --- a/pe_profiling/operf.cpp
> +++ b/pe_profiling/operf.cpp
> @@ -87,8 +87,9 @@ bool track_new_forks;
> 
> 
>  #define DEFAULT_OPERF_OUTFILE "operf.data"
> -#define KERN_ADDR_SPACE_START_SYMBOL  "_text"
> +#define KERN_ADDR_SPACE_START_SYMBOL  "_stext"
>  #define KERN_ADDR_SPACE_END_SYMBOL    "_etext"
> +#define KERN_ADDR_SPACE_START_SYMBOL_OBSOLETE  "_text"
> 
>  static operf_record * operfRecord = NULL;
>  static char * app_name_SAVE = NULL;
> @@ -1141,6 +1142,22 @@ static bool _process_kallsyms(void)
>          iss >> type;
>          iss >> name;
> 
> +        /* accept _text as the start symbol only in case there is no _stext
> +         * because _stext has higher priority
> +        */
> +        if (start_addr_str.empty()) {
> +            if (strncmp(name.c_str(), KERN_ADDR_SPACE_START_SYMBOL_OBSOLETE,
> +                    strlen(name.c_str())) == 0) {
> +                /* found the symbol for the start of the kernel
> +                 * address space.
> +                */
> +                start_addr_str.assign(address_str);
> +            }
> +        }
> +
> +        /* if _stext is found, it will overwrite the _text if it has been
> +         * already found
> +        */

This logic seems a okay. 
 
>          if (strncmp(name.c_str(), KERN_ADDR_SPACE_START_SYMBOL,
>                  strlen(name.c_str())) == 0) {
>              /* found the symbol for the start of the kernel


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140