| Message ID | 20211104111047.302660-9-tz.stoyanov@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libtracefs dynamic events support | expand |
On 4.11.21 г. 13:10, Tzvetomir Stoyanov (VMware) wrote: > Synthetic events are type of ftrace dynamic events. The tracefs library > has dedicated APIs to manage dynamic events of all types. In order the > code to be consistent, the creation of synthetic events inside the > library is reimplemented with these new dynamic events APIs. > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > src/tracefs-hist.c | 103 +++++++++++++++++++++------------------------ > 1 file changed, 47 insertions(+), 56 deletions(-) > > diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c > index 9009dba..08bb2da 100644 > --- a/src/tracefs-hist.c > +++ b/src/tracefs-hist.c > @@ -661,6 +661,7 @@ struct tracefs_synth { > struct tep_event *end_event; > struct action *actions; > struct action **next_action; > + struct tracefs_dynevent *dyn_event; > char *name; > char **synthetic_fields; > char **synthetic_args; > @@ -719,6 +720,7 @@ void tracefs_synth_free(struct tracefs_synth *synth) > synth->actions = action->next; > action_free(action); > } > + tracefs_dynevent_free(synth->dyn_event); > > free(synth); > } > @@ -889,6 +891,28 @@ synth_init_from(struct tep_handle *tep, const char *start_system, > return synth; > } > > +static int alloc_synthetic_event(struct tracefs_synth *synth) > +{ > + char *synthetic_format; > + const char *field; > + int i; > + > + synthetic_format = strdup(""); > + if (!synthetic_format) > + return -1; > + > + for (i = 0; synth->synthetic_fields && synth->synthetic_fields[i]; i++) { > + field = synth->synthetic_fields[i]; > + synthetic_format = append_string(synthetic_format, " ", field); > + } > + > + synth->dyn_event = dynevent_alloc(TRACEFS_DYNEVENT_SYNTH, NULL, > + synth->name, NULL, synthetic_format); > + free(synthetic_format); > + > + return synth->dyn_event ? 0 : -1; > +} > + > /** > * tracefs_synth_alloc - create a new tracefs_synth instance > * @tep: The tep handle that holds the events to work on > @@ -1609,38 +1633,6 @@ int tracefs_synth_save(struct tracefs_synth *synth, > return 0; > } > > -static char *create_synthetic_event(struct tracefs_synth *synth) > -{ > - char *synthetic_event; > - const char *field; > - int i; > - > - synthetic_event = strdup(synth->name); > - if (!synthetic_event) > - return NULL; > - > - for (i = 0; synth->synthetic_fields && synth->synthetic_fields[i]; i++) { > - field = synth->synthetic_fields[i]; > - synthetic_event = append_string(synthetic_event, " ", field); > - } > - > - return synthetic_event; > -} > - > -static int remove_synthetic(const char *synthetic) > -{ > - char *str; > - int ret; > - > - ret = asprintf(&str, "!%s", synthetic); > - if (ret < 0) > - return -1; > - > - ret = tracefs_instance_file_append(NULL, "synthetic_events", str); > - free(str); > - return ret < 0 ? -1 : 0; > -} > - > static int remove_hist(struct tracefs_instance *instance, > struct tep_event *event, const char *hist) > { > @@ -1919,7 +1911,6 @@ tracefs_synth_get_start_hist(struct tracefs_synth *synth) > int tracefs_synth_create(struct tracefs_instance *instance, I wonder is it really necessary this function to take 'tracefs_instance' as argument? I remember from conversations with Steven that setting the histograms/triggers in the top instance will have the same effect. Or maybe I am wrong? The same question applies for the 'destroy' API. Thanks! Yordan > struct tracefs_synth *synth) > { > - char *synthetic_event; > char *start_hist = NULL; > char *end_hist = NULL; > int ret; > @@ -1937,14 +1928,10 @@ int tracefs_synth_create(struct tracefs_instance *instance, > if (verify_state(synth) < 0) > return -1; > > - synthetic_event = create_synthetic_event(synth); > - if (!synthetic_event) > + if (!synth->dyn_event && alloc_synthetic_event(synth)) > + return -1; > + if (!tracefs_dynevent_create(synth->dyn_event)) > return -1; > - > - ret = tracefs_instance_file_append(NULL, "synthetic_events", > - synthetic_event); > - if (ret < 0) > - goto free_synthetic; > > start_hist = create_hist(synth->start_keys, synth->start_vars); > start_hist = append_filter(start_hist, synth->start_filter, > @@ -1980,9 +1967,7 @@ int tracefs_synth_create(struct tracefs_instance *instance, > remove_synthetic: > free(end_hist); > free(start_hist); > - remove_synthetic(synthetic_event); > - free_synthetic: > - free(synthetic_event); > + tracefs_dynevent_destroy(synth->dyn_event, false); > return -1; > } > > @@ -2007,7 +1992,6 @@ int tracefs_synth_create(struct tracefs_instance *instance, > int tracefs_synth_destroy(struct tracefs_instance *instance, > struct tracefs_synth *synth) > { > - char *synthetic_event; > char *hist; > int ret; > > @@ -2041,11 +2025,7 @@ int tracefs_synth_destroy(struct tracefs_instance *instance, > ret = remove_hist(instance, synth->start_event, hist); > free(hist); > > - synthetic_event = create_synthetic_event(synth); > - if (!synthetic_event) > - return -1; > - > - ret = remove_synthetic(synthetic_event); > + ret = tracefs_dynevent_destroy(synth->dyn_event, false); > > return ret ? -1 : 0; > } > @@ -2067,7 +2047,7 @@ int tracefs_synth_show(struct trace_seq *seq, > struct tracefs_instance *instance, > struct tracefs_synth *synth) > { > - char *synthetic_event = NULL; > + bool new_event = false; > char *hist = NULL; > char *path; > int ret = -1; > @@ -2082,16 +2062,19 @@ int tracefs_synth_show(struct trace_seq *seq, > return -1; > } > > - synthetic_event = create_synthetic_event(synth); > - if (!synthetic_event) > - return -1; > + if (!synth->dyn_event) { > + if (alloc_synthetic_event(synth)) > + return -1; > + new_event = true; > + } > > path = trace_find_tracing_dir(); > if (!path) > goto out_free; > > - trace_seq_printf(seq, "echo '%s' > %s/synthetic_events\n", > - synthetic_event, path); > + trace_seq_printf(seq, "echo '%s%s %s' > %s/%s\n", > + synth->dyn_event->prefix, synth->dyn_event->event, > + synth->dyn_event->format, path, synth->dyn_event->trace_file); > > tracefs_put_tracing_file(path); > path = tracefs_instance_get_dir(instance); > @@ -2116,10 +2099,18 @@ int tracefs_synth_show(struct trace_seq *seq, > hist, path, synth->end_event->system, > synth->end_event->name); > > + if (new_event) { > + tracefs_dynevent_free(synth->dyn_event); > + synth->dyn_event = NULL; > + } > + > ret = 0; > out_free: > - free(synthetic_event); > free(hist); > tracefs_put_tracing_file(path); > + if (new_event) { > + tracefs_dynevent_free(synth->dyn_event); > + synth->dyn_event = NULL; > + } > return ret; > } >
On Thu, 4 Nov 2021 14:47:34 +0200 Yordan Karadzhov <y.karadz@gmail.com> wrote: > > @@ -1919,7 +1911,6 @@ tracefs_synth_get_start_hist(struct tracefs_synth *synth) > > int tracefs_synth_create(struct tracefs_instance *instance, > > I wonder is it really necessary this function to take 'tracefs_instance' as argument? > > I remember from conversations with Steven that setting the histograms/triggers in the top instance will have the same > effect. Or maybe I am wrong? The same question applies for the 'destroy' API. Good point. Yeah, let's make the events global, and just use the top level for creation and destruction. As events added to the system affect all instances, they should only be done at the top level. -- Steve
On Thu, 4 Nov 2021 13:10:45 +0200 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > Synthetic events are type of ftrace dynamic events. The tracefs library > has dedicated APIs to manage dynamic events of all types. In order the > code to be consistent, the creation of synthetic events inside the > library is reimplemented with these new dynamic events APIs. > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > src/tracefs-hist.c | 103 +++++++++++++++++++++------------------------ > 1 file changed, 47 insertions(+), 56 deletions(-) > > diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c > index 9009dba..08bb2da 100644 > --- a/src/tracefs-hist.c > +++ b/src/tracefs-hist.c > @@ -661,6 +661,7 @@ struct tracefs_synth { > struct tep_event *end_event; > struct action *actions; > struct action **next_action; > + struct tracefs_dynevent *dyn_event; > char *name; > char **synthetic_fields; > char **synthetic_args; > @@ -719,6 +720,7 @@ void tracefs_synth_free(struct tracefs_synth *synth) > synth->actions = action->next; > action_free(action); > } > + tracefs_dynevent_free(synth->dyn_event); > > free(synth); > } > @@ -889,6 +891,28 @@ synth_init_from(struct tep_handle *tep, const char *start_system, > return synth; > } > > +static int alloc_synthetic_event(struct tracefs_synth *synth) > +{ > + char *synthetic_format; Should we just call it "format". It's obviously for the synthetic event. > + const char *field; > + int i; > + > + synthetic_format = strdup(""); > + if (!synthetic_format) > + return -1; > + > + for (i = 0; synth->synthetic_fields && synth->synthetic_fields[i]; i++) { > + field = synth->synthetic_fields[i]; > + synthetic_format = append_string(synthetic_format, " ", field); My OCD is bothering me that the first field has a space appended to it. format = append_string(format, i ? " " : NULL, field); > + } > + > + synth->dyn_event = dynevent_alloc(TRACEFS_DYNEVENT_SYNTH, NULL, > + synth->name, NULL, synthetic_format); > + free(synthetic_format); > + > + return synth->dyn_event ? 0 : -1; > +} > + > /** > * tracefs_synth_alloc - create a new tracefs_synth instance > * @tep: The tep handle that holds the events to work on > @@ -1609,38 +1633,6 @@ int tracefs_synth_save(struct tracefs_synth *synth, > return 0; > } > > -static char *create_synthetic_event(struct tracefs_synth *synth) > -{ > - char *synthetic_event; > - const char *field; > - int i; > - > - synthetic_event = strdup(synth->name); > - if (!synthetic_event) > - return NULL; > - > - for (i = 0; synth->synthetic_fields && synth->synthetic_fields[i]; i++) { > - field = synth->synthetic_fields[i]; > - synthetic_event = append_string(synthetic_event, " ", field); > - } > - > - return synthetic_event; > -} > - > -static int remove_synthetic(const char *synthetic) > -{ > - char *str; > - int ret; > - > - ret = asprintf(&str, "!%s", synthetic); > - if (ret < 0) > - return -1; > - > - ret = tracefs_instance_file_append(NULL, "synthetic_events", str); > - free(str); > - return ret < 0 ? -1 : 0; > -} > - > static int remove_hist(struct tracefs_instance *instance, > struct tep_event *event, const char *hist) > { > @@ -1919,7 +1911,6 @@ tracefs_synth_get_start_hist(struct tracefs_synth *synth) > int tracefs_synth_create(struct tracefs_instance *instance, > struct tracefs_synth *synth) I think we should call this: tracefs_synth_create_raw() that passes in the "instance". Then we could have tracefs_synth_create() just use the top instance (no instance passed to it), and we could decide later if we want to create an internal instance or not to do the work to keep from modifying the top instance. That is, we would have: int tracefs_synth_create(struct tracefs_synth *synth) { return tracefs_synth_create_raw(synth->create_instance, synth); } Have synth->create_instance be defaulted to NULL on allocation, and would get updated by another interface perhaps? We could also add to the _raw() function: if (synth->create_instance && instance && synth->create_instance != instance) { errno = EINVAL; return -1 } if (!synth->create_instance && instance) { syth->create_instance = instance; trace_get_instance(instance); } Would need to have in the destructor: if (synth->create_instance) trace_put_instance(synth->create_instance); Or something like that. > { > - char *synthetic_event; > char *start_hist = NULL; > char *end_hist = NULL; > int ret; > @@ -1937,14 +1928,10 @@ int tracefs_synth_create(struct tracefs_instance *instance, > if (verify_state(synth) < 0) > return -1; > > - synthetic_event = create_synthetic_event(synth); > - if (!synthetic_event) > + if (!synth->dyn_event && alloc_synthetic_event(synth)) > + return -1; > + if (!tracefs_dynevent_create(synth->dyn_event)) > return -1; > - > - ret = tracefs_instance_file_append(NULL, "synthetic_events", > - synthetic_event); > - if (ret < 0) > - goto free_synthetic; > > start_hist = create_hist(synth->start_keys, synth->start_vars); > start_hist = append_filter(start_hist, synth->start_filter, > @@ -1980,9 +1967,7 @@ int tracefs_synth_create(struct tracefs_instance *instance, > remove_synthetic: > free(end_hist); > free(start_hist); > - remove_synthetic(synthetic_event); > - free_synthetic: > - free(synthetic_event); > + tracefs_dynevent_destroy(synth->dyn_event, false); > return -1; > } > > @@ -2007,7 +1992,6 @@ int tracefs_synth_create(struct tracefs_instance *instance, > int tracefs_synth_destroy(struct tracefs_instance *instance, > struct tracefs_synth *synth) > { > - char *synthetic_event; > char *hist; > int ret; > > @@ -2041,11 +2025,7 @@ int tracefs_synth_destroy(struct tracefs_instance *instance, > ret = remove_hist(instance, synth->start_event, hist); > free(hist); > > - synthetic_event = create_synthetic_event(synth); > - if (!synthetic_event) > - return -1; > - > - ret = remove_synthetic(synthetic_event); > + ret = tracefs_dynevent_destroy(synth->dyn_event, false); > > return ret ? -1 : 0; > } > @@ -2067,7 +2047,7 @@ int tracefs_synth_show(struct trace_seq *seq, > struct tracefs_instance *instance, > struct tracefs_synth *synth) > { > - char *synthetic_event = NULL; > + bool new_event = false; > char *hist = NULL; > char *path; > int ret = -1; > @@ -2082,16 +2062,19 @@ int tracefs_synth_show(struct trace_seq *seq, > return -1; > } > > - synthetic_event = create_synthetic_event(synth); > - if (!synthetic_event) > - return -1; > + if (!synth->dyn_event) { > + if (alloc_synthetic_event(synth)) > + return -1; > + new_event = true; > + } We should start looking at making this thread safe. Probably need to add comments with /* TODO - make thread safe */ so we know where to look. Don't need to do it for this patch set, but we need to remember. The comment might help for now (that is, add the comment to the next series). -- Steve > > path = trace_find_tracing_dir(); > if (!path) > goto out_free; > > - trace_seq_printf(seq, "echo '%s' > %s/synthetic_events\n", > - synthetic_event, path); > + trace_seq_printf(seq, "echo '%s%s %s' > %s/%s\n", > + synth->dyn_event->prefix, synth->dyn_event->event, > + synth->dyn_event->format, path, synth->dyn_event->trace_file); > > tracefs_put_tracing_file(path); > path = tracefs_instance_get_dir(instance); > @@ -2116,10 +2099,18 @@ int tracefs_synth_show(struct trace_seq *seq, > hist, path, synth->end_event->system, > synth->end_event->name); > > + if (new_event) { > + tracefs_dynevent_free(synth->dyn_event); > + synth->dyn_event = NULL; > + } > + > ret = 0; > out_free: > - free(synthetic_event); > free(hist); > tracefs_put_tracing_file(path); > + if (new_event) { > + tracefs_dynevent_free(synth->dyn_event); > + synth->dyn_event = NULL; > + } > return ret; > }
On Thu, Nov 4, 2021 at 7:13 PM Steven Rostedt <rostedt@goodmis.org> wrote: > [ ... ] > > static int remove_hist(struct tracefs_instance *instance, > > struct tep_event *event, const char *hist) > > { > > @@ -1919,7 +1911,6 @@ tracefs_synth_get_start_hist(struct tracefs_synth *synth) > > int tracefs_synth_create(struct tracefs_instance *instance, > > struct tracefs_synth *synth) > > I think we should call this: > > tracefs_synth_create_raw() > > that passes in the "instance". > > Then we could have tracefs_synth_create() just use the top instance (no > instance passed to it), and we could decide later if we want to create an > internal instance or not to do the work to keep from modifying the top > instance. > > That is, we would have: > > int tracefs_synth_create(struct tracefs_synth *synth) > { > return tracefs_synth_create_raw(synth->create_instance, synth); > } > > Have synth->create_instance be defaulted to NULL on allocation, and would > get updated by another interface perhaps? > > We could also add to the _raw() function: > > if (synth->create_instance && instance && > synth->create_instance != instance) { > errno = EINVAL; > return -1 > } > > if (!synth->create_instance && instance) { > syth->create_instance = instance; > trace_get_instance(instance); > } > > Would need to have in the destructor: > > if (synth->create_instance) > trace_put_instance(synth->create_instance); > > > Or something like that. > I looked and tested more this code today, but as I understood the instance is mandatory in histograms creation and should not be removed from this API. That API appends the histogram config in the event's trigger files, which is for specific instances. The histogram is accumulated only in the context of that instance. I'm going to send next version of the patch set, but without changes to this API. > [ ... ] >
On Fri, 5 Nov 2021 14:15:05 +0200 Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote: > I looked and tested more this code today, but as I understood the > instance is mandatory in histograms creation and should not be removed > from this API. That API appends the histogram config in the event's > trigger files, which is for specific instances. The histogram is > accumulated only in the context of that instance. I'm going to send > next version of the patch set, but without changes to this API. The histogram code is internal to the synthetic events. That's because it uses the histogram to save the data across the start to end event to create the synthetic event. The histogram is an implementation detail of the synthetic event and should not be exposed via the API. And that is *exactly* why instances should not be exposed to the synthetic event! In fact, I don't wan the user to have control over the instances because they are likely to screw it up if they don't know what they are doing. That's why I suggested to convert the current API into a raw event, because to use instances, you had better know what you are doing, otherwise you can easily screw it up. The histograms used by synthetic events are "special" and if not done properly, will not do what people expect. If you don't want to make this change, I will. -- Steve
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c index 9009dba..08bb2da 100644 --- a/src/tracefs-hist.c +++ b/src/tracefs-hist.c @@ -661,6 +661,7 @@ struct tracefs_synth { struct tep_event *end_event; struct action *actions; struct action **next_action; + struct tracefs_dynevent *dyn_event; char *name; char **synthetic_fields; char **synthetic_args; @@ -719,6 +720,7 @@ void tracefs_synth_free(struct tracefs_synth *synth) synth->actions = action->next; action_free(action); } + tracefs_dynevent_free(synth->dyn_event); free(synth); } @@ -889,6 +891,28 @@ synth_init_from(struct tep_handle *tep, const char *start_system, return synth; } +static int alloc_synthetic_event(struct tracefs_synth *synth) +{ + char *synthetic_format; + const char *field; + int i; + + synthetic_format = strdup(""); + if (!synthetic_format) + return -1; + + for (i = 0; synth->synthetic_fields && synth->synthetic_fields[i]; i++) { + field = synth->synthetic_fields[i]; + synthetic_format = append_string(synthetic_format, " ", field); + } + + synth->dyn_event = dynevent_alloc(TRACEFS_DYNEVENT_SYNTH, NULL, + synth->name, NULL, synthetic_format); + free(synthetic_format); + + return synth->dyn_event ? 0 : -1; +} + /** * tracefs_synth_alloc - create a new tracefs_synth instance * @tep: The tep handle that holds the events to work on @@ -1609,38 +1633,6 @@ int tracefs_synth_save(struct tracefs_synth *synth, return 0; } -static char *create_synthetic_event(struct tracefs_synth *synth) -{ - char *synthetic_event; - const char *field; - int i; - - synthetic_event = strdup(synth->name); - if (!synthetic_event) - return NULL; - - for (i = 0; synth->synthetic_fields && synth->synthetic_fields[i]; i++) { - field = synth->synthetic_fields[i]; - synthetic_event = append_string(synthetic_event, " ", field); - } - - return synthetic_event; -} - -static int remove_synthetic(const char *synthetic) -{ - char *str; - int ret; - - ret = asprintf(&str, "!%s", synthetic); - if (ret < 0) - return -1; - - ret = tracefs_instance_file_append(NULL, "synthetic_events", str); - free(str); - return ret < 0 ? -1 : 0; -} - static int remove_hist(struct tracefs_instance *instance, struct tep_event *event, const char *hist) { @@ -1919,7 +1911,6 @@ tracefs_synth_get_start_hist(struct tracefs_synth *synth) int tracefs_synth_create(struct tracefs_instance *instance, struct tracefs_synth *synth) { - char *synthetic_event; char *start_hist = NULL; char *end_hist = NULL; int ret; @@ -1937,14 +1928,10 @@ int tracefs_synth_create(struct tracefs_instance *instance, if (verify_state(synth) < 0) return -1; - synthetic_event = create_synthetic_event(synth); - if (!synthetic_event) + if (!synth->dyn_event && alloc_synthetic_event(synth)) + return -1; + if (!tracefs_dynevent_create(synth->dyn_event)) return -1; - - ret = tracefs_instance_file_append(NULL, "synthetic_events", - synthetic_event); - if (ret < 0) - goto free_synthetic; start_hist = create_hist(synth->start_keys, synth->start_vars); start_hist = append_filter(start_hist, synth->start_filter, @@ -1980,9 +1967,7 @@ int tracefs_synth_create(struct tracefs_instance *instance, remove_synthetic: free(end_hist); free(start_hist); - remove_synthetic(synthetic_event); - free_synthetic: - free(synthetic_event); + tracefs_dynevent_destroy(synth->dyn_event, false); return -1; } @@ -2007,7 +1992,6 @@ int tracefs_synth_create(struct tracefs_instance *instance, int tracefs_synth_destroy(struct tracefs_instance *instance, struct tracefs_synth *synth) { - char *synthetic_event; char *hist; int ret; @@ -2041,11 +2025,7 @@ int tracefs_synth_destroy(struct tracefs_instance *instance, ret = remove_hist(instance, synth->start_event, hist); free(hist); - synthetic_event = create_synthetic_event(synth); - if (!synthetic_event) - return -1; - - ret = remove_synthetic(synthetic_event); + ret = tracefs_dynevent_destroy(synth->dyn_event, false); return ret ? -1 : 0; } @@ -2067,7 +2047,7 @@ int tracefs_synth_show(struct trace_seq *seq, struct tracefs_instance *instance, struct tracefs_synth *synth) { - char *synthetic_event = NULL; + bool new_event = false; char *hist = NULL; char *path; int ret = -1; @@ -2082,16 +2062,19 @@ int tracefs_synth_show(struct trace_seq *seq, return -1; } - synthetic_event = create_synthetic_event(synth); - if (!synthetic_event) - return -1; + if (!synth->dyn_event) { + if (alloc_synthetic_event(synth)) + return -1; + new_event = true; + } path = trace_find_tracing_dir(); if (!path) goto out_free; - trace_seq_printf(seq, "echo '%s' > %s/synthetic_events\n", - synthetic_event, path); + trace_seq_printf(seq, "echo '%s%s %s' > %s/%s\n", + synth->dyn_event->prefix, synth->dyn_event->event, + synth->dyn_event->format, path, synth->dyn_event->trace_file); tracefs_put_tracing_file(path); path = tracefs_instance_get_dir(instance); @@ -2116,10 +2099,18 @@ int tracefs_synth_show(struct trace_seq *seq, hist, path, synth->end_event->system, synth->end_event->name); + if (new_event) { + tracefs_dynevent_free(synth->dyn_event); + synth->dyn_event = NULL; + } + ret = 0; out_free: - free(synthetic_event); free(hist); tracefs_put_tracing_file(path); + if (new_event) { + tracefs_dynevent_free(synth->dyn_event); + synth->dyn_event = NULL; + } return ret; }
Synthetic events are type of ftrace dynamic events. The tracefs library has dedicated APIs to manage dynamic events of all types. In order the code to be consistent, the creation of synthetic events inside the library is reimplemented with these new dynamic events APIs. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- src/tracefs-hist.c | 103 +++++++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 56 deletions(-)