perf-buildid-cache: Enhance --update and add --purge
v5 cover perf-buildid-cache: Enhance --update and add --purge
Hi,
Here is the 5th version of of perf buildid-cache update. This updates the 2nd patch and add 2 patches just for cleanup and improve usability a bit.
Here are the changes in v5.
- [2/4] Remove NULL check before calling strlist__delete() (Thanks to Hemant!)
- [3/4] Use pr_debug instead of verbose && pr_info (Thanks to Namhyung!)
- [4/4] Show usage with incorrect params
Thank you,
---
Masami Hiramatsu (4):
perf buildid-cache: Add new buildid cache if update target is not cached perf buildid-cache: Add --purge FILE to remove all caches of FILE
perf-buildid-cache: Use pr_debug instead of verbose && pr_info
perf-buildid-cache: Show usage with incorrect params
tools/perf/Documentation/perf-buildid-cache.txt | 24 ++++-- tools/perf/builtin-buildid-cache.c | 69 ++++++++++++++-- tools/perf/util/build-id.c | 97 +++++++++++++++++++---- tools/perf/util/build-id.h | 2 4 files changed, 157 insertions(+), 35 deletions(-)
--
Masami HIRAMATSU
Software Platform Research Dpt. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
PATCH v5 1/4 perf buildid-cache: Add new buildid cache if update target is not cached
Add new buildid cache if the update target file is not cached. This can happen when an old binary is replaced by new one after caching the old one. In this case, user sees his operation just failed. But it does not look straight, since user just pass the binary "path", not "build-id".
----
# ./perf buildid-cache --add ./perf (update ./perf to new binary) # ./perf buildid-cache --update ./perf ./perf wasn't in the cache # ----
This patch adds given new binary to cache if the new binary is not cached. So we'll not see the above error.
----
# ./perf buildid-cache --add ./perf (update ./perf to new binary) # ./perf buildid-cache --update ./perf # ----
--- tools/perf/Documentation/perf-buildid-cache.txt | 11 ++++++++--- tools/perf/builtin-buildid-cache.c | 6 ++++-- tools/perf/util/build-id.c | 12 ++++++++++++ tools/perf/util/build-id.h | 1 + 4 files changed, 25 insertions(+), 5 deletions(-)
tools/perf/Documentation/perf-buildid-cache.txt +8 -3
@@ -41,9 +41,14 @@ OPTIONS --missing=:: List missing build ids in the cache for the specified file. -u:: ---update:: - Update specified file of the cache. It can be used to update kallsyms - kernel dso to vmlinux in order to support annotation. +--update=:: + Update specified file of the cache. Note that this doesn't remove + older entires since those may be still needed for annotating old + (or remote) perf.data. Only if there is already a cache which has + exactly same build-id, that is replaced by new one. It can be used + to update kallsyms and kernel dso to vmlinux in order to support + annotation. + -v:: --verbose:: Be more verbose.
tools/perf/builtin-buildid-cache.c +4 -2
@@ -255,7 +255,7 @@ static int build_id_cache__update_file(const char *filename)u8 build_id[BUILD_ID_SIZE ];char sbuild_id[BUILD_ID_SIZE *2 +1 ]; -int err; +int err =0 ;if (filename__read_build_id (filename, &build_id,sizeof (build_id)) <0 ) {pr_debug ("Couldn't read a build-id in %s\n" , filename);
@@ -263,7 +263,9 @@ static int build_id_cache__update_file(const char *filename) }build_id__sprintf (build_id,sizeof (build_id), sbuild_id); - err =build_id_cache__remove_s (sbuild_id); +if (build_id_cache__cached (sbuild_id)) + err =build_id_cache__remove_s (sbuild_id); +if (!err) err =build_id_cache__add_s (sbuild_id, filename, false, false);
tools/perf/util/build-id.c +12 -0
@@ -352,6 +352,18 @@ static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,return build_id_cache__add_s (sbuild_id, name, is_kallsyms, is_vdso); } +bool build_id_cache__cached (const char *sbuild_id) +{ +bool ret = false; +char *filename =build_id__filename (sbuild_id,NULL ,0 ); + +if (filename && !access (filename,F_OK )) + ret = true; +free (filename); + +return ret; +} +int build_id_cache__remove_s (const char *sbuild_id) {const size_t size =PATH_MAX ;
tools/perf/util/build-id.h +1 -0
@@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);int perf_session__write_buildid_table (struct perf_session *session,int fd);int perf_session__cache_build_ids (struct perf_session *session); +bool build_id_cache__cached (const char *sbuild_id);int build_id_cache__add_s (const char *sbuild_id,const char *name,bool is_kallsyms,bool is_vdso);int build_id_cache__remove_s (const char *sbuild_id);
tip-bot for Masami Hiramatsu
merge bot[tip:perf/core] perf buildid-cache: Add new buildid cache if update target is not cached
perf buildid-cache: Add new buildid cache if update target is not cached
Commit-ID: a50d11a10c2db86d7383c281d4e249d5393661e9 Gitweb: http://git.kernel.org/tip/a50d11a10c2db86d7383c281d4e249d5393661e9 Author: Masami Hiramatsu <[email protected]> AuthorDate: Thu, 26 Feb 2015 15:54:40 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 27 Feb 2015 10:08:37 -0300
perf buildid-cache: Add new buildid cache if update target is not cached
Add new buildid cache if the update target file is not cached.
This can happen when an old binary is replaced by new one after caching the old one. In this case, user sees his operation just failed.
But it does not look straight, since user just pass the binary "path", not "build-id".
----
# ./perf buildid-cache --add ./perf (update ./perf to new binary) # ./perf buildid-cache --update ./perf ./perf wasn't in the cache # ----
This patch adds given new binary to cache if the new binary is not cached. So we'll not see the above error.
----
# ./perf buildid-cache --add ./perf (update ./perf to new binary) # ./perf buildid-cache --update ./perf # ----
--- tools/perf/Documentation/perf-buildid-cache.txt | 11 ++++++++--- tools/perf/builtin-buildid-cache.c | 6 ++++-- tools/perf/util/build-id.c | 12 ++++++++++++ tools/perf/util/build-id.h | 1 + 4 files changed, 25 insertions(+), 5 deletions(-)
tools/perf/Documentation/perf-buildid-cache.txt +8 -3
@@ -41,9 +41,14 @@ OPTIONS --missing=:: List missing build ids in the cache for the specified file. -u:: ---update:: - Update specified file of the cache. It can be used to update kallsyms - kernel dso to vmlinux in order to support annotation. +--update=:: + Update specified file of the cache. Note that this doesn't remove + older entires since those may be still needed for annotating old + (or remote) perf.data. Only if there is already a cache which has + exactly same build-id, that is replaced by new one. It can be used + to update kallsyms and kernel dso to vmlinux in order to support + annotation. + -v:: --verbose:: Be more verbose.
tools/perf/builtin-buildid-cache.c +4 -2
@@ -255,7 +255,7 @@ static int build_id_cache__update_file(const char *filename)u8 build_id[BUILD_ID_SIZE ];char sbuild_id[BUILD_ID_SIZE *2 +1 ]; -int err; +int err =0 ;if (filename__read_build_id (filename, &build_id,sizeof (build_id)) <0 ) {pr_debug ("Couldn't read a build-id in %s\n" , filename);
@@ -263,7 +263,9 @@ static int build_id_cache__update_file(const char *filename) }build_id__sprintf (build_id,sizeof (build_id), sbuild_id); - err =build_id_cache__remove_s (sbuild_id); +if (build_id_cache__cached (sbuild_id)) + err =build_id_cache__remove_s (sbuild_id); +if (!err) err =build_id_cache__add_s (sbuild_id, filename, false, false);
tools/perf/util/build-id.c +12 -0
@@ -352,6 +352,18 @@ static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,return build_id_cache__add_s (sbuild_id, name, is_kallsyms, is_vdso); } +bool build_id_cache__cached (const char *sbuild_id) +{ +bool ret = false; +char *filename =build_id__filename (sbuild_id,NULL ,0 ); + +if (filename && !access (filename,F_OK )) + ret = true; +free (filename); + +return ret; +} +int build_id_cache__remove_s (const char *sbuild_id) {const size_t size =PATH_MAX ;
tools/perf/util/build-id.h +1 -0
@@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);int perf_session__write_buildid_table (struct perf_session *session,int fd);int perf_session__cache_build_ids (struct perf_session *session); +bool build_id_cache__cached (const char *sbuild_id);int build_id_cache__add_s (const char *sbuild_id,const char *name,bool is_kallsyms,bool is_vdso);int build_id_cache__remove_s (const char *sbuild_id);
PATCH v5 2/4 perf buildid-cache: Add --purge FILE to remove all caches of FILE
In reply to Masami HiramatsuAdd --purge FILE to remove all caches of FILE. Since the current --remove FILE removes a cache which has same build-id of given FILE. Since the command takes a FILE path, it can confuse user who tries to remove cache about FILE path.
-----
# ./perf buildid-cache -v --add ./perf Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok # (update the ./perf binary) # ./perf buildid-cache -v --remove ./perf Removing 305bbd1be68f66eca7e2d78db294653031edfa79 ./perf: FAIL ./perf wasn't in the cache ----- Actually, the --remove's FAIL is not shown, it just silently fails.
So, this patch adds --purge FILE action for such usecase. perf buildid-cache --purge FILE removes all caches which has same FILE path.
In other words, it removes all caches including old binaries.
-----
# ./perf buildid-cache -v --add ./perf Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok # (update the ./perf binary) # ./perf buildid-cache -v --purge ./perf Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok -----
BTW, if you want to purge all the caches, remove ~/.debug/* .
--- tools/perf/Documentation/perf-buildid-cache.txt | 13 ++-- tools/perf/builtin-buildid-cache.c | 44 ++++++++++++ tools/perf/util/build-id.c | 85 ++++++++++++++++++----- tools/perf/util/build-id.h | 1 4 files changed, 122 insertions(+), 21 deletions(-)
tools/perf/Documentation/perf-buildid-cache.txt +9 -4
@@ -12,9 +12,9 @@ SYNOPSIS DESCRIPTION ----------- -This command manages the build-id cache. It can add and remove files to/from -the cache. In the future it should as well purge older entries, set upper -limits for the space used by the cache, etc. +This command manages the build-id cache. It can add, remove, update and purge +files to/from the cache. In the future it should as well set upper limits for +the space used by the cache, etc. OPTIONS -------
@@ -36,7 +36,12 @@ OPTIONS actually made. -r:: --remove=:: - Remove specified file from the cache. + Remove a cached binary which has same build-id of specified file + from the cache. +-p:: +--purge=:: + Purge all cached binaries including older caches which have specified + path from the cache. -M:: --missing=:: List missing build ids in the cache for the specified file.
tools/perf/builtin-buildid-cache.c +44 -0
68 lines not on this page · raw message
tools/perf/util/build-id.c +68 -17
109 lines not on this page · raw message
tools/perf/util/build-id.h +1 -0
@@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);int perf_session__write_buildid_table (struct perf_session *session,int fd);int perf_session__cache_build_ids (struct perf_session *session); +struct strlist *build_id_cache__list_build_ids (const char *pathname);bool build_id_cache__cached (const char *sbuild_id);int build_id_cache__add_s (const char *sbuild_id,const char *name,bool is_kallsyms,bool is_vdso);
Arnaldo Carvalho de Melo
maintainerRe: [perf/core PATCH v5 2/4] perf buildid-cache: Add --purge FILE to remove all caches of FILE
Em Thu, Feb 26, 2015 at 03:54:47PM +0900, Masami Hiramatsu escreveu:
Quoted 27 lines
tools/perf/builtin-buildid-cache.c +9
-0@@ -285,6 +308,7 @@ int cmd_buildid_cache(int argc, const char **argv,bool force = false;char const *add_name_list_str =NULL , *remove_name_list_str =NULL , + *purge_name_list_str =NULL , *missing_filename =NULL , *update_name_list_str =NULL , *kcore_filename =NULL ;@@ -302,6 +326,8 @@ int cmd_buildid_cache(int argc, const char **argv, "file", "kcore file to add"),OPT_STRING ('r' ,"remove" , &remove_name_list_str,"file list" ,"file(s) to remove" ), +OPT_STRING ('p' ,"purge" , &purge_name_list_str,"path list" , +"path(s) to remove (remove old caches too)" ),OPT_STRING ('M' ,"missing" , &missing_filename,"file" ,"to find missing build ids in the cache" ), OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),@@ -368,6 +394,24 @@ int cmd_buildid_cache(int argc, const char **argv, } } +if (purge_name_list_str) { + list =strlist__new (true, purge_name_list_str); +if (list) { +strlist__for_each (pos, list) +if (build_id_cache__purge_path (pos->s)) { +if (errno ==ENOENT ) {
tools/perf/builtin-buildid-cache.c int cmd_buildid_cache(int argc, const char **argv, · line 412
Quoted 10 lines
tools/perf/builtin-buildid-cache.c +10
-0+pr_debug ("%s wasn't in the cache\n" , + pos->s); +continue ; + } +pr_warning ("Couldn't remove %s: %s\n" , + pos->s,strerror_r (errno, sbuf,sizeof (sbuf))); + } + +strlist__delete (list); + }
Here we need to at least warn the user that what he/she asked couldn't be done, i.e. strlist__new() failed.
Seeing --remove, --purge and --update makes me think eventually we need a OPT_STRLIST() :-)
I'm going thru the other patches, the first got in already.
Thanks for implementing --purge!
- Arnaldo
Quoted 133 lines
tools/perf/builtin-buildid-cache.c +2
-0+ } +if (missing_filename) ret =build_id_cache__fprintf_missing (session, stdout);tools/perf/util/build-id.c +68
-17@@ -281,35 +281,85 @@ void disable_buildid_cache(void) no_buildid_cache = true; } +static char *build_id_cache__dirname_from_path (const char *name, +bool is_kallsyms,bool is_vdso) +{ +char *realname = (char *)name, *filename; +bool slash = is_kallsyms || is_vdso; + +if (!slash) { + realname =realpath (name,NULL ); +if (!realname) +return NULL ; + } + +if (asprintf (&filename,"%s%s%s" , buildid_dir, slash ?"/" :"" , + is_vdso ?DSO__NAME_VDSO : realname) <0 ) + filename =NULL ; + +if (!slash) +free (realname); + +return filename; +} + +struct strlist *build_id_cache__list_build_ids (const char *pathname) +{ +struct strlist *list; +char *dirname; +DIR *dir; +struct dirent *d; + + list =strlist__new (true,NULL ); + dirname =build_id_cache__dirname_from_path (pathname, false, false); +if (!list || !dirname) + goto error_free; + +/* List up all dirents */ + dir =opendir (dirname); +if (!dir) + goto error_free; +while ((d =readdir (dir)) !=NULL ) { +if (!strcmp (d->d_name,"." ) || !strcmp (d->d_name,".." )) +continue ; +strlist__add (list, d->d_name); + } +closedir (dir); + +free (dirname); +return list; + +error_free: +free (dirname); +strlist__delete (list); +return NULL ; +} +int build_id_cache__add_s (const char *sbuild_id,const char *name,bool is_kallsyms,bool is_vdso) {const size_t size =PATH_MAX ; -char *realname, *filename =zalloc (size), +char *realname =NULL , *filename =NULL , *dirname =NULL , *linkname =zalloc (size), *targetname, *tmp; -int len, err =-1 ; -bool slash = is_kallsyms || is_vdso; +int err =-1 ; -if (is_kallsyms) { -if (symbol_conf.kptr_restrict) { -pr_debug ("Not caching a kptr_restrict'ed /proc/kallsyms\n" ); - err =0 ; - goto out_free; - } - realname = (char *) name; - }else +if (!is_kallsyms) { realname =realpath (name,NULL ); +if (!realname) + goto out_free; + } -if (realname ==NULL || filename ==NULL || linkname ==NULL ) + dirname =build_id_cache__dirname_from_path (name, is_kallsyms, is_vdso); +if (!dirname) goto out_free; - len =scnprintf (filename, size,"%s%s%s" , - buildid_dir, slash ?"/" :"" , - is_vdso ?DSO__NAME_VDSO : realname); -if (mkdir_p (filename,0755 )) +if (mkdir_p (dirname,0755 )) goto out_free; -snprintf (filename + len, size - len,"/%s" , sbuild_id); +if (asprintf (&filename,"%s/%s" , dirname, sbuild_id) <0 ) { + filename =NULL ; + goto out_free; + }if (access (filename,F_OK )) {if (is_kallsyms) {@@ -337,6 +387,7 @@ out_free:if (!is_kallsyms)free (realname);free (filename); +free (dirname);free (linkname);return err; }tools/perf/util/build-id.h +1
-0@@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);int perf_session__write_buildid_table (struct perf_session *session,int fd);int perf_session__cache_build_ids (struct perf_session *session); +struct strlist *build_id_cache__list_build_ids (const char *pathname);bool build_id_cache__cached (const char *sbuild_id);int build_id_cache__add_s (const char *sbuild_id,const char *name,bool is_kallsyms,bool is_vdso);
Re: Re: [perf/core PATCH v5 2/4] perf buildid-cache: Add --purge FILE to remove all caches of FILE
Quoted 33 lines · Arnaldo Carvalho de Melo
Em Thu, Feb 26, 2015 at 03:54:47PM +0900, Masami Hiramatsu escreveu:
Quoted 32 lines
tools/perf/builtin-buildid-cache.c +14
-0@@ -285,6 +308,7 @@ int cmd_buildid_cache(int argc, const char **argv,bool force = false;char const *add_name_list_str =NULL , *remove_name_list_str =NULL , + *purge_name_list_str =NULL , *missing_filename =NULL , *update_name_list_str =NULL , *kcore_filename =NULL ;@@ -302,6 +326,8 @@ int cmd_buildid_cache(int argc, const char **argv, "file", "kcore file to add"),OPT_STRING ('r' ,"remove" , &remove_name_list_str,"file list" ,"file(s) to remove" ), +OPT_STRING ('p' ,"purge" , &purge_name_list_str,"path list" , +"path(s) to remove (remove old caches too)" ),OPT_STRING ('M' ,"missing" , &missing_filename,"file" ,"to find missing build ids in the cache" ), OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),@@ -368,6 +394,24 @@ int cmd_buildid_cache(int argc, const char **argv, } } +if (purge_name_list_str) { + list =strlist__new (true, purge_name_list_str); +if (list) { +strlist__for_each (pos, list) +if (build_id_cache__purge_path (pos->s)) { +if (errno ==ENOENT ) { +pr_debug ("%s wasn't in the cache\n" , + pos->s); +continue ; + } + pr_warning("Couldn't remove %s: %s\n",
Quoted 8 lines
tools/perf/builtin-buildid-cache.c int cmd_buildid_cache(int argc, const char **argv, · line 412
Quoted 5 lines
tools/perf/builtin-buildid-cache.c +5
-0+ pos->s, strerror_r(errno, sbuf, sizeof(sbuf))); + } + +strlist__delete (list); + }Here we need to at least warn the user that what he/she asked couldn't be done, i.e.
strlist__new()failed.
Ah, OK I'll update it.
Quoted 3 lines
Seeing --remove, --purge and --update makes me think eventually we need a
OPT_STRLIST():-)
Agreed :)
Quoted 2 lines
I'm going thru the other patches, the first got in already.
Thanks!
Quoted 144 lines
Thanks for implementing --purge!
- Arnaldo
Quoted 126 lines
tools/perf/builtin-buildid-cache.c +2
-0+ } +if (missing_filename) ret =build_id_cache__fprintf_missing (session, stdout);tools/perf/util/build-id.c +68
-17@@ -281,35 +281,85 @@ void disable_buildid_cache(void) no_buildid_cache = true; } +static char *build_id_cache__dirname_from_path (const char *name, +bool is_kallsyms,bool is_vdso) +{ +char *realname = (char *)name, *filename; +bool slash = is_kallsyms || is_vdso; + +if (!slash) { + realname =realpath (name,NULL ); +if (!realname) +return NULL ; + } + +if (asprintf (&filename,"%s%s%s" , buildid_dir, slash ?"/" :"" , + is_vdso ?DSO__NAME_VDSO : realname) <0 ) + filename =NULL ; + +if (!slash) +free (realname); + +return filename; +} + +struct strlist *build_id_cache__list_build_ids (const char *pathname) +{ +struct strlist *list; +char *dirname; +DIR *dir; +struct dirent *d; + + list =strlist__new (true,NULL ); + dirname =build_id_cache__dirname_from_path (pathname, false, false); +if (!list || !dirname) + goto error_free; + +/* List up all dirents */ + dir =opendir (dirname); +if (!dir) + goto error_free; +while ((d =readdir (dir)) !=NULL ) { +if (!strcmp (d->d_name,"." ) || !strcmp (d->d_name,".." )) +continue ; +strlist__add (list, d->d_name); + } +closedir (dir); + +free (dirname); +return list; + +error_free: +free (dirname); +strlist__delete (list); +return NULL ; +} +int build_id_cache__add_s (const char *sbuild_id,const char *name,bool is_kallsyms,bool is_vdso) {const size_t size =PATH_MAX ; -char *realname, *filename =zalloc (size), +char *realname =NULL , *filename =NULL , *dirname =NULL , *linkname =zalloc (size), *targetname, *tmp; -int len, err =-1 ; -bool slash = is_kallsyms || is_vdso; +int err =-1 ; -if (is_kallsyms) { -if (symbol_conf.kptr_restrict) { -pr_debug ("Not caching a kptr_restrict'ed /proc/kallsyms\n" ); - err =0 ; - goto out_free; - } - realname = (char *) name; - }else +if (!is_kallsyms) { realname =realpath (name,NULL ); +if (!realname) + goto out_free; + } -if (realname ==NULL || filename ==NULL || linkname ==NULL ) + dirname =build_id_cache__dirname_from_path (name, is_kallsyms, is_vdso); +if (!dirname) goto out_free; - len =scnprintf (filename, size,"%s%s%s" , - buildid_dir, slash ?"/" :"" , - is_vdso ?DSO__NAME_VDSO : realname); -if (mkdir_p (filename,0755 )) +if (mkdir_p (dirname,0755 )) goto out_free; -snprintf (filename + len, size - len,"/%s" , sbuild_id); +if (asprintf (&filename,"%s/%s" , dirname, sbuild_id) <0 ) { + filename =NULL ; + goto out_free; + }if (access (filename,F_OK )) {if (is_kallsyms) {@@ -337,6 +387,7 @@ out_free:if (!is_kallsyms)free (realname);free (filename); +free (dirname);free (linkname);return err; }tools/perf/util/build-id.h +0
-0tools/perf/util/build-id.h bool perf_session__read_build_ids(struct perf_session *session, bool with_hits); · line 28
Quoted 7 lines
tools/perf/util/build-id.h +1
-0@@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);int perf_session__write_buildid_table (struct perf_session *session,int fd);int perf_session__cache_build_ids (struct perf_session *session); +struct strlist *build_id_cache__list_build_ids (const char *pathname);bool build_id_cache__cached (const char *sbuild_id);int build_id_cache__add_s (const char *sbuild_id,const char *name,bool is_kallsyms,bool is_vdso);--
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/
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
PATCH v5 3/4 perf-buildid-cache: Use pr_debug instead of verbose && pr_info
In reply to Masami HiramatsuUse pr_debug instead of the combination of verbose and pr_info.
"if (verbose) pr_info(...)" is same as "pr_debug(...)", replace it.
--- tools/perf/builtin-buildid-cache.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)
tools/perf/builtin-buildid-cache.c +8 -12
@@ -196,9 +196,8 @@ static int build_id_cache__add_file(const char *filename)build_id__sprintf (build_id,sizeof (build_id), sbuild_id); err =build_id_cache__add_s (sbuild_id, filename, false, false); -if (verbose) -pr_info ("Adding %s %s: %s\n" , sbuild_id, filename, - err ?"FAIL" :"Ok" ); +pr_debug ("Adding %s %s: %s\n" , sbuild_id, filename, + err ?"FAIL" :"Ok" );return err; }
@@ -216,9 +215,8 @@ static int build_id_cache__remove_file(const char *filename)build_id__sprintf (build_id,sizeof (build_id), sbuild_id); err =build_id_cache__remove_s (sbuild_id); -if (verbose) -pr_info ("Removing %s %s: %s\n" , sbuild_id, filename, - err ?"FAIL" :"Ok" ); +pr_debug ("Removing %s %s: %s\n" , sbuild_id, filename, + err ?"FAIL" :"Ok" );return err; }
@@ -235,9 +233,8 @@ static int build_id_cache__purge_path(const char *pathname)strlist__for_each (pos, list) { err =build_id_cache__remove_s (pos->s); -if (verbose) -pr_info ("Removing %s %s: %s\n" , pos->s, pathname, - err ?"FAIL" :"Ok" ); +pr_debug ("Removing %s %s: %s\n" , pos->s, pathname, + err ?"FAIL" :"Ok" );if (err)break ; }
@@ -292,9 +289,8 @@ static int build_id_cache__update_file(const char *filename)if (!err) err =build_id_cache__add_s (sbuild_id, filename, false, false); -if (verbose) -pr_info ("Updating %s %s: %s\n" , sbuild_id, filename, - err ?"FAIL" :"Ok" ); +pr_debug ("Updating %s %s: %s\n" , sbuild_id, filename, + err ?"FAIL" :"Ok" );return err; }
Arnaldo Carvalho de Melo
maintainerRe: [perf/core PATCH v5 3/4] perf-buildid-cache: Use pr_debug instead of verbose && pr_info
Em Thu, Feb 26, 2015 at 03:54:54PM +0900, Masami Hiramatsu escreveu:
Quoted 3 lines
Use
pr_debuginstead of the combination of verbose andpr_info."if (verbose)
pr_info(...)" is same as "pr_debug(...)", replace it.
This one doesn't apply because 2/4 got delayed.
- Arnaldo
PATCH v5 4/4 perf-buildid-cache: Show usage with incorrect params
In reply to Masami HiramatsuShow usage if no action is specified or unexpected parameter is given. In other words, be more user friendly.
--- tools/perf/builtin-buildid-cache.c | 5 +++++ 1 file changed, 5 insertions(+)
tools/perf/builtin-buildid-cache.c +5 -0
@@ -340,6 +340,11 @@ int cmd_buildid_cache(int argc, const char **argv, argc =parse_options (argc, argv, buildid_cache_options, buildid_cache_usage,0 ); +if (argc || (!add_name_list_str && !kcore_filename && + !remove_name_list_str && !purge_name_list_str && + !missing_filename && !update_name_list_str)) +usage_with_options (buildid_cache_usage, buildid_cache_options); +if (missing_filename) { file.path = missing_filename; file.force = force;
Arnaldo Carvalho de Melo
maintainerRe: [perf/core PATCH v5 4/4] perf-buildid-cache: Show usage with incorrect params
Em Thu, Feb 26, 2015 at 03:55:01PM +0900, Masami Hiramatsu escreveu:
Quoted 2 lines
Show usage if no action is specified or unexpected parameter is given. In other words, be more user friendly.
Thanks for doing that!
Doesn't apply due to previous patches being delayer at the moment.
- Arnaldo
Quoted 22 lines
Signed-off-by Masami Hiramatsu <[email protected]>--- tools/perf/builtin-buildid-cache.c | 5 +++++ 1 file changed, 5 insertions(+)tools/perf/builtin-buildid-cache.c +5
-0@@ -340,6 +340,11 @@ int cmd_buildid_cache(int argc, const char **argv, argc =parse_options (argc, argv, buildid_cache_options, buildid_cache_usage,0 ); +if (argc || (!add_name_list_str && !kcore_filename && + !remove_name_list_str && !purge_name_list_str && + !missing_filename && !update_name_list_str)) +usage_with_options (buildid_cache_usage, buildid_cache_options); +if (missing_filename) { file.path = missing_filename; file.force = force;