(2015/03/11 22:30), Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 04, 2015 at 09:34:38PM +0900, Masami Hiramatsu escreveu:
>> (2015/03/04 16:52), Naohiro Aota wrote:
>>> If we use lazy matching, it failed to open a souce file if perf command
>>> is invoked outside of compilation directory:
>>>
>>> $ perf probe -a '__schedule;clear_*'
>>> Failed to open kernel/sched/core.c: No such file or directory
>>>   Error: Failed to add events. (-2)
>>>
>>> OTOH, other commands like "probe -L" can solve the souce directory by
>>> themselves. Let's make it possible for lazy matching too!
>>>
>>
>> Looks good to me :)
>>
>> Acked-by: Masami Hiramatsu <[email protected]>
> 
> This doesn't make sense... se below:
>  
>> Thank you!
>>
>>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>>> index 46f009a..e6c0262 100644
>>> --- a/tools/perf/util/probe-finder.c
>>> +++ b/tools/perf/util/probe-finder.c
>>> @@ -849,11 +849,22 @@ static int probe_point_lazy_walker(const char *fname, 
>>> int lineno,
>>>  static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder 
>>> *pf)
>>>  {
>>>     int ret = 0;
>>> +   char *fpath;
>>>  
>>>     if (intlist__empty(pf->lcache)) {
>>> +           const char *comp_dir;
>>> +
>>> +           comp_dir = cu_get_comp_dir(&pf->cu_die);
>>> +           ret = get_real_path(pf->fname, comp_dir, &fpath);
>>> +           if (ret < 0) {
>>> +                   pr_warning("Failed to find source file path.\n");
>>> +                   return ret;
>>> +           }
>>> +
>>>             /* Matching lazy line pattern */
>>> -           ret = find_lazy_match_lines(pf->lcache, pf->fname,
>>> +           ret = find_lazy_match_lines(pf->lcache, fpath,
>>>                                         pf->pev->point.lazy_line);
>>> +           free(fpath);
>>>             if (ret <= 0)
>>>                     return ret;
>>>     }
>>> @@ -1616,3 +1627,61 @@ found:
>>>     return (ret < 0) ? ret : lf.found;
>>>  }
>>>  
>>> +/*
>>> + * Find a src file from a DWARF tag path. Prepend optional source path 
>>> prefix
>>> + * and chop off leading directories that do not exist. Result is passed 
>>> back as
>>> + * a newly allocated path on success.
>>> + * Return 0 if file was found and readable, -errno otherwise.
>>> + */
>>> +static int get_real_path(const char *raw_path, const char *comp_dir,
>>> +                    char **new_path)
> 
> The function is marked "static"
> 
>>> +{
>>> +   const char *prefix = symbol_conf.source_prefix;
>>> +
>>> +   if (!prefix) {
>>> +           if (raw_path[0] != '/' && comp_dir)
>>> +                   /* If not an absolute path, try to use comp_dir */
>>> +                   prefix = comp_dir;
>>> +           else {
>>> +                   if (access(raw_path, R_OK) == 0) {
>>> +                           *new_path = strdup(raw_path);
>>> +                           return *new_path ? 0 : -ENOMEM;
>>> +                   } else
>>> +                           return -errno;
>>> +           }
>>> +   }
>>> +
>>> +   *new_path = malloc((strlen(prefix) + strlen(raw_path) + 2));
>>> +   if (!*new_path)
>>> +           return -ENOMEM;
>>> +
>>> +   for (;;) {
>>> +           sprintf(*new_path, "%s/%s", prefix, raw_path);
>>> +
>>> +           if (access(*new_path, R_OK) == 0)
>>> +                   return 0;
>>> +
>>> +           if (!symbol_conf.source_prefix) {
>>> +                   /* In case of searching comp_dir, don't retry */
>>> +                   zfree(new_path);
>>> +                   return -errno;
>>> +           }
>>> +
>>> +           switch (errno) {
>>> +           case ENAMETOOLONG:
>>> +           case ENOENT:
>>> +           case EROFS:
>>> +           case EFAULT:
>>> +                   raw_path = strchr(++raw_path, '/');
>>> +                   if (!raw_path) {
>>> +                           zfree(new_path);
>>> +                           return -ENOENT;
>>> +                   }
>>> +                   continue;
>>> +
>>> +           default:
>>> +                   zfree(new_path);
>>> +                   return -errno;
>>> +           }
>>> +   }
>>> +}
>>> diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
>>> index 92590b2..5ef82dd 100644
>>> --- a/tools/perf/util/probe-finder.h
>>> +++ b/tools/perf/util/probe-finder.h
>>> @@ -55,6 +55,10 @@ extern int debuginfo__find_available_vars_at(struct 
>>> debuginfo *dbg,
>>>                                          struct variable_list **vls,
>>>                                          int max_points, bool externs);
>>>  
>>> +/* Find a src file from a DWARF tag path */
>>> +extern int get_real_path(const char *raw_path, const char *comp_dir,
>>> +                    char **new_path);
>>> +
> 
> And then you mark it "extern"? Have you tried to compile this? I tried:
>  
>   CC       /tmp/build/perf/tests/task-exit.o
> util/probe-finder.c:1636:12: error: static declaration of ‘get_real_path’ 
> follows non-static declaration
>  static int get_real_path(const char *raw_path, const char *comp_dir,
>             ^
> In file included from util/probe-finder.c:41:0:
> util/probe-finder.h:59:12: note: previous declaration of ‘get_real_path’ was 
> here
>  extern int get_real_path(const char *raw_path, const char *comp_dir,
>             ^
> util/probe-finder.c:1636:12: error: ‘get_real_path’ defined but not used 
> [-Werror=unused-function]
>  static int get_real_path(const char *raw_path, const char *comp_dir,
>             ^
> cc1: all warnings being treated as errors
> make[3]: *** [/tmp/build/perf/util/probe-finder.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
>   CC       /tmp/build/perf/tests/sw-clock.o
> 
> Also please refrain from using 'extern' in front of function prototypes, its 
> just noise.1

Ooops! Sorry, I missed this ...

Naohiro, could you fix this?

Thanks,

> 
> - Arnaldo
> 
>>>  struct probe_finder {
>>>     struct perf_probe_event *pev;           /* Target probe event */
>>>  
>>>
>>
>>
>> -- 
>> Masami HIRAMATSU
>> Software Platform Research Dept. Linux Technology Research Center
>> Hitachi, Ltd., Yokohama Research Laboratory
>> E-mail: [email protected]
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to