| Message ID | 20191030223448.12930-7-irogers@google.com |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Improvements to memory usage by parse events | expand |
On Wed, Oct 30, 2019 at 03:34:44PM -0700, Ian Rogers wrote: > If parsing fails then destructors are ran to clean the up the stack. > Rename the head union member to make the term and evlist use cases more > distinct, this simplifies matching the correct destructor. > > Signed-off-by: Ian Rogers <irogers@google.com> Acked-by: Jiri Olsa <jolsa@kernel.org> thanks, jirka > --- > tools/perf/util/parse-events.y | 69 +++++++++++++++++++++++----------- > 1 file changed, 48 insertions(+), 21 deletions(-) > > diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y > index 545ab7cefc20..035edfa8d42e 100644 > --- a/tools/perf/util/parse-events.y > +++ b/tools/perf/util/parse-events.y > @@ -12,6 +12,7 @@ > #include <stdio.h> > #include <linux/compiler.h> > #include <linux/types.h> > +#include <linux/zalloc.h> > #include "pmu.h" > #include "evsel.h" > #include "parse-events.h" > @@ -37,6 +38,25 @@ static struct list_head* alloc_list() > return list; > } > > +static void free_list_evsel(struct list_head* list_evsel) > +{ > + struct evsel *evsel, *tmp; > + > + list_for_each_entry_safe(evsel, tmp, list_evsel, core.node) { > + list_del_init(&evsel->core.node); > + perf_evsel__delete(evsel); > + } > + free(list_evsel); > +} > + > +static void free_term(struct parse_events_term *term) > +{ > + if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) > + free(term->val.str); > + zfree(&term->array.ranges); > + free(term); > +} > + > static void inc_group_count(struct list_head *list, > struct parse_events_state *parse_state) > { > @@ -66,6 +86,7 @@ static void inc_group_count(struct list_head *list, > %type <num> PE_VALUE_SYM_TOOL > %type <num> PE_RAW > %type <num> PE_TERM > +%type <num> value_sym > %type <str> PE_NAME > %type <str> PE_BPF_OBJECT > %type <str> PE_BPF_SOURCE > @@ -76,37 +97,43 @@ static void inc_group_count(struct list_head *list, > %type <str> PE_EVENT_NAME > %type <str> PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT > %type <str> PE_DRV_CFG_TERM > -%type <num> value_sym > -%type <head> event_config > -%type <head> opt_event_config > -%type <head> opt_pmu_config > +%destructor { free ($$); } <str> > %type <term> event_term > -%type <head> event_pmu > -%type <head> event_legacy_symbol > -%type <head> event_legacy_cache > -%type <head> event_legacy_mem > -%type <head> event_legacy_tracepoint > +%destructor { free_term ($$); } <term> > +%type <list_terms> event_config > +%type <list_terms> opt_event_config > +%type <list_terms> opt_pmu_config > +%destructor { parse_events_terms__delete ($$); } <list_terms> > +%type <list_evsel> event_pmu > +%type <list_evsel> event_legacy_symbol > +%type <list_evsel> event_legacy_cache > +%type <list_evsel> event_legacy_mem > +%type <list_evsel> event_legacy_tracepoint > +%type <list_evsel> event_legacy_numeric > +%type <list_evsel> event_legacy_raw > +%type <list_evsel> event_bpf_file > +%type <list_evsel> event_def > +%type <list_evsel> event_mod > +%type <list_evsel> event_name > +%type <list_evsel> event > +%type <list_evsel> events > +%type <list_evsel> group_def > +%type <list_evsel> group > +%type <list_evsel> groups > +%destructor { free_list_evsel ($$); } <list_evsel> > %type <tracepoint_name> tracepoint_name > -%type <head> event_legacy_numeric > -%type <head> event_legacy_raw > -%type <head> event_bpf_file > -%type <head> event_def > -%type <head> event_mod > -%type <head> event_name > -%type <head> event > -%type <head> events > -%type <head> group_def > -%type <head> group > -%type <head> groups > +%destructor { free ($$.sys); free ($$.event); } <tracepoint_name> > %type <array> array > %type <array> array_term > %type <array> array_terms > +%destructor { free ($$.ranges); } <array> > > %union > { > char *str; > u64 num; > - struct list_head *head; > + struct list_head *list_evsel; > + struct list_head *list_terms; > struct parse_events_term *term; > struct tracepoint_name { > char *sys; > -- > 2.24.0.rc1.363.gb1bccd3e3d-goog >
Em Wed, Nov 06, 2019 at 03:24:24PM +0100, Jiri Olsa escreveu: > On Wed, Oct 30, 2019 at 03:34:44PM -0700, Ian Rogers wrote: > > If parsing fails then destructors are ran to clean the up the stack. > > Rename the head union member to make the term and evlist use cases more > > distinct, this simplifies matching the correct destructor. > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > Acked-by: Jiri Olsa <jolsa@kernel.org> > > thanks, > jirka > > @@ -37,6 +38,25 @@ static struct list_head* alloc_list() > > return list; > > } > > > > +static void free_list_evsel(struct list_head* list_evsel) > > +{ > > + struct evsel *evsel, *tmp; > > + > > + list_for_each_entry_safe(evsel, tmp, list_evsel, core.node) { > > + list_del_init(&evsel->core.node); > > + perf_evsel__delete(evsel); > > + } > > + free(list_evsel); > > +} Applying, but later I think we should use something like: void __perf_evlist__purge(truct list_head *list) { with the above code } And: void perf_evlist__purge(struct perf_evlist *evlist) { __perf_evlist__purge(&evlist->entries); evlist->nr_entries = 0; } To replace the existing: static void perf_evlist__purge(struct perf_evlist *evlist) { struct perf_evsel *pos, *n; perf_evlist__for_each_entry_safe(evlist, n, pos) { list_del_init(&pos->node); perf_evsel__delete(pos); } evlist->nr_entries = 0; } Anyway, applied. - Arnaldo > > +static void free_term(struct parse_events_term *term) > > +{ > > + if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) > > + free(term->val.str); > > + zfree(&term->array.ranges); > > + free(term); > > +} > > + > > static void inc_group_count(struct list_head *list, > > struct parse_events_state *parse_state) > > { > > @@ -66,6 +86,7 @@ static void inc_group_count(struct list_head *list, > > %type <num> PE_VALUE_SYM_TOOL > > %type <num> PE_RAW > > %type <num> PE_TERM > > +%type <num> value_sym > > %type <str> PE_NAME > > %type <str> PE_BPF_OBJECT > > %type <str> PE_BPF_SOURCE > > @@ -76,37 +97,43 @@ static void inc_group_count(struct list_head *list, > > %type <str> PE_EVENT_NAME > > %type <str> PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT > > %type <str> PE_DRV_CFG_TERM > > -%type <num> value_sym > > -%type <head> event_config > > -%type <head> opt_event_config > > -%type <head> opt_pmu_config > > +%destructor { free ($$); } <str> > > %type <term> event_term > > -%type <head> event_pmu > > -%type <head> event_legacy_symbol > > -%type <head> event_legacy_cache > > -%type <head> event_legacy_mem > > -%type <head> event_legacy_tracepoint > > +%destructor { free_term ($$); } <term> > > +%type <list_terms> event_config > > +%type <list_terms> opt_event_config > > +%type <list_terms> opt_pmu_config > > +%destructor { parse_events_terms__delete ($$); } <list_terms> > > +%type <list_evsel> event_pmu > > +%type <list_evsel> event_legacy_symbol > > +%type <list_evsel> event_legacy_cache > > +%type <list_evsel> event_legacy_mem > > +%type <list_evsel> event_legacy_tracepoint > > +%type <list_evsel> event_legacy_numeric > > +%type <list_evsel> event_legacy_raw > > +%type <list_evsel> event_bpf_file > > +%type <list_evsel> event_def > > +%type <list_evsel> event_mod > > +%type <list_evsel> event_name > > +%type <list_evsel> event > > +%type <list_evsel> events > > +%type <list_evsel> group_def > > +%type <list_evsel> group > > +%type <list_evsel> groups > > +%destructor { free_list_evsel ($$); } <list_evsel> > > %type <tracepoint_name> tracepoint_name > > -%type <head> event_legacy_numeric > > -%type <head> event_legacy_raw > > -%type <head> event_bpf_file > > -%type <head> event_def > > -%type <head> event_mod > > -%type <head> event_name > > -%type <head> event > > -%type <head> events > > -%type <head> group_def > > -%type <head> group > > -%type <head> groups > > +%destructor { free ($$.sys); free ($$.event); } <tracepoint_name> > > %type <array> array > > %type <array> array_term > > %type <array> array_terms > > +%destructor { free ($$.ranges); } <array> > > > > %union > > { > > char *str; > > u64 num; > > - struct list_head *head; > > + struct list_head *list_evsel; > > + struct list_head *list_terms; > > struct parse_events_term *term; > > struct tracepoint_name { > > char *sys; > > -- > > 2.24.0.rc1.363.gb1bccd3e3d-goog > >
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 545ab7cefc20..035edfa8d42e 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -12,6 +12,7 @@ #include <stdio.h> #include <linux/compiler.h> #include <linux/types.h> +#include <linux/zalloc.h> #include "pmu.h" #include "evsel.h" #include "parse-events.h" @@ -37,6 +38,25 @@ static struct list_head* alloc_list() return list; } +static void free_list_evsel(struct list_head* list_evsel) +{ + struct evsel *evsel, *tmp; + + list_for_each_entry_safe(evsel, tmp, list_evsel, core.node) { + list_del_init(&evsel->core.node); + perf_evsel__delete(evsel); + } + free(list_evsel); +} + +static void free_term(struct parse_events_term *term) +{ + if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) + free(term->val.str); + zfree(&term->array.ranges); + free(term); +} + static void inc_group_count(struct list_head *list, struct parse_events_state *parse_state) { @@ -66,6 +86,7 @@ static void inc_group_count(struct list_head *list, %type <num> PE_VALUE_SYM_TOOL %type <num> PE_RAW %type <num> PE_TERM +%type <num> value_sym %type <str> PE_NAME %type <str> PE_BPF_OBJECT %type <str> PE_BPF_SOURCE @@ -76,37 +97,43 @@ static void inc_group_count(struct list_head *list, %type <str> PE_EVENT_NAME %type <str> PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT %type <str> PE_DRV_CFG_TERM -%type <num> value_sym -%type <head> event_config -%type <head> opt_event_config -%type <head> opt_pmu_config +%destructor { free ($$); } <str> %type <term> event_term -%type <head> event_pmu -%type <head> event_legacy_symbol -%type <head> event_legacy_cache -%type <head> event_legacy_mem -%type <head> event_legacy_tracepoint +%destructor { free_term ($$); } <term> +%type <list_terms> event_config +%type <list_terms> opt_event_config +%type <list_terms> opt_pmu_config +%destructor { parse_events_terms__delete ($$); } <list_terms> +%type <list_evsel> event_pmu +%type <list_evsel> event_legacy_symbol +%type <list_evsel> event_legacy_cache +%type <list_evsel> event_legacy_mem +%type <list_evsel> event_legacy_tracepoint +%type <list_evsel> event_legacy_numeric +%type <list_evsel> event_legacy_raw +%type <list_evsel> event_bpf_file +%type <list_evsel> event_def +%type <list_evsel> event_mod +%type <list_evsel> event_name +%type <list_evsel> event +%type <list_evsel> events +%type <list_evsel> group_def +%type <list_evsel> group +%type <list_evsel> groups +%destructor { free_list_evsel ($$); } <list_evsel> %type <tracepoint_name> tracepoint_name -%type <head> event_legacy_numeric -%type <head> event_legacy_raw -%type <head> event_bpf_file -%type <head> event_def -%type <head> event_mod -%type <head> event_name -%type <head> event -%type <head> events -%type <head> group_def -%type <head> group -%type <head> groups +%destructor { free ($$.sys); free ($$.event); } <tracepoint_name> %type <array> array %type <array> array_term %type <array> array_terms +%destructor { free ($$.ranges); } <array> %union { char *str; u64 num; - struct list_head *head; + struct list_head *list_evsel; + struct list_head *list_terms; struct parse_events_term *term; struct tracepoint_name { char *sys;
If parsing fails then destructors are ran to clean the up the stack. Rename the head union member to make the term and evlist use cases more distinct, this simplifies matching the correct destructor. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/parse-events.y | 69 +++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 21 deletions(-)