Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1213882 > unrolled thread

[PATCH 09/11] perf tools: Propagate error info for the tracepoint parsing

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 09/11] perf tools: Propagate error info for the tracepoint parsing Jiri Olsa <jolsa@kernel.org> - 2015-08-26 15:50 +0200
    Re: [PATCH 09/11] perf tools: Propagate error info for the  tracepoint parsing Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-28 15:30 +0200
      Re: [PATCH 09/11] perf tools: Propagate error info for the  tracepoint parsing Jiri Olsa <jolsa@redhat.com> - 2015-08-28 15:40 +0200

#1213882 — [PATCH 09/11] perf tools: Propagate error info for the tracepoint parsing

Pass 'struct parse_events_error *error' to the parse-event.c
tracepoint adding path. It will be filled with error data
in following patches.

Link: http://lkml.kernel.org/n/tip-las1hm5zf58b0twd27h9895b@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.c | 27 ++++++++++++++++-----------
 tools/perf/util/parse-events.h |  3 ++-
 tools/perf/util/parse-events.y |  4 ++--
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index d826e6f515db..5d17409039c8 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -387,7 +387,8 @@ int parse_events_add_cache(struct list_head *list, int *idx,
 }
 
 static int add_tracepoint(struct list_head *list, int *idx,
-			  char *sys_name, char *evt_name)
+			  char *sys_name, char *evt_name,
+			  struct parse_events_error *error __maybe_unused)
 {
 	struct perf_evsel *evsel;
 
@@ -401,7 +402,8 @@ static int add_tracepoint(struct list_head *list, int *idx,
 }
 
 static int add_tracepoint_multi_event(struct list_head *list, int *idx,
-				      char *sys_name, char *evt_name)
+				      char *sys_name, char *evt_name,
+				      struct parse_events_error *error)
 {
 	char evt_path[MAXPATHLEN];
 	struct dirent *evt_ent;
@@ -425,7 +427,7 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx,
 		if (!strglobmatch(evt_ent->d_name, evt_name))
 			continue;
 
-		ret = add_tracepoint(list, idx, sys_name, evt_ent->d_name);
+		ret = add_tracepoint(list, idx, sys_name, evt_ent->d_name, error);
 	}
 
 	closedir(evt_dir);
@@ -433,15 +435,17 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx,
 }
 
 static int add_tracepoint_event(struct list_head *list, int *idx,
-				char *sys_name, char *evt_name)
+				char *sys_name, char *evt_name,
+				struct parse_events_error *error)
 {
 	return strpbrk(evt_name, "*?") ?
-	       add_tracepoint_multi_event(list, idx, sys_name, evt_name) :
-	       add_tracepoint(list, idx, sys_name, evt_name);
+	       add_tracepoint_multi_event(list, idx, sys_name, evt_name, error) :
+	       add_tracepoint(list, idx, sys_name, evt_name, error);
 }
 
 static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
-				    char *sys_name, char *evt_name)
+				    char *sys_name, char *evt_name,
+				    struct parse_events_error *error)
 {
 	struct dirent *events_ent;
 	DIR *events_dir;
@@ -465,7 +469,7 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
 			continue;
 
 		ret = add_tracepoint_event(list, idx, events_ent->d_name,
-					   evt_name);
+					   evt_name, error);
 	}
 
 	closedir(events_dir);
@@ -473,12 +477,13 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
 }
 
 int parse_events_add_tracepoint(struct list_head *list, int *idx,
-				char *sys, char *event)
+				char *sys, char *event,
+				struct parse_events_error *error)
 {
 	if (strpbrk(sys, "*?"))
-		return add_tracepoint_multi_sys(list, idx, sys, event);
+		return add_tracepoint_multi_sys(list, idx, sys, event, error);
 	else
-		return add_tracepoint_event(list, idx, sys, event);
+		return add_tracepoint_event(list, idx, sys, event, error);
 }
 
 static int
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index a09b0e210997..ffee7ece75a6 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -118,7 +118,8 @@ int parse_events__modifier_event(struct list_head *list, char *str, bool add);
 int parse_events__modifier_group(struct list_head *list, char *event_mod);
 int parse_events_name(struct list_head *list, char *name);
 int parse_events_add_tracepoint(struct list_head *list, int *idx,
-				char *sys, char *event);
+				char *sys, char *event,
+				struct parse_events_error *error);
 int parse_events_add_numeric(struct parse_events_evlist *data,
 			     struct list_head *list,
 			     u32 type, u64 config,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 591905a02b92..742914746794 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -376,7 +376,7 @@ PE_NAME '-' PE_NAME ':' PE_NAME
 	snprintf(&sys_name, 128, "%s-%s", $1, $3);
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5));
+	ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5, data->error));
 	$$ = list;
 }
 |
@@ -386,7 +386,7 @@ PE_NAME ':' PE_NAME
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	if (parse_events_add_tracepoint(list, &data->idx, $1, $3)) {
+	if (parse_events_add_tracepoint(list, &data->idx, $1, $3, data->error)) {
 		struct parse_events_error *error = data->error;
 
 		if (error) {
-- 
2.4.3

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

[toc] | [next] | [standalone]

#1215335 — Re: [PATCH 09/11] perf tools: Propagate error info for the tracepoint parsing

On Wed, 26 Aug, at 03:46:51PM, Jiri Olsa wrote:
> Pass 'struct parse_events_error *error' to the parse-event.c
> tracepoint adding path. It will be filled with error data
> in following patches.
> 
> Link: http://lkml.kernel.org/n/tip-las1hm5zf58b0twd27h9895b@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/parse-events.c | 27 ++++++++++++++++-----------
>  tools/perf/util/parse-events.h |  3 ++-
>  tools/perf/util/parse-events.y |  4 ++--
>  3 files changed, 20 insertions(+), 14 deletions(-)

Is there a reason you decided to go ahead with passing 'data->error'
directly to the parsing functions instead of adding some global state
for recording errors, like a globally accessible 'parse_events_error'?

Because I notice that if someone wanted to improve the breakpoint
parsing code, they'd basically need to make the same changes you've
made in this patch.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1215343 — Re: [PATCH 09/11] perf tools: Propagate error info for the tracepoint parsing

On Fri, Aug 28, 2015 at 02:20:52PM +0100, Matt Fleming wrote:
> On Wed, 26 Aug, at 03:46:51PM, Jiri Olsa wrote:
> > Pass 'struct parse_events_error *error' to the parse-event.c
> > tracepoint adding path. It will be filled with error data
> > in following patches.
> > 
> > Link: http://lkml.kernel.org/n/tip-las1hm5zf58b0twd27h9895b@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/util/parse-events.c | 27 ++++++++++++++++-----------
> >  tools/perf/util/parse-events.h |  3 ++-
> >  tools/perf/util/parse-events.y |  4 ++--
> >  3 files changed, 20 insertions(+), 14 deletions(-)
> 
> Is there a reason you decided to go ahead with passing 'data->error'
> directly to the parsing functions instead of adding some global state
> for recording errors, like a globally accessible 'parse_events_error'?
> 
> Because I notice that if someone wanted to improve the breakpoint
> parsing code, they'd basically need to make the same changes you've
> made in this patch.

well, I dont see much benefit other than loosing one extra arg
and worrying about concurent access in the future.. but I might
have missed some

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web