lkml.org 
[lkml]   [2014]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 1/3] perf/sdt : Listing of SDT markers by perf
    Hi Namhyung,

    On 07/21/2014 08:31 AM, Namhyung Kim wrote:
    > Hi Hemant,
    >
    > On Thu, 17 Jul 2014 11:25:12 +0530, Hemant Kumar wrote:
    >> This patch enables perf to list the SDT markers present in a system. It looks
    >> in dsos given by ldconfig --print-cache and for other binaries, it looks into
    >> the PATH environment variable. After preparing a list of the binaries, then
    >> it starts searching for SDT markers in them.
    >> To find the SDT markers, first an elf section named .note.stapsdt is searched
    >> for. And then the SDT notes are retreived one by one from that section.
    >> To counter the effect of prelinking, the section ".stapsdt.base" is searched.
    >> If its found, then the location of the SDT marker is adjusted.
    >>
    >> All these markers' info is written into a cache file
    >> "/var/cache/perf/perf-sdt.cache".
    >> Since, the presence of SDT markers is quite common these days, hence, its better
    >> to make them visible to a user easily. Also, creating a cache file will help a user
    >> to probe (to be implemented) these markers without much hussle. This cache file will
    >> hold most of the SDT markers.
    >>
    >> The format of each SDT cache entry is -
    >>
    >> %provider:marker:file_path:build_id:location:semaphore_loc
    >>
    >> % - marks the beginning of each entry.
    >> provider - The provider name of the SDT marker.
    >> marker - The marker name of the SDT marker.
    >> file_path - Full/absolute path of the file in which this marker is present.
    >> location : Adjusted location of the SDT marker inside the program.
    >> semaphore_loc : The semaphore address if present otherwise 0x0.
    >>
    >> This format should help when probing will be implemented. The adjusted address
    >> from the required entry can be directly used in probing if the build_id matches.
    >>
    >> To scan the system for SDT markers invoke :
    >> # perf list sdt --scan
    >>
    >> "--scan" should be used for the first time and whenever there is any change in
    >> the files containing the SDT markers. It looks into the common binaries available
    >> in a system.
    >>
    >> And then use :
    >> # perf list sdt
    >>
    >> This displays the list of SDT markers recorded in the SDT cache.
    >> This shows the SDT markers present in the common binaries stored in the system,
    >> present in PATH variable and the /lib/ and /lib64/ directories.
    >>
    >> Or use:
    >> # perf list
    >>
    >> It should display all events including the SDT events.
    >>
    >> Signed-off-by : hemant@linux.vnet.ibm.com
    > Missing your real name in the sob line. (other patchse too)

    Oops. Will add that!

    >
    >> ---
    >> tools/perf/Makefile.perf | 1
    >> tools/perf/builtin-list.c | 6
    >> tools/perf/util/parse-events.h | 3
    >> tools/perf/util/sdt.c | 503 ++++++++++++++++++++++++++++++++++++++++
    >> tools/perf/util/symbol-elf.c | 229 ++++++++++++++++++
    >> tools/perf/util/symbol.h | 19 ++
    >> 6 files changed, 760 insertions(+), 1 deletion(-)
    >> create mode 100644 tools/perf/util/sdt.c
    >>
    >> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
    >> index 9670a16..e098dcd 100644
    >> --- a/tools/perf/Makefile.perf
    >> +++ b/tools/perf/Makefile.perf
    >> @@ -373,6 +373,7 @@ LIB_OBJS += $(OUTPUT)util/stat.o
    >> LIB_OBJS += $(OUTPUT)util/record.o
    >> LIB_OBJS += $(OUTPUT)util/srcline.o
    >> LIB_OBJS += $(OUTPUT)util/data.o
    >> +LIB_OBJS += $(OUTPUT)util/sdt.o
    >>
    >> LIB_OBJS += $(OUTPUT)ui/setup.o
    >> LIB_OBJS += $(OUTPUT)ui/helpline.o
    >> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
    >> index 011195e..e1e654b 100644
    >> --- a/tools/perf/builtin-list.c
    >> +++ b/tools/perf/builtin-list.c
    >> @@ -23,7 +23,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
    >> OPT_END()
    >> };
    >> const char * const list_usage[] = {
    >> - "perf list [hw|sw|cache|tracepoint|pmu|event_glob]",
    >> + "perf list [hw|sw|cache|tracepoint|pmu|event_glob|sdt]",
    > Hmm.. I think it'd be better like below
    >
    > "perf list [hw|sw|cache|tracepoint|pmu|sdt|<event_glob>]",
    >

    Ok, looks good.

    >> NULL
    >> };
    >>
    >> @@ -34,6 +34,8 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
    >>
    >> if (argc == 0) {
    >> print_events(NULL, false);
    >> + printf("\n\nSDT events :\n");
    >> + sdt_cache__display();
    > What about make print_events() to print SDT events also?

    Hmm, alright.

    >
    >> return 0;
    >> }
    >>
    >> @@ -55,6 +57,8 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
    >> print_pmu_events(NULL, false);
    >> else if (strcmp(argv[i], "--raw-dump") == 0)
    >> print_events(NULL, true);
    >> + else if (strcmp(argv[i], "sdt") == 0)
    >> + print_sdt_events(argv[++i]);
    > Please move this above the --raw-dump block, it's a special marker to
    > help shell completion for event names so I'd like to keep it last.

    Oh! will move that.

    >
    >> else {
    >> char *sep = strchr(argv[i], ':'), *s;
    >> int sep_idx;
    >
    > [SNIP]
    >> +/*
    >> + * Finds out the libraries present in a system as shown by the command
    >> + * "ldconfig --print-cache". Uses "=>" and '/' to find out the start of a
    >> + * dso path.
    >> + */
    > If we received the list of libraries from user directly, it can go
    > away IMHO.
    >

    I may be wrong but I think until we close the pipe, the data will stay,
    right?

    >> +static char *parse_lib_name(char *str)
    >> +{
    >> + char *ptr, *q, *r;
    >> +
    >> + while (str != NULL) {
    >> + /* look for "=>" and then '/' */
    >> + ptr = strstr(str, "=>");
    >> + if (ptr) {
    >> + ptr++;
    >> + q = strchr(ptr, '/');
    >> + if (!q)
    >> + return NULL;
    >> + r = strchr(ptr, '\n');
    >> + *r = '\0';
    >> + return q;
    >> + } else if (ptr == NULL) {
    >> + return NULL;
    >> + } else {
    >> + str = ptr + 1;
    >> + continue;
    >> + }
    >> + }
    >> +
    >> + return NULL;
    >> +}
    >> +
    >> +/*
    >> + * Checks if a path is already present in the list.
    >> + * Returns 'true' if present and 'false' otherwise.
    >> + */
    >> +static bool is_present_in_list(struct list_head *path_list, char *path)
    >> +{
    >> + struct path_list *tmp;
    >> +
    >> + list_for_each_entry(tmp, path_list, list) {
    >> + if (!strcmp(path, tmp->path))
    >> + return true;
    >> + }
    > As Andi pointed out, you can use a hashtable for this.

    Ok, will use a hashtable for this.

    >
    >> +
    >> + return false;
    >> +}
    >> +
    >> +static inline void append_path(char *path, struct list_head *list)
    >> +{
    >> + char *res_path = NULL;
    >> + struct path_list *tmp = NULL;
    >> +
    >> + res_path = (char *)malloc(sizeof(char) * PATH_MAX);
    >> +
    >> + if (!res_path)
    >> + return;
    >> +
    >> + memset(res_path, '\0', PATH_MAX);
    >> +
    >> + if (realpath(path, res_path) && !is_present_in_list(list, res_path)) {
    >> + tmp = (struct path_list *) malloc(sizeof(struct path_list));
    > Hmm... why not reusing res_path and make struct path_list to just have a
    > pointer? Also you can use readpath(path, NULL) rather than allocating
    > a PATH_MAX buffer and zero'ing it (can use calloc for it anyway).
    >

    Ah! ok, will make the changes.

    >> + if (!tmp) {
    >> + free(res_path);
    >> + return;
    >> + }
    >> + strcpy(tmp->path, res_path);
    >> + list_add(&(tmp->list), list);
    >> + if (res_path)
    >> + free(res_path);
    >> + }
    >> +}
    >
    > [SNIP]
    >> +/*
    >> + * Go through all the files inside "path".
    >> + * But don't go into sub-directories.
    >> + */
    >> +static void walk_through_dir(char *path)
    >> +{
    >> + struct dirent *entry;
    >> + DIR *dir;
    >> + struct stat sb;
    >> + int ret = 0;
    >> + char *swd;
    >> +
    >> + dir = opendir(path);
    >> + if (!dir)
    >> + return;
    >> +
    >> + /* save the current working directory */
    >> + swd = getcwd(NULL, 0);
    >> + if (!swd) {
    >> + pr_err("getcwd : error");
    >> + return;
    >> + }
    >> +
    >> + ret = chdir(path);
    >> + if (ret) {
    >> + pr_err("chdir : error in %s", path);
    >> + return;
    >> + }
    > Is this really needed? I guess opendir() already covers possible
    > failure cases, if not, we might use stat() anyway.
    >
    >
    >> + while ((entry = readdir(dir)) != NULL) {
    >> +
    >> + ret = stat(entry->d_name, &sb);
    >> + if (ret == -1) {
    >> + pr_debug("%s : error in stat!\n", entry->d_name);
    >> + continue;
    >> + }
    >> +
    >> + /* Not pursuing sub-directories */
    >> + if ((sb.st_mode & S_IFMT) != S_IFDIR)
    >> + if (sb.st_mode & S_IXUSR)
    >> + append_path(entry->d_name, &execs.list);
    >> + }
    >> +
    >> + closedir(dir);
    >> +
    >> + /* return to the saved working directory */
    >> + ret = chdir(swd);
    >> + if (ret) {
    >> + pr_err("chdir : error");
    >> + return;
    >> + }
    >> +}
    >
    > [SNIP]
    >> --- a/tools/perf/util/symbol-elf.c
    >> +++ b/tools/perf/util/symbol-elf.c
    >> @@ -1619,6 +1619,235 @@ void kcore_extract__delete(struct kcore_extract *kce)
    >> unlink(kce->extract_filename);
    >> }
    >>
    > Like I said earlier in a different thread, the below code can be a
    > sepatate change.

    Ok, will move that to a separate patch.

    Thanks a lot for the review!

    >
    >
    >> +static int populate_sdt_note(Elf **elf, const char *data, size_t len, int type,
    >> + struct sdt_note **note)
    >> +{
    >> + const char *provider, *name;
    >> + struct sdt_note *tmp = NULL;
    >> + GElf_Ehdr ehdr;
    >> + GElf_Addr base_off = 0;
    >> [SNIP]

    --
    Thanks,
    Hemant Kumar



    \
     
     \ /
      Last update: 2014-07-22 14:21    [W:3.369 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site