Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1254047 > unrolled thread
| Started by | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| First post | 2015-10-22 20:20 +0200 |
| Last post | 2015-10-29 19:40 +0100 |
| Articles | 16 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 00/28] tracing: 'hist' triggers Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:20 +0200
[PATCH v11 16/28] tracing: Add hist trigger 'sym' and 'sym-offset' modifiers Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:20 +0200
[PATCH v11 12/28] tracing: Add hist trigger support for user-defined sorting ('sort=' param) Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:20 +0200
[PATCH v11 01/28] tracing: Update cond flag when enabling or disabling a trigger Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:30 +0200
[PATCH v11 27/28] kselftests/ftrace : Add event trigger testcases Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:30 +0200
[PATCH v11 10/28] tracing: Add hist trigger support for multiple values ('vals=' param) Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:30 +0200
[PATCH v11 06/28] tracing: Add a per-event-trigger 'paused' field Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:30 +0200
[PATCH v11 25/28] tracing: Add support for named triggers Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:30 +0200
[PATCH v11 18/28] tracing: Add hist trigger 'syscall' modifier Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:30 +0200
[PATCH v11 03/28] tracing: Make event trigger functions available Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:30 +0200
[PATCH v11 04/28] tracing: Add event record param to trigger_ops.func() Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:30 +0200
Re: [PATCH 00/28] tracing: 'hist' triggers Steven Rostedt <rostedt@goodmis.org> - 2015-10-22 20:30 +0200
Re: [PATCH 00/28] tracing: 'hist' triggers Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:40 +0200
[PATCH v11 05/28] tracing: Add get_syscall_name() Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-22 20:30 +0200
Re: [PATCH v11 08/28] tracing: Add lock-free tracing_map Namhyung Kim <namhyung@kernel.org> - 2015-10-29 09:40 +0100
Re: [PATCH v11 08/28] tracing: Add lock-free tracing_map Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-10-29 19:40 +0100
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:20 +0200 |
| Subject | [PATCH 00/28] tracing: 'hist' triggers |
| Message-ID | <qmsD0-1Xu-7@gated-at.bofh.it> |
This is v11 of the 'hist triggers' patchset, which is the same as v10
but adds the 2 new ftrace/hist triggers kselftest patches from Masami.
Changes from v10:
- Rebased to latest trace/for-next.
- Added Masami's ftrace/hist triggers kselftest patches.
- Added Masami's Tested-by: to the series.
Changes from v9:
v10 adds some major new features - 'named' hist triggers and support
for multiple hist triggers on a single event - basically all of the
features and suggestions suggested by Masami, except for one: I
decided after all not to implement the 'pause/continue/clear' commands
on the hist file. I've gotten so used to the habit of calling up the
previous trigger-setting command in the command history and simply
appending a 'pause/cont/clear' command to it (and later removing it by
prepending a '!') that making the user change the command doesn't seem
as naturally usable. Masami, if you disagree, let me know and I'll
reconsider it but at this point I'd rather not make that change.
In addition, I think I've fixed all the other various smaller problems
mentioned in the previous review, please let me know if I missed any...
- Added support for 'named' hist triggers
- Added support for multiple hist triggers on a single trace event
- Added documentation and examples for the above new features
- Streamlined the README to only include the essentials for hist triggers
- Fixed the 'hex' modifier for values, which was previously ignored
- Replaced the kmalloc of large arrays with a simple page-based scheme
- Fixed the tracing_map_insert() case where jhash returns 0
- Fixed various other small problems noted along the way
Changes from v8:
Same as v8, but with the RFC patch [ftrace: Add function_hist tracer]
removed, and rebased to latest trace/for-next.
Changes from v7:
This version refactors the commits as suggested by Masami. There are
now more commits, but the result should be much more reviewable. The
ending code is the same as before, modulo a couple minor bug fixes I
discovered while refactoring and testing.
I've also reviewed and fixed a number of shortcomings and errors in
the comments, and have added a new discussion of the tracing_map data
structures after Steve mentioned he found them confusing and/or
insufficiently documented.
Also, I kept Namhyung's string patch [tracing: Support string type key
properly] as submitted, but added a follow-on patch that refactors it
and fixes a problem I found with it that enabled static string keys to
contain random chars and therefore incorrect map insertions.
Changes from v6:
This version adds a new 'sym-offset' modifier as requested by Masami.
I implemented it as a modifier rather than using the trace option as
suggested, in part because I wanted to keep it all self-contained and
it seemed more consistent to just add it alongside the 'sym' modifier.
Also, hist triggers arent't really a tracer and therefore don't
directly tie into the option update/callback mechanism so making use
of it isn't as simple as a normal tracer.
I also changed the sort key specification to be stricter and signal an
error if the specified sort key wasn't found (rather than defaulting
to hitcount in those cases), also suggested by Masami. Thanks,
Masami, for your input!
Also updated the Documentation and tracing/README to reflect the
changes.
Changes from v5:
This version adds support for compound keys, along with the related
ability to sort using primary and secondary keys. This was mentioned
in previous versions as the last important piece that remained
unimplemented, and is now implemented. (I didn't have time to get to
the couple of enhancements suggested by Masami, but I expect to be
able to add those later on top of these.)
Because we now support compound keys and it's not immediately clear in
the output exactly which fields correspond to keys, the key(s),
compound or not, are now enclosed by curly braces.
The Documentation and README have been updated to reflect the changes,
and several new examples have been added to illustrate how to use
compound keys.
Also, the code was updated to work with the new ftrace_event_file,
etc, renaming in tracing/for-next.
Changes from v4:
This version addresses some problems and suggestions made by Daniel
Wagner - a lot of the code was reworked to get rid of the distinction
between keys and values, and as a result, both keys and values can be
used as sort keys. As suggested, it also allows 'val=' to be absent
in a trigger command - if no 'val' is specified, hitcount is assumed
and automatically used as the only val.
The map code was also separated out into a separate file,
tracing_map.c, allowing it to be reused. It also adds a second tracer
called function_hist that actually does reuse the code, as an RFC
patch.
Patch 01/10 [tracing: Update cond flag when enabling or disabling..]
is a fix for a problem noticed by Daniel and that fixes a problem in
existing trigger code and should be applied regardless of whether the
rest of the patchset is merged.
As mentioned, patch 10/10 is an RFC patch implementing a new tracer
based on the function tracer code. It's a fun little tool and is
useful for a specific problem I'm working on (and is also a nice test
of the tracing_map code), but is an RFC because first, I'm not sure it
would really be of general interest and secondly, it's POC-level
quality and I'd need to spend more time fixing it up to make it
upstreamable, but I don't want to waste my time if not.
There are a couple of important bits of functionality that were
present in v1 but not yet reimplemented in v5.
The first is support for compound keys. Currently, maps can only be
keyed on a single event field, whereas in v1 they could be keyed on
multiple keys. With support for compound keys, you can create much
more interesting output, such as for example per-pid lists of
syscalls or read counts e.g.:
# echo 'hist:keys=common_pid.execname,id.syscall:vals=hitcount' > \
/sys/kernel/debug/tracing/events/raw_syscalls/sys_enter/trigger
# cat /sys/kernel/debug/tracing/events/raw_syscalls/sys_enter/hist
key: common_pid:bash[3112], id:sys_write vals: count:69
key: common_pid:bash[3112], id:sys_rt_sigprocmask vals: count:218
key: common_pid:update-notifier[3164], id:sys_poll vals: count:37
key: common_pid:update-notifier[3164], id:sys_recvfrom vals: count:118
key: common_pid:deja-dup-monito[3194], id:sys_sendto vals: count:1
key: common_pid:deja-dup-monito[3194], id:sys_read vals: count:4
key: common_pid:deja-dup-monito[3194], id:sys_poll vals: count:8
key: common_pid:deja-dup-monito[3194], id:sys_recvmsg vals: count:8
key: common_pid:deja-dup-monito[3194], id:sys_getegid vals: count:8
key: common_pid:emacs[3275], id:sys_fsync vals: count:1
key: common_pid:emacs[3275], id:sys_open vals: count:1
key: common_pid:emacs[3275], id:sys_symlink vals: count:2
key: common_pid:emacs[3275], id:sys_poll vals: count:23
key: common_pid:emacs[3275], id:sys_select vals: count:23
key: common_pid:emacs[3275], id:unknown_syscall vals: count:34
key: common_pid:emacs[3275], id:sys_ioctl vals: count:60
key: common_pid:emacs[3275], id:sys_rt_sigprocmask vals: count:116
key: common_pid:cat[3323], id:sys_munmap vals: count:1
key: common_pid:cat[3323], id:sys_fadvise64 vals: count:1
Related to that is support for sorting on multiple fields. Currently,
you can sort using only a primary key. Being able to sort on multiple
or at least a secondary key is indispensible for seeing trends when
displaying multiple values.
Changes from v3:
v4 fixes the race in tracing_map_insert() noted in v3, where
map.val.key could be checked even if map.val wasn't yet set. The
simple fix for that in tracing_map_insert() introduces the possibility
of duplicates in the map, which though rare, need to be accounted for
in the output. To address that, duplicate-merging code was added to
the map-printing code.
It was also pointed out that it didn't seem correct to include
module.h, but the fix for that has deeper roots and is being addressed
by a separate patchset; for now we need to continue including
module.h, though prompted by that I did some other header include
cleanup.
The functionality remains the same as v2, but this version no longer
tries to export and use bpf_maps, and more importantly removes the
associated GFP_NOTRACE/trace event hacks and kmem macros required to
work around the bpf_map implementation.
The tracing_map functionality is instead built on top of a simple
lock-free map algorithm originated by Dr. Cliff Click (see references
in the code for more details), which though too restrictive to be
general-purpose in its current form, functions nicely as a
special-purpose tracing map.
v3 also moves the hist triggers code into a separate file and puts it
all behind a new config option, CONFIG_HIST_TRIGGERS. It also merges
in the sorting code rather than keeping it as a separate patch.
This patchset also includes a couple other new and related triggers,
enable_hist and disable_hist, very similar to the existing
enable_event/disable_event triggers used to automatically enable and
disable events based on a triggering condition, but in this case
allowing hist triggers to be enabled and disabled in the same way.
- Added an insert check for val before checking the key associated with val
- Added code to merge possible duplicates in the map
Changes from v2:
- reimplemented tracing_map, replacing bpf_map with nmi-safe/lock-free map
- removed GPF_NOTRACE, kmalloc/free macros and event hacks needed by bpf_maps
- moved hist triggers from trace_events_trigger.c to trace_events_hist.c
- added CONFIG_HIST_TRIGGERS config option
- consolidated sorting code with main patch
Changes from v1:
- completely rewritten on top of tracing_map (renamed and exported bpf_map)
- added map clearing and client ops to tracing_map
- changed the name from 'hash' triggers to 'hist' triggers
- added new trigger 'pause' feature
- added new enable_hist and disable_hist triggers
- added usage for hist/enable_hist/disable hist to tracing/README
- moved examples into Documentation/trace/event.txt
- added ___GFP_NOTRACE, kmalloc/kfree macros, and conditional kmem tracepoints
The following changes since commit ddd70280bf0e92ad81a9526971409603fba21679:
tracing: gpio: Add Kconfig option for enabling/disabling trace events (2015-10-20 21:56:10 -0400)
are available in the git repository at:
git://git.yoctoproject.org/linux-yocto-contrib.git tzanussi/hist-triggers-v11
http://git.yoctoproject.org/cgit/cgit.cgi/linux-yocto-contrib/log/?h=tzanussi/hist-triggers-v11
Masami Hiramatsu (2):
kselftests/ftrace : Add event trigger testcases
kselftests/ftrace: Add hist trigger testcases
Namhyung Kim (1):
tracing: Support string type key properly
Tom Zanussi (25):
tracing: Update cond flag when enabling or disabling a trigger
tracing: Make ftrace_event_field checking functions available
tracing: Make event trigger functions available
tracing: Add event record param to trigger_ops.func()
tracing: Add get_syscall_name()
tracing: Add a per-event-trigger 'paused' field
tracing: Add needs_rec flag to event triggers
tracing: Add lock-free tracing_map
tracing: Add 'hist' event trigger command
tracing: Add hist trigger support for multiple values ('vals=' param)
tracing: Add hist trigger support for compound keys
tracing: Add hist trigger support for user-defined sorting ('sort='
param)
tracing: Add hist trigger support for pausing and continuing a trace
tracing: Add hist trigger support for clearing a trace
tracing: Add hist trigger 'hex' modifier for displaying numeric fields
tracing: Add hist trigger 'sym' and 'sym-offset' modifiers
tracing: Add hist trigger 'execname' modifier
tracing: Add hist trigger 'syscall' modifier
tracing: Add hist trigger support for stacktraces as keys
tracing: Remove restriction on string position in hist trigger keys
tracing: Add enable_hist/disable_hist triggers
tracing: Add 'hist' trigger Documentation
tracing: Add support for multiple hist triggers per event
tracing: Add support for named triggers
tracing: Add support for named hist triggers
Documentation/trace/events.txt | 1532 ++++++++++++++++++
include/linux/trace_events.h | 9 +-
kernel/trace/Kconfig | 27 +
kernel/trace/Makefile | 2 +
kernel/trace/trace.c | 56 +
kernel/trace/trace.h | 97 +-
kernel/trace/trace_events.c | 4 +
kernel/trace/trace_events_filter.c | 12 -
kernel/trace/trace_events_hist.c | 1656 ++++++++++++++++++++
kernel/trace/trace_events_trigger.c | 296 +++-
kernel/trace/trace_syscalls.c | 11 +
kernel/trace/tracing_map.c | 1005 ++++++++++++
kernel/trace/tracing_map.h | 277 ++++
tools/testing/selftests/ftrace/test.d/functions | 9 +
.../ftrace/test.d/trigger/trigger-eventonoff.tc | 64 +
.../ftrace/test.d/trigger/trigger-filter.tc | 59 +
.../ftrace/test.d/trigger/trigger-hist-mod.tc | 65 +
.../ftrace/test.d/trigger/trigger-hist.tc | 83 +
.../ftrace/test.d/trigger/trigger-multihist.tc | 73 +
.../ftrace/test.d/trigger/trigger-snapshot.tc | 56 +
.../ftrace/test.d/trigger/trigger-stacktrace.tc | 53 +
.../ftrace/test.d/trigger/trigger-traceonoff.tc | 58 +
22 files changed, 5419 insertions(+), 85 deletions(-)
create mode 100644 kernel/trace/trace_events_hist.c
create mode 100644 kernel/trace/tracing_map.c
create mode 100644 kernel/trace/tracing_map.h
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-eventonoff.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-filter.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-snapshot.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-stacktrace.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-traceonoff.tc
--
1.9.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]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:20 +0200 |
| Subject | [PATCH v11 16/28] tracing: Add hist trigger 'sym' and 'sym-offset' modifiers |
| Message-ID | <qmsD2-1Xu-55@gated-at.bofh.it> |
| In reply to | #1254047 |
Allow users to have address fields displayed as symbols in the output
by appending '.sym' or 'sym-offset' to field names:
# echo hist:keys=aaa.sym,bbb.sym-offset ... \
[ if filter] > event/trigger
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
kernel/trace/trace.c | 4 +++-
kernel/trace/trace_events_hist.c | 21 +++++++++++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4373714..4f0968e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3820,7 +3820,9 @@ static const char readme_msg[] =
"\t table in its entirety to stdout. The default format used to\n"
"\t display a given field can be modified by appending any of the\n"
"\t following modifiers to the field name, as applicable:\n\n"
- "\t .hex display a number as a hex value\n\n"
+ "\t .hex display a number as a hex value\n"
+ "\t .sym display an address as a symbol\n"
+ "\t .sym-offset display an address as a symbol and offset\n\n"
"\t The 'pause' parameter can be used to pause an existing hist\n"
"\t trigger or to start a hist trigger but not log any events\n"
"\t until told to do so. 'continue' can be used to start or\n"
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index b4f4934..e6e5c6e 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -73,6 +73,8 @@ enum hist_field_flags {
HIST_FIELD_KEY = 2,
HIST_FIELD_STRING = 4,
HIST_FIELD_HEX = 8,
+ HIST_FIELD_SYM = 16,
+ HIST_FIELD_SYM_OFFSET = 32,
};
struct hist_trigger_attrs {
@@ -370,6 +372,10 @@ static int create_key_field(struct hist_trigger_data *hist_data,
if (field_str) {
if (!strcmp(field_str, "hex"))
flags |= HIST_FIELD_HEX;
+ else if (!strcmp(field_str, "sym"))
+ flags |= HIST_FIELD_SYM;
+ else if (!strcmp(field_str, "sym-offset"))
+ flags |= HIST_FIELD_SYM_OFFSET;
else {
ret = -EINVAL;
goto out;
@@ -700,6 +706,7 @@ hist_trigger_entry_print(struct seq_file *m,
struct tracing_map_elt *elt)
{
struct hist_field *key_field;
+ char str[KSYM_SYMBOL_LEN];
unsigned int i;
u64 uval;
@@ -715,6 +722,16 @@ hist_trigger_entry_print(struct seq_file *m,
uval = *(u64 *)(key + key_field->offset);
seq_printf(m, "%s: %llx",
key_field->field->name, uval);
+ } else if (key_field->flags & HIST_FIELD_SYM) {
+ uval = *(u64 *)(key + key_field->offset);
+ sprint_symbol_no_offset(str, uval);
+ seq_printf(m, "%s: [%llx] %-45s",
+ key_field->field->name, uval, str);
+ } else if (key_field->flags & HIST_FIELD_SYM_OFFSET) {
+ uval = *(u64 *)(key + key_field->offset);
+ sprint_symbol(str, uval);
+ seq_printf(m, "%s: [%llx] %-55s",
+ key_field->field->name, uval, str);
} else if (key_field->flags & HIST_FIELD_STRING) {
seq_printf(m, "%s: %-50s", key_field->field->name,
(char *)(key + key_field->offset));
@@ -830,6 +847,10 @@ static const char *get_hist_field_flags(struct hist_field *hist_field)
if (hist_field->flags & HIST_FIELD_HEX)
flags_str = "hex";
+ else if (hist_field->flags & HIST_FIELD_SYM)
+ flags_str = "sym";
+ else if (hist_field->flags & HIST_FIELD_SYM_OFFSET)
+ flags_str = "sym-offset";
return flags_str;
}
--
1.9.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] | [prev] | [next] | [standalone]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:20 +0200 |
| Subject | [PATCH v11 12/28] tracing: Add hist trigger support for user-defined sorting ('sort=' param) |
| Message-ID | <qmsD2-1Xu-59@gated-at.bofh.it> |
| In reply to | #1254047 |
Allow users to specify keys and/or values to sort on. With this
addition, keys and values specified using the 'keys=' and 'vals='
keywords can be used to sort the hist trigger output via a new 'sort='
keyword. If multiple sort keys are specified, the output will be
sorted using the second key as a secondary sort key, etc. The default
sort order is ascending; if the user wants a different sort order,
'.descending' can be appended to the specific sort key. Before this
addition, output was always sorted by 'hitcount' in ascending order.
This expands the hist trigger syntax from this:
# echo hist:keys=xxx:vals=yyy \
[ if filter] > event/trigger
to this:
# echo hist:keys=xxx:vals=yyy:sort=zzz.descending \
[ if filter] > event/trigger
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
kernel/trace/trace.c | 6 ++-
kernel/trace/trace_events_hist.c | 101 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8180a99..efa8d05 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3800,6 +3800,7 @@ static const char readme_msg[] =
" hist trigger\t- If set, event hits are aggregated into a hash table\n"
"\t Format: hist:keys=<field1>[,field2,...]\n"
"\t [:values=<field1[,field2,...]]\n"
+ "\t [:sort=field1,field2,...]\n"
"\t [:size=#entries]\n"
"\t [if <filter>]\n\n"
"\t When a matching event is hit, an entry is added to a hash\n"
@@ -3808,7 +3809,10 @@ static const char readme_msg[] =
"\t correspond to fields in the event's format description. Keys\n"
"\t can be any field. Compound keys consisting of up to two\n"
"\t fields can be specified by the 'keys' keyword. Values must\n"
- "\t correspond to numeric fields. The 'size' parameter can be\n"
+ "\t correspond to numeric fields. Sort keys consisting of up to\n"
+ "\t two fields can be specified using the 'sort' keyword. The\n"
+ "\t sort direction can be modified by appending '.descending' or\n"
+ "\t '.ascending' to a sort field. The 'size' parameter can be\n"
"\t used to specify more or fewer than the default 2048 entries\n"
"\t for the hashtable size.\n\n"
"\t Reading the 'hist' file for the event will dump the hash\n"
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 112b8a2..4fc3136 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -77,6 +77,7 @@ enum hist_field_flags {
struct hist_trigger_attrs {
char *keys_str;
char *vals_str;
+ char *sort_key_str;
unsigned int map_bits;
};
@@ -156,6 +157,7 @@ static int parse_map_size(char *str)
static void destroy_hist_trigger_attrs(struct hist_trigger_attrs *attrs)
{
+ kfree(attrs->sort_key_str);
kfree(attrs->keys_str);
kfree(attrs->vals_str);
kfree(attrs);
@@ -180,6 +182,8 @@ static struct hist_trigger_attrs *parse_hist_trigger_attrs(char *trigger_str)
!strncmp(str, "vals", strlen("vals")) ||
!strncmp(str, "val", strlen("val")))
attrs->vals_str = kstrdup(str, GFP_KERNEL);
+ else if (!strncmp(str, "sort", strlen("sort")))
+ attrs->sort_key_str = kstrdup(str, GFP_KERNEL);
else if (!strncmp(str, "size", strlen("size"))) {
int map_bits = parse_map_size(str);
@@ -420,12 +424,88 @@ static int create_hist_fields(struct hist_trigger_data *hist_data,
return ret;
}
+static int is_descending(const char *str)
+{
+ if (!str)
+ return 0;
+
+ if (!strcmp(str, "descending"))
+ return 1;
+
+ if (!strcmp(str, "ascending"))
+ return 0;
+
+ return -EINVAL;
+}
+
static int create_sort_keys(struct hist_trigger_data *hist_data)
{
+ char *fields_str = hist_data->attrs->sort_key_str;
+ struct ftrace_event_field *field = NULL;
+ struct tracing_map_sort_key *sort_key;
+ unsigned int i, j;
int ret = 0;
hist_data->n_sort_keys = 1; /* sort_keys[0] is always hitcount */
+ if (!fields_str)
+ goto out;
+
+ strsep(&fields_str, "=");
+ if (!fields_str) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ for (i = 0; i < TRACING_MAP_SORT_KEYS_MAX; i++) {
+ char *field_str, *field_name;
+
+ sort_key = &hist_data->sort_keys[i];
+
+ field_str = strsep(&fields_str, ",");
+ if (!field_str) {
+ if (i == 0)
+ ret = -EINVAL;
+ break;
+ }
+
+ if ((i == TRACING_MAP_SORT_KEYS_MAX - 1) && fields_str) {
+ ret = -EINVAL;
+ break;
+ }
+
+ field_name = strsep(&field_str, ".");
+ if (!field_name) {
+ ret = -EINVAL;
+ break;
+ }
+
+ if (!strcmp(field_name, "hitcount")) {
+ ret = is_descending(field_str);
+ if (ret < 0)
+ break;
+ sort_key->descending = ret;
+ continue;
+ }
+
+ for (j = 1; j < hist_data->n_fields; j++) {
+ field = hist_data->fields[j]->field;
+ if (field && !strcmp(field_name, field->name)) {
+ sort_key->field_idx = j;
+ ret = is_descending(field_str);
+ if (ret < 0)
+ goto out;
+ sort_key->descending = ret;
+ break;
+ }
+ }
+ if (j == hist_data->n_fields) {
+ ret = -EINVAL;
+ break;
+ }
+ }
+ hist_data->n_sort_keys = i;
+ out:
return ret;
}
@@ -737,7 +817,26 @@ static int event_hist_trigger_print(struct seq_file *m,
}
seq_puts(m, ":sort=");
- seq_puts(m, "hitcount");
+
+ for (i = 0; i < hist_data->n_sort_keys; i++) {
+ struct tracing_map_sort_key *sort_key;
+
+ sort_key = &hist_data->sort_keys[i];
+
+ if (i > 0)
+ seq_puts(m, ",");
+
+ if (sort_key->field_idx == HITCOUNT_IDX)
+ seq_puts(m, "hitcount");
+ else {
+ unsigned int idx = sort_key->field_idx;
+
+ hist_field_print(m, hist_data->fields[idx]);
+ }
+
+ if (sort_key->descending)
+ seq_puts(m, ".descending");
+ }
seq_printf(m, ":size=%u", (1 << hist_data->map->map_bits));
--
1.9.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] | [prev] | [next] | [standalone]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:30 +0200 |
| Subject | [PATCH v11 01/28] tracing: Update cond flag when enabling or disabling a trigger |
| Message-ID | <qmsMF-299-1@gated-at.bofh.it> |
| In reply to | #1254047 |
When a trigger is enabled, the cond flag should be set beforehand,
otherwise a trigger that's expecting to process a trace record
(e.g. one with post_trigger set) could be invoked without one.
Likewise a trigger's cond flag should be reset after it's disabled,
not before.
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
kernel/trace/trace_events_trigger.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 42a4009..4d2f3cc 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -543,11 +543,12 @@ static int register_trigger(char *glob, struct event_trigger_ops *ops,
list_add_rcu(&data->list, &file->triggers);
ret++;
+ update_cond_flag(file);
if (trace_event_trigger_enable_disable(file, 1) < 0) {
list_del_rcu(&data->list);
+ update_cond_flag(file);
ret--;
}
- update_cond_flag(file);
out:
return ret;
}
@@ -575,8 +576,8 @@ static void unregister_trigger(char *glob, struct event_trigger_ops *ops,
if (data->cmd_ops->trigger_type == test->cmd_ops->trigger_type) {
unregistered = true;
list_del_rcu(&data->list);
- update_cond_flag(file);
trace_event_trigger_enable_disable(file, 0);
+ update_cond_flag(file);
break;
}
}
@@ -1319,11 +1320,12 @@ static int event_enable_register_trigger(char *glob,
list_add_rcu(&data->list, &file->triggers);
ret++;
+ update_cond_flag(file);
if (trace_event_trigger_enable_disable(file, 1) < 0) {
list_del_rcu(&data->list);
+ update_cond_flag(file);
ret--;
}
- update_cond_flag(file);
out:
return ret;
}
@@ -1344,8 +1346,8 @@ static void event_enable_unregister_trigger(char *glob,
(enable_data->file == test_enable_data->file)) {
unregistered = true;
list_del_rcu(&data->list);
- update_cond_flag(file);
trace_event_trigger_enable_disable(file, 0);
+ update_cond_flag(file);
break;
}
}
--
1.9.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] | [prev] | [next] | [standalone]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:30 +0200 |
| Subject | [PATCH v11 27/28] kselftests/ftrace : Add event trigger testcases |
| Message-ID | <qmsMG-299-7@gated-at.bofh.it> |
| In reply to | #1254047 |
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
This adds simple event trigger testcases for ftracetest,
which covers following triggers.
- traceon-traceoff trigger
- enable/disable_event trigger
- snapshot trigger
- stacktrace trigger
- trigger filters
Here is the test result.
----
# ./ftracetest test.d/trigger/
=== Ftrace unit tests ===
[1] event trigger - test event enable/disable trigger [PASS]
[2] event trigger - test trigger filter [PASS]
[3] event trigger - test snapshot-trigger [PASS]
[4] event trigger - test stacktrace-trigger [PASS]
[5] event trigger - test traceon/off trigger [PASS]
# of passed: 5
# of failed: 0
# of unresolved: 0
# of untested: 0
# of unsupported: 0
# of xfailed: 0
# of undefined(test bug): 0
----
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
---
tools/testing/selftests/ftrace/test.d/functions | 9 +++
.../ftrace/test.d/trigger/trigger-eventonoff.tc | 64 ++++++++++++++++++++++
.../ftrace/test.d/trigger/trigger-filter.tc | 59 ++++++++++++++++++++
.../ftrace/test.d/trigger/trigger-snapshot.tc | 56 +++++++++++++++++++
.../ftrace/test.d/trigger/trigger-stacktrace.tc | 53 ++++++++++++++++++
.../ftrace/test.d/trigger/trigger-traceonoff.tc | 58 ++++++++++++++++++++
6 files changed, 299 insertions(+)
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-eventonoff.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-filter.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-snapshot.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-stacktrace.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-traceonoff.tc
diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
index 5d8cd06..c37262f 100644
--- a/tools/testing/selftests/ftrace/test.d/functions
+++ b/tools/testing/selftests/ftrace/test.d/functions
@@ -14,3 +14,12 @@ enable_tracing() { # start trace recording
reset_tracer() { # reset the current tracer
echo nop > current_tracer
}
+
+reset_trigger() { # reset all current setting triggers
+ grep -v ^# events/*/*/trigger |
+ while read line; do
+ cmd=`echo $line | cut -f2- -d: | cut -f1 -d" "`
+ echo "!$cmd" > `echo $line | cut -f1 -d:`
+ done
+}
+
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-eventonoff.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-eventonoff.tc
new file mode 100644
index 0000000..1a94450
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-eventonoff.tc
@@ -0,0 +1,64 @@
+#!/bin/sh
+# description: event trigger - test event enable/disable trigger
+
+do_reset() {
+ reset_trigger
+ echo > set_event
+ clear_trace
+}
+
+fail() { #msg
+ do_reset
+ echo $1
+ exit $FAIL
+}
+
+if [ ! -f set_event -o ! -d events/sched ]; then
+ echo "event tracing is not supported"
+ exit_unsupported
+fi
+
+if [ ! -f events/sched/sched_process_fork/trigger ]; then
+ echo "event trigger is not supported"
+ exit_unsupported
+fi
+
+reset_tracer
+do_reset
+
+FEATURE=`grep enable_event events/sched/sched_process_fork/trigger`
+if [ -z "$FEATURE" ]; then
+ echo "event enable/disable trigger is not supported"
+ exit_unsupported
+fi
+
+echo "Test enable_event trigger"
+echo 0 > events/sched/sched_switch/enable
+echo 'enable_event:sched:sched_switch' > events/sched/sched_process_fork/trigger
+( echo "forked")
+if [ `cat events/sched/sched_switch/enable` != '1*' ]; then
+ fail "enable_event trigger on sched_process_fork did not work"
+fi
+
+reset_trigger
+
+echo "Test disable_event trigger"
+echo 1 > events/sched/sched_switch/enable
+echo 'disable_event:sched:sched_switch' > events/sched/sched_process_fork/trigger
+( echo "forked")
+if [ `cat events/sched/sched_switch/enable` != '0*' ]; then
+ fail "disable_event trigger on sched_process_fork did not work"
+fi
+
+reset_trigger
+
+echo "Test semantic error for event enable/disable trigger"
+! echo 'enable_event:nogroup:noevent' > events/sched/sched_process_fork/trigger
+! echo 'disable_event+1' > events/sched/sched_process_fork/trigger
+echo 'enable_event:sched:sched_switch' > events/sched/sched_process_fork/trigger
+! echo 'enable_event:sched:sched_switch' > events/sched/sched_process_fork/trigger
+! echo 'disable_event:sched:sched_switch' > events/sched/sched_process_fork/trigger
+
+do_reset
+
+exit 0
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-filter.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-filter.tc
new file mode 100644
index 0000000..514e466
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-filter.tc
@@ -0,0 +1,59 @@
+#!/bin/sh
+# description: event trigger - test trigger filter
+
+do_reset() {
+ reset_trigger
+ echo > set_event
+ clear_trace
+}
+
+fail() { #msg
+ do_reset
+ echo $1
+ exit $FAIL
+}
+
+if [ ! -f set_event -o ! -d events/sched ]; then
+ echo "event tracing is not supported"
+ exit_unsupported
+fi
+
+if [ ! -f events/sched/sched_process_fork/trigger ]; then
+ echo "event trigger is not supported"
+ exit_unsupported
+fi
+
+reset_tracer
+do_reset
+
+echo "Test trigger filter"
+echo 1 > tracing_on
+echo 'traceoff if child_pid == 0' > events/sched/sched_process_fork/trigger
+( echo "forked")
+if [ `cat tracing_on` -ne 1 ]; then
+ fail "traceoff trigger on sched_process_fork did not work"
+fi
+
+reset_trigger
+
+echo "Test semantic error for trigger filter"
+! echo 'traceoff if a' > events/sched/sched_process_fork/trigger
+! echo 'traceoff if common_pid=0' > events/sched/sched_process_fork/trigger
+! echo 'traceoff if common_pid==b' > events/sched/sched_process_fork/trigger
+echo 'traceoff if common_pid == 0' > events/sched/sched_process_fork/trigger
+echo '!traceoff' > events/sched/sched_process_fork/trigger
+! echo 'traceoff if common_pid == child_pid' > events/sched/sched_process_fork/trigger
+echo 'traceoff if common_pid <= 0' > events/sched/sched_process_fork/trigger
+echo '!traceoff' > events/sched/sched_process_fork/trigger
+echo 'traceoff if common_pid >= 0' > events/sched/sched_process_fork/trigger
+echo '!traceoff' > events/sched/sched_process_fork/trigger
+echo 'traceoff if parent_pid >= 0 && child_pid >= 0' > events/sched/sched_process_fork/trigger
+echo '!traceoff' > events/sched/sched_process_fork/trigger
+echo 'traceoff if parent_pid >= 0 || child_pid >= 0' > events/sched/sched_process_fork/trigger
+echo '!traceoff' > events/sched/sched_process_fork/trigger
+
+
+
+do_reset
+
+exit 0
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-snapshot.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-snapshot.tc
new file mode 100644
index 0000000..f84b80d
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-snapshot.tc
@@ -0,0 +1,56 @@
+#!/bin/sh
+# description: event trigger - test snapshot-trigger
+
+do_reset() {
+ reset_trigger
+ echo > set_event
+ clear_trace
+}
+
+fail() { #msg
+ do_reset
+ echo $1
+ exit $FAIL
+}
+
+if [ ! -f set_event -o ! -d events/sched ]; then
+ echo "event tracing is not supported"
+ exit_unsupported
+fi
+
+if [ ! -f events/sched/sched_process_fork/trigger ]; then
+ echo "event trigger is not supported"
+ exit_unsupported
+fi
+
+reset_tracer
+do_reset
+
+FEATURE=`grep snapshot events/sched/sched_process_fork/trigger`
+if [ -z "$FEATURE" ]; then
+ echo "snapshot trigger is not supported"
+ exit_unsupported
+fi
+
+echo "Test snapshot tigger"
+echo 0 > snapshot
+echo 1 > events/sched/sched_process_fork/enable
+( echo "forked")
+echo 'snapshot:1' > events/sched/sched_process_fork/trigger
+( echo "forked")
+grep sched_process_fork snapshot > /dev/null || \
+ fail "snapshot trigger on sched_process_fork did not work"
+
+reset_trigger
+echo 0 > snapshot
+echo 0 > events/sched/sched_process_fork/enable
+
+echo "Test snapshot semantic errors"
+
+! echo "snapshot+1" > events/sched/sched_process_fork/trigger
+echo "snapshot" > events/sched/sched_process_fork/trigger
+! echo "snapshot" > events/sched/sched_process_fork/trigger
+
+do_reset
+
+exit 0
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-stacktrace.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-stacktrace.tc
new file mode 100644
index 0000000..9fa23b0
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-stacktrace.tc
@@ -0,0 +1,53 @@
+#!/bin/sh
+# description: event trigger - test stacktrace-trigger
+
+do_reset() {
+ reset_trigger
+ echo > set_event
+ clear_trace
+}
+
+fail() { #msg
+ do_reset
+ echo $1
+ exit $FAIL
+}
+
+if [ ! -f set_event -o ! -d events/sched ]; then
+ echo "event tracing is not supported"
+ exit_unsupported
+fi
+
+if [ ! -f events/sched/sched_process_fork/trigger ]; then
+ echo "event trigger is not supported"
+ exit_unsupported
+fi
+
+reset_tracer
+do_reset
+
+FEATURE=`grep stacktrace events/sched/sched_process_fork/trigger`
+if [ -z "$FEATURE" ]; then
+ echo "stacktrace trigger is not supported"
+ exit_unsupported
+fi
+
+echo "Test stacktrace tigger"
+echo 0 > trace
+echo 0 > options/stacktrace
+echo 'stacktrace' > events/sched/sched_process_fork/trigger
+( echo "forked")
+grep "<stack trace>" trace > /dev/null || \
+ fail "stacktrace trigger on sched_process_fork did not work"
+
+reset_trigger
+
+echo "Test stacktrace semantic errors"
+
+! echo "stacktrace:foo" > events/sched/sched_process_fork/trigger
+echo "stacktrace" > events/sched/sched_process_fork/trigger
+! echo "stacktrace" > events/sched/sched_process_fork/trigger
+
+do_reset
+
+exit 0
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-traceonoff.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-traceonoff.tc
new file mode 100644
index 0000000..87648e5
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-traceonoff.tc
@@ -0,0 +1,58 @@
+#!/bin/sh
+# description: event trigger - test traceon/off trigger
+
+do_reset() {
+ reset_trigger
+ echo > set_event
+ clear_trace
+}
+
+fail() { #msg
+ do_reset
+ echo $1
+ exit $FAIL
+}
+
+if [ ! -f set_event -o ! -d events/sched ]; then
+ echo "event tracing is not supported"
+ exit_unsupported
+fi
+
+if [ ! -f events/sched/sched_process_fork/trigger ]; then
+ echo "event trigger is not supported"
+ exit_unsupported
+fi
+
+reset_tracer
+do_reset
+
+echo "Test traceoff trigger"
+echo 1 > tracing_on
+echo 'traceoff' > events/sched/sched_process_fork/trigger
+( echo "forked")
+if [ `cat tracing_on` -ne 0 ]; then
+ fail "traceoff trigger on sched_process_fork did not work"
+fi
+
+reset_trigger
+
+echo "Test traceon trigger"
+echo 0 > tracing_on
+echo 'traceon' > events/sched/sched_process_fork/trigger
+( echo "forked")
+if [ `cat tracing_on` -ne 1 ]; then
+ fail "traceoff trigger on sched_process_fork did not work"
+fi
+
+reset_trigger
+
+echo "Test semantic error for traceoff/on trigger"
+! echo 'traceoff:badparam' > events/sched/sched_process_fork/trigger
+! echo 'traceoff+0' > events/sched/sched_process_fork/trigger
+echo 'traceon' > events/sched/sched_process_fork/trigger
+! echo 'traceon' > events/sched/sched_process_fork/trigger
+! echo 'traceoff' > events/sched/sched_process_fork/trigger
+
+do_reset
+
+exit 0
--
1.9.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] | [prev] | [next] | [standalone]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:30 +0200 |
| Subject | [PATCH v11 10/28] tracing: Add hist trigger support for multiple values ('vals=' param) |
| Message-ID | <qmsMG-299-9@gated-at.bofh.it> |
| In reply to | #1254047 |
Allow users to specify trace event fields to use in aggregated sums
via a new 'vals=' keyword. Before this addition, the only aggregated
sum supported was the implied value 'hitcount'. With this addition,
'hitcount' is also supported as an explicit value field, as is any
numeric trace event field.
This expands the hist trigger syntax from this:
# echo hist:keys=xxx [ if filter] > event/trigger
to this:
# echo hist:keys=xxx:vals=yyy [ if filter] > event/trigger
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
kernel/trace/trace.c | 12 ++++---
kernel/trace/trace_events_hist.c | 75 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 81 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6d5870b..98817d5 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3799,14 +3799,16 @@ static const char readme_msg[] =
#ifdef CONFIG_HIST_TRIGGERS
" hist trigger\t- If set, event hits are aggregated into a hash table\n"
"\t Format: hist:keys=<field1>\n"
+ "\t [:values=<field1[,field2,...]]\n"
"\t [:size=#entries]\n"
"\t [if <filter>]\n\n"
"\t When a matching event is hit, an entry is added to a hash\n"
- "\t table using the key named, and the value of a sum called\n"
- "\t 'hitcount' is incremented. Keys correspond to fields in the\n"
- "\t event's format description. Keys can be any field. The\n"
- "\t 'size' parameter can be used to specify more or fewer than\n"
- "\t the default 2048 entries for the hashtable size.\n\n"
+ "\t table using the key(s) and value(s) named, and the value of a\n"
+ "\t sum called 'hitcount' is incremented. Keys and values\n"
+ "\t correspond to fields in the event's format description. Keys\n"
+ "\t can be any field. Values must correspond to numeric fields.\n"
+ "\t The 'size' parameter can be used to specify more or fewer\n"
+ "\t than the default 2048 entries for the hashtable size.\n\n"
"\t Reading the 'hist' file for the event will dump the hash\n"
"\t table in its entirety to stdout."
#endif
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 376626b..efa62b3 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -75,6 +75,7 @@ enum hist_field_flags {
struct hist_trigger_attrs {
char *keys_str;
+ char *vals_str;
unsigned int map_bits;
};
@@ -155,6 +156,7 @@ static int parse_map_size(char *str)
static void destroy_hist_trigger_attrs(struct hist_trigger_attrs *attrs)
{
kfree(attrs->keys_str);
+ kfree(attrs->vals_str);
kfree(attrs);
}
@@ -173,6 +175,10 @@ static struct hist_trigger_attrs *parse_hist_trigger_attrs(char *trigger_str)
if (!strncmp(str, "keys", strlen("keys")) ||
!strncmp(str, "key", strlen("key")))
attrs->keys_str = kstrdup(str, GFP_KERNEL);
+ else if (!strncmp(str, "values", strlen("values")) ||
+ !strncmp(str, "vals", strlen("vals")) ||
+ !strncmp(str, "val", strlen("val")))
+ attrs->vals_str = kstrdup(str, GFP_KERNEL);
else if (!strncmp(str, "size", strlen("size"))) {
int map_bits = parse_map_size(str);
@@ -256,13 +262,66 @@ static int create_hitcount_val(struct hist_trigger_data *hist_data)
return 0;
}
+static int create_val_field(struct hist_trigger_data *hist_data,
+ unsigned int val_idx,
+ struct trace_event_file *file,
+ char *field_str)
+{
+ struct ftrace_event_field *field = NULL;
+ unsigned long flags = 0;
+ int ret = 0;
+
+ field = trace_find_event_field(file->event_call, field_str);
+ if (!field) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ hist_data->fields[val_idx] = create_hist_field(field, flags);
+ if (!hist_data->fields[val_idx]) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ hist_data->n_vals++;
+ out:
+ return ret;
+}
+
static int create_val_fields(struct hist_trigger_data *hist_data,
struct trace_event_file *file)
{
+ unsigned int vals_max = TRACING_MAP_FIELDS_MAX - TRACING_MAP_KEYS_MAX;
+ char *fields_str, *field_str;
+ unsigned int i, j;
int ret;
ret = create_hitcount_val(hist_data);
+ if (ret)
+ goto out;
+ fields_str = hist_data->attrs->vals_str;
+ if (!fields_str)
+ goto out;
+
+ strsep(&fields_str, "=");
+ if (!fields_str)
+ goto out;
+
+ vals_max = TRACING_MAP_FIELDS_MAX - TRACING_MAP_KEYS_MAX;
+
+ for (i = 0, j = 1; i < vals_max; i++) {
+ field_str = strsep(&fields_str, ",");
+ if (!field_str)
+ break;
+ if (!strcmp(field_str, "hitcount"))
+ continue;
+ ret = create_val_field(hist_data, j++, file, field_str);
+ if (ret)
+ goto out;
+ }
+ if (fields_str)
+ ret = -EINVAL;
+ out:
return ret;
}
@@ -534,6 +593,12 @@ hist_trigger_entry_print(struct seq_file *m,
seq_printf(m, " hitcount: %10llu",
tracing_map_read_sum(elt, HITCOUNT_IDX));
+ for (i = 1; i < hist_data->n_vals; i++) {
+ seq_printf(m, " %s: %10llu",
+ hist_data->fields[i]->field->name,
+ tracing_map_read_sum(elt, i));
+ }
+
seq_puts(m, "\n");
}
@@ -641,7 +706,15 @@ static int event_hist_trigger_print(struct seq_file *m,
}
seq_puts(m, ":vals=");
- seq_puts(m, "hitcount");
+
+ for (i = 0; i < hist_data->n_vals; i++) {
+ if (i == 0)
+ seq_puts(m, "hitcount");
+ else {
+ seq_puts(m, ",");
+ hist_field_print(m, hist_data->fields[i]);
+ }
+ }
seq_puts(m, ":sort=");
seq_puts(m, "hitcount");
--
1.9.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] | [prev] | [next] | [standalone]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:30 +0200 |
| Subject | [PATCH v11 06/28] tracing: Add a per-event-trigger 'paused' field |
| Message-ID | <qmsMG-299-23@gated-at.bofh.it> |
| In reply to | #1254047 |
Add a simple per-trigger 'paused' flag, allowing individual triggers
to pause. We could leave it to individual triggers that need this
functionality to do it themselves, but we also want to allow other
events to control pausing, so add it to the trigger data.
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
kernel/trace/trace.h | 1 +
kernel/trace/trace_events_trigger.c | 4 ++++
2 files changed, 5 insertions(+)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 892335a..08d9654 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1168,6 +1168,7 @@ struct event_trigger_data {
struct event_filter __rcu *filter;
char *filter_str;
void *private_data;
+ bool paused;
struct list_head list;
};
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index c3abef6..8375d29 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -72,6 +72,8 @@ event_triggers_call(struct trace_event_file *file, void *rec)
return tt;
list_for_each_entry_rcu(data, &file->triggers, list) {
+ if (data->paused)
+ continue;
if (!rec) {
data->ops->func(data, rec);
continue;
@@ -110,6 +112,8 @@ event_triggers_post_call(struct trace_event_file *file,
struct event_trigger_data *data;
list_for_each_entry_rcu(data, &file->triggers, list) {
+ if (data->paused)
+ continue;
if (data->cmd_ops->trigger_type & tt)
data->ops->func(data, rec);
}
--
1.9.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] | [prev] | [next] | [standalone]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:30 +0200 |
| Subject | [PATCH v11 25/28] tracing: Add support for named triggers |
| Message-ID | <qmsMG-299-19@gated-at.bofh.it> |
| In reply to | #1254047 |
Named triggers are sets of triggers that share a common set of trigger
data. An example of functionality that could benefit from this type
of capability would be a set of inlined probes that would each
contribute event counts, for example, to a shared counter data
structure.
The first named trigger registered with a given name owns the common
trigger data that the others subsequently registered with the same
name will reference. The functions defined here allow users to add,
delete, and find named triggers.
It also adds functions to pause and unpause named triggers; since
named triggers act upon common data, they should also be paused and
unpaused as a group.
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
kernel/trace/trace.h | 13 ++++
kernel/trace/trace_events_trigger.c | 143 ++++++++++++++++++++++++++++++++++++
2 files changed, 156 insertions(+)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index bbc091a..f8bb42d 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1178,7 +1178,11 @@ struct event_trigger_data {
char *filter_str;
void *private_data;
bool paused;
+ bool paused_tmp;
struct list_head list;
+ char *name;
+ struct list_head named_list;
+ struct event_trigger_data *named_data;
};
/* Avoid typos */
@@ -1221,6 +1225,15 @@ extern void unregister_trigger(char *glob, struct event_trigger_ops *ops,
extern int set_trigger_filter(char *filter_str,
struct event_trigger_data *trigger_data,
struct trace_event_file *file);
+extern struct event_trigger_data *find_named_trigger(const char *name);
+extern bool is_named_trigger(struct event_trigger_data *test);
+extern int save_named_trigger(const char *name,
+ struct event_trigger_data *data);
+extern void del_named_trigger(struct event_trigger_data *data);
+extern void pause_named_trigger(struct event_trigger_data *data);
+extern void unpause_named_trigger(struct event_trigger_data *data);
+extern void set_named_trigger_data(struct event_trigger_data *data,
+ struct event_trigger_data *named_data);
extern int register_event_command(struct event_command *cmd);
extern int unregister_event_command(struct event_command *cmd);
extern int register_trigger_hist_enable_disable_cmds(void);
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 6a28cd2..59e2a98 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -633,6 +633,7 @@ event_trigger_callback(struct event_command *cmd_ops,
trigger_data->ops = trigger_ops;
trigger_data->cmd_ops = cmd_ops;
INIT_LIST_HEAD(&trigger_data->list);
+ INIT_LIST_HEAD(&trigger_data->named_list);
if (glob[0] == '!') {
cmd_ops->unreg(glob+1, trigger_ops, trigger_data, file);
@@ -756,6 +757,148 @@ int set_trigger_filter(char *filter_str,
return ret;
}
+static LIST_HEAD(named_triggers);
+
+/**
+ * find_named_trigger - Find the common named trigger associated with @name
+ * @name: The name of the set of named triggers to find the common data for
+ *
+ * Named triggers are sets of triggers that share a common set of
+ * trigger data. The first named trigger registered with a given name
+ * owns the common trigger data that the others subsequently
+ * registered with the same name will reference. This function
+ * returns the common trigger data associated with that first
+ * registered instance.
+ *
+ * Return: the common trigger data for the given named trigger on
+ * success, NULL otherwise.
+ */
+struct event_trigger_data *find_named_trigger(const char *name)
+{
+ struct event_trigger_data *data;
+
+ if (!name)
+ return NULL;
+
+ list_for_each_entry(data, &named_triggers, named_list) {
+ if (data->named_data)
+ continue;
+ if (!strcmp(data->name, name))
+ return data;
+ }
+
+ return NULL;
+}
+
+/**
+ * is_named_trigger - determine if a given trigger is a named trigger
+ * @test: The trigger data to test
+ *
+ * Return: true if 'test' is a named trigger, false otherwise.
+ */
+bool is_named_trigger(struct event_trigger_data *test)
+{
+ struct event_trigger_data *data;
+
+ list_for_each_entry(data, &named_triggers, named_list) {
+ if (test == data)
+ return true;
+ }
+
+ return false;
+}
+
+/**
+ * save_named_trigger - save the trigger in the named trigger list
+ * @name: The name of the named trigger set
+ * @data: The trigger data to save
+ *
+ * Return: 0 if successful, negative error otherwise.
+ */
+int save_named_trigger(const char *name, struct event_trigger_data *data)
+{
+ data->name = kstrdup(name, GFP_KERNEL);
+ if (!data->name)
+ return -ENOMEM;
+
+ list_add(&data->named_list, &named_triggers);
+
+ return 0;
+}
+
+/**
+ * del_named_trigger - delete a trigger from the named trigger list
+ * @data: The trigger data to delete
+ */
+void del_named_trigger(struct event_trigger_data *data)
+{
+ kfree(data->name);
+ data->name = NULL;
+
+ list_del(&data->named_list);
+}
+
+static void __pause_named_trigger(struct event_trigger_data *data, bool pause)
+{
+ struct event_trigger_data *test;
+
+ list_for_each_entry(test, &named_triggers, named_list) {
+ if (!strcmp(test->name, data->name)) {
+ if (pause) {
+ test->paused_tmp = test->paused;
+ test->paused = true;
+ } else {
+ test->paused = test->paused_tmp;
+ }
+ }
+ }
+}
+
+/**
+ * pause_named_trigger - Pause all named triggers with the same name
+ * @data: The trigger data of a named trigger to pause
+ *
+ * Pauses a named trigger along with all other triggers having the
+ * same name. Because named triggers share a common set of data,
+ * pausing only one is meaningless, so pausing one named trigger needs
+ * to pause all triggers with the same name.
+ */
+void pause_named_trigger(struct event_trigger_data *data)
+{
+ __pause_named_trigger(data, true);
+}
+
+/**
+ * unpause_named_trigger - Un-pause all named triggers with the same name
+ * @data: The trigger data of a named trigger to unpause
+ *
+ * Un-pauses a named trigger along with all other triggers having the
+ * same name. Because named triggers share a common set of data,
+ * unpausing only one is meaningless, so unpausing one named trigger
+ * needs to unpause all triggers with the same name.
+ */
+void unpause_named_trigger(struct event_trigger_data *data)
+{
+ __pause_named_trigger(data, false);
+}
+
+/**
+ * set_named_trigger_data - Associate common named trigger data
+ * @data: The trigger data of a named trigger to unpause
+ *
+ * Named triggers are sets of triggers that share a common set of
+ * trigger data. The first named trigger registered with a given name
+ * owns the common trigger data that the others subsequently
+ * registered with the same name will reference. This function
+ * associates the common trigger data from the first trigger with the
+ * given trigger.
+ */
+void set_named_trigger_data(struct event_trigger_data *data,
+ struct event_trigger_data *named_data)
+{
+ data->named_data = named_data;
+}
+
static void
traceon_trigger(struct event_trigger_data *data, void *rec)
{
--
1.9.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] | [prev] | [next] | [standalone]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:30 +0200 |
| Subject | [PATCH v11 18/28] tracing: Add hist trigger 'syscall' modifier |
| Message-ID | <qmsMG-299-21@gated-at.bofh.it> |
| In reply to | #1254047 |
Allow users to have syscall id fields displayed as syscall names in
the output by appending '.syscall' to field names:
# echo hist:keys=aaa.syscall ... \
[ if filter] > event/trigger
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
kernel/trace/trace.c | 3 ++-
kernel/trace/trace_events_hist.c | 15 +++++++++++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index a143be0..5d9d193 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3823,7 +3823,8 @@ static const char readme_msg[] =
"\t .hex display a number as a hex value\n"
"\t .sym display an address as a symbol\n"
"\t .sym-offset display an address as a symbol and offset\n"
- "\t .execname display a common_pid as a program name\n\n"
+ "\t .execname display a common_pid as a program name\n"
+ "\t .syscall display a syscall id as a syscall name\n\n"
"\t The 'pause' parameter can be used to pause an existing hist\n"
"\t trigger or to start a hist trigger but not log any events\n"
"\t until told to do so. 'continue' can be used to start or\n"
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 656973a..b514719 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -76,6 +76,7 @@ enum hist_field_flags {
HIST_FIELD_SYM = 16,
HIST_FIELD_SYM_OFFSET = 32,
HIST_FIELD_EXECNAME = 64,
+ HIST_FIELD_SYSCALL = 128,
};
struct hist_trigger_attrs {
@@ -452,6 +453,8 @@ static int create_key_field(struct hist_trigger_data *hist_data,
else if (!strcmp(field_str, "execname") &&
!strcmp(field_name, "common_pid"))
flags |= HIST_FIELD_EXECNAME;
+ else if (!strcmp(field_str, "syscall"))
+ flags |= HIST_FIELD_SYSCALL;
else {
ret = -EINVAL;
goto out;
@@ -814,6 +817,16 @@ hist_trigger_entry_print(struct seq_file *m,
uval = *(u64 *)(key + key_field->offset);
seq_printf(m, "%s: %-16s[%10llu]",
key_field->field->name, comm, uval);
+ } else if (key_field->flags & HIST_FIELD_SYSCALL) {
+ const char *syscall_name;
+
+ uval = *(u64 *)(key + key_field->offset);
+ syscall_name = get_syscall_name(uval);
+ if (!syscall_name)
+ syscall_name = "unknown_syscall";
+
+ seq_printf(m, "%s: %-30s[%3llu]",
+ key_field->field->name, syscall_name, uval);
} else if (key_field->flags & HIST_FIELD_STRING) {
seq_printf(m, "%s: %-50s", key_field->field->name,
(char *)(key + key_field->offset));
@@ -935,6 +948,8 @@ static const char *get_hist_field_flags(struct hist_field *hist_field)
flags_str = "sym-offset";
else if (hist_field->flags & HIST_FIELD_EXECNAME)
flags_str = "execname";
+ else if (hist_field->flags & HIST_FIELD_SYSCALL)
+ flags_str = "syscall";
return flags_str;
}
--
1.9.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] | [prev] | [next] | [standalone]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:30 +0200 |
| Subject | [PATCH v11 03/28] tracing: Make event trigger functions available |
| Message-ID | <qmsMG-299-25@gated-at.bofh.it> |
| In reply to | #1254047 |
Make various event trigger utility functions available outside of
trace_events_trigger.c so that new triggers can be defined outside of
that file.
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
kernel/trace/trace.h | 14 ++++++++++++++
kernel/trace/trace_events_trigger.c | 28 +++++++++++++---------------
2 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 72ce78d..f88e104 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1171,6 +1171,20 @@ struct event_trigger_data {
struct list_head list;
};
+extern void trigger_data_free(struct event_trigger_data *data);
+extern int event_trigger_init(struct event_trigger_ops *ops,
+ struct event_trigger_data *data);
+extern int trace_event_trigger_enable_disable(struct trace_event_file *file,
+ int trigger_enable);
+extern void update_cond_flag(struct trace_event_file *file);
+extern void unregister_trigger(char *glob, struct event_trigger_ops *ops,
+ struct event_trigger_data *test,
+ struct trace_event_file *file);
+extern int set_trigger_filter(char *filter_str,
+ struct event_trigger_data *trigger_data,
+ struct trace_event_file *file);
+extern int register_event_command(struct event_command *cmd);
+
/**
* struct event_trigger_ops - callbacks for trace event triggers
*
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 4d2f3cc..6087052 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -28,8 +28,7 @@
static LIST_HEAD(trigger_commands);
static DEFINE_MUTEX(trigger_cmd_mutex);
-static void
-trigger_data_free(struct event_trigger_data *data)
+void trigger_data_free(struct event_trigger_data *data)
{
if (data->cmd_ops->set_filter)
data->cmd_ops->set_filter(NULL, data, NULL);
@@ -311,7 +310,7 @@ const struct file_operations event_trigger_fops = {
* Currently we only register event commands from __init, so mark this
* __init too.
*/
-static __init int register_event_command(struct event_command *cmd)
+__init int register_event_command(struct event_command *cmd)
{
struct event_command *p;
int ret = 0;
@@ -400,9 +399,8 @@ event_trigger_print(const char *name, struct seq_file *m,
*
* Return: 0 on success, errno otherwise
*/
-static int
-event_trigger_init(struct event_trigger_ops *ops,
- struct event_trigger_data *data)
+int event_trigger_init(struct event_trigger_ops *ops,
+ struct event_trigger_data *data)
{
data->ref++;
return 0;
@@ -430,8 +428,8 @@ event_trigger_free(struct event_trigger_ops *ops,
trigger_data_free(data);
}
-static int trace_event_trigger_enable_disable(struct trace_event_file *file,
- int trigger_enable)
+int trace_event_trigger_enable_disable(struct trace_event_file *file,
+ int trigger_enable)
{
int ret = 0;
@@ -488,7 +486,7 @@ clear_event_triggers(struct trace_array *tr)
* its TRIGGER_COND bit set, otherwise the TRIGGER_COND bit should be
* cleared.
*/
-static void update_cond_flag(struct trace_event_file *file)
+void update_cond_flag(struct trace_event_file *file)
{
struct event_trigger_data *data;
bool set_cond = false;
@@ -565,9 +563,9 @@ out:
* Usually used directly as the @unreg method in event command
* implementations.
*/
-static void unregister_trigger(char *glob, struct event_trigger_ops *ops,
- struct event_trigger_data *test,
- struct trace_event_file *file)
+void unregister_trigger(char *glob, struct event_trigger_ops *ops,
+ struct event_trigger_data *test,
+ struct trace_event_file *file)
{
struct event_trigger_data *data;
bool unregistered = false;
@@ -701,9 +699,9 @@ event_trigger_callback(struct event_command *cmd_ops,
*
* Return: 0 on success, errno otherwise
*/
-static int set_trigger_filter(char *filter_str,
- struct event_trigger_data *trigger_data,
- struct trace_event_file *file)
+int set_trigger_filter(char *filter_str,
+ struct event_trigger_data *trigger_data,
+ struct trace_event_file *file)
{
struct event_trigger_data *data = trigger_data;
struct event_filter *filter = NULL, *tmp;
--
1.9.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] | [prev] | [next] | [standalone]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:30 +0200 |
| Subject | [PATCH v11 04/28] tracing: Add event record param to trigger_ops.func() |
| Message-ID | <qmsMH-299-33@gated-at.bofh.it> |
| In reply to | #1254047 |
Some triggers may need access to the trace event, so pass it in. Also
fix up the existing trigger funcs and their callers.
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
include/linux/trace_events.h | 7 ++++---
kernel/trace/trace.h | 6 ++++--
kernel/trace/trace_events_trigger.c | 36 +++++++++++++++++++-----------------
3 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index f85693b..b1e938d 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -427,7 +427,8 @@ extern int call_filter_check_discard(struct trace_event_call *call, void *rec,
extern enum event_trigger_type event_triggers_call(struct trace_event_file *file,
void *rec);
extern void event_triggers_post_call(struct trace_event_file *file,
- enum event_trigger_type tt);
+ enum event_trigger_type tt,
+ void *rec);
/**
* trace_trigger_soft_disabled - do triggers and test if soft disabled
@@ -510,7 +511,7 @@ event_trigger_unlock_commit(struct trace_event_file *file,
trace_buffer_unlock_commit(file->tr, buffer, event, irq_flags, pc);
if (tt)
- event_triggers_post_call(file, tt);
+ event_triggers_post_call(file, tt, entry);
}
/**
@@ -543,7 +544,7 @@ event_trigger_unlock_commit_regs(struct trace_event_file *file,
irq_flags, pc, regs);
if (tt)
- event_triggers_post_call(file, tt);
+ event_triggers_post_call(file, tt, entry);
}
#ifdef CONFIG_BPF_EVENTS
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index f88e104..bc039bc 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1197,7 +1197,8 @@ extern int register_event_command(struct event_command *cmd);
* @func: The trigger 'probe' function called when the triggering
* event occurs. The data passed into this callback is the data
* that was supplied to the event_command @reg() function that
- * registered the trigger (see struct event_command).
+ * registered the trigger (see struct event_command) along with
+ * the trace record, rec.
*
* @init: An optional initialization function called for the trigger
* when the trigger is registered (via the event_command reg()
@@ -1222,7 +1223,8 @@ extern int register_event_command(struct event_command *cmd);
* (see trace_event_triggers.c).
*/
struct event_trigger_ops {
- void (*func)(struct event_trigger_data *data);
+ void (*func)(struct event_trigger_data *data,
+ void *rec);
int (*init)(struct event_trigger_ops *ops,
struct event_trigger_data *data);
void (*free)(struct event_trigger_ops *ops,
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 6087052..c3abef6 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -73,7 +73,7 @@ event_triggers_call(struct trace_event_file *file, void *rec)
list_for_each_entry_rcu(data, &file->triggers, list) {
if (!rec) {
- data->ops->func(data);
+ data->ops->func(data, rec);
continue;
}
filter = rcu_dereference_sched(data->filter);
@@ -83,7 +83,7 @@ event_triggers_call(struct trace_event_file *file, void *rec)
tt |= data->cmd_ops->trigger_type;
continue;
}
- data->ops->func(data);
+ data->ops->func(data, rec);
}
return tt;
}
@@ -93,6 +93,7 @@ EXPORT_SYMBOL_GPL(event_triggers_call);
* event_triggers_post_call - Call 'post_triggers' for a trace event
* @file: The trace_event_file associated with the event
* @tt: enum event_trigger_type containing a set bit for each trigger to invoke
+ * @rec: The trace entry for the event
*
* For each trigger associated with an event, invoke the trigger
* function registered with the associated trigger command, if the
@@ -103,13 +104,14 @@ EXPORT_SYMBOL_GPL(event_triggers_call);
*/
void
event_triggers_post_call(struct trace_event_file *file,
- enum event_trigger_type tt)
+ enum event_trigger_type tt,
+ void *rec)
{
struct event_trigger_data *data;
list_for_each_entry_rcu(data, &file->triggers, list) {
if (data->cmd_ops->trigger_type & tt)
- data->ops->func(data);
+ data->ops->func(data, rec);
}
}
EXPORT_SYMBOL_GPL(event_triggers_post_call);
@@ -750,7 +752,7 @@ int set_trigger_filter(char *filter_str,
}
static void
-traceon_trigger(struct event_trigger_data *data)
+traceon_trigger(struct event_trigger_data *data, void *rec)
{
if (tracing_is_on())
return;
@@ -759,7 +761,7 @@ traceon_trigger(struct event_trigger_data *data)
}
static void
-traceon_count_trigger(struct event_trigger_data *data)
+traceon_count_trigger(struct event_trigger_data *data, void *rec)
{
if (tracing_is_on())
return;
@@ -774,7 +776,7 @@ traceon_count_trigger(struct event_trigger_data *data)
}
static void
-traceoff_trigger(struct event_trigger_data *data)
+traceoff_trigger(struct event_trigger_data *data, void *rec)
{
if (!tracing_is_on())
return;
@@ -783,7 +785,7 @@ traceoff_trigger(struct event_trigger_data *data)
}
static void
-traceoff_count_trigger(struct event_trigger_data *data)
+traceoff_count_trigger(struct event_trigger_data *data, void *rec)
{
if (!tracing_is_on())
return;
@@ -879,13 +881,13 @@ static struct event_command trigger_traceoff_cmd = {
#ifdef CONFIG_TRACER_SNAPSHOT
static void
-snapshot_trigger(struct event_trigger_data *data)
+snapshot_trigger(struct event_trigger_data *data, void *rec)
{
tracing_snapshot();
}
static void
-snapshot_count_trigger(struct event_trigger_data *data)
+snapshot_count_trigger(struct event_trigger_data *data, void *rec)
{
if (!data->count)
return;
@@ -893,7 +895,7 @@ snapshot_count_trigger(struct event_trigger_data *data)
if (data->count != -1)
(data->count)--;
- snapshot_trigger(data);
+ snapshot_trigger(data, rec);
}
static int
@@ -972,13 +974,13 @@ static __init int register_trigger_snapshot_cmd(void) { return 0; }
#define STACK_SKIP 3
static void
-stacktrace_trigger(struct event_trigger_data *data)
+stacktrace_trigger(struct event_trigger_data *data, void *rec)
{
trace_dump_stack(STACK_SKIP);
}
static void
-stacktrace_count_trigger(struct event_trigger_data *data)
+stacktrace_count_trigger(struct event_trigger_data *data, void *rec)
{
if (!data->count)
return;
@@ -986,7 +988,7 @@ stacktrace_count_trigger(struct event_trigger_data *data)
if (data->count != -1)
(data->count)--;
- stacktrace_trigger(data);
+ stacktrace_trigger(data, rec);
}
static int
@@ -1057,7 +1059,7 @@ struct enable_trigger_data {
};
static void
-event_enable_trigger(struct event_trigger_data *data)
+event_enable_trigger(struct event_trigger_data *data, void *rec)
{
struct enable_trigger_data *enable_data = data->private_data;
@@ -1068,7 +1070,7 @@ event_enable_trigger(struct event_trigger_data *data)
}
static void
-event_enable_count_trigger(struct event_trigger_data *data)
+event_enable_count_trigger(struct event_trigger_data *data, void *rec)
{
struct enable_trigger_data *enable_data = data->private_data;
@@ -1082,7 +1084,7 @@ event_enable_count_trigger(struct event_trigger_data *data)
if (data->count != -1)
(data->count)--;
- event_enable_trigger(data);
+ event_enable_trigger(data, rec);
}
static int
--
1.9.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] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2015-10-22 20:30 +0200 |
| Message-ID | <qmsMH-299-35@gated-at.bofh.it> |
| In reply to | #1254047 |
On Thu, 22 Oct 2015 13:14:04 -0500 Tom Zanussi <tom.zanussi@linux.intel.com> wrote: > This is v11 of the 'hist triggers' patchset, which is the same as v10 > but adds the 2 new ftrace/hist triggers kselftest patches from Masami. > > Changes from v10: > > - Rebased to latest trace/for-next. > - Added Masami's ftrace/hist triggers kselftest patches. > - Added Masami's Tested-by: to the series. > Every time you send a patch set it's always when I'm about to leave for a conference (last one was for Dubin and I think plumbers was before that). Tomorrow I leave for Kernel Summit (takes me three days to get there :-p). Maybe, just maybe (I'm not going to hold my breath), I'll get to look at them while there. But between jet lag and concentrating on the topics (there's no keynotes to ignore), I don't think I'll get to it. Feel free to ping me on Monday Nov 2nd, and hopefully I'll be recovered by then. -- Steve -- 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]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:40 +0200 |
| Message-ID | <qmsWm-2kA-21@gated-at.bofh.it> |
| In reply to | #1254062 |
Hi Steve, On Thu, 2015-10-22 at 14:21 -0400, Steven Rostedt wrote: > On Thu, 22 Oct 2015 13:14:04 -0500 > Tom Zanussi <tom.zanussi@linux.intel.com> wrote: > > > This is v11 of the 'hist triggers' patchset, which is the same as v10 > > but adds the 2 new ftrace/hist triggers kselftest patches from Masami. > > > > Changes from v10: > > > > - Rebased to latest trace/for-next. > > - Added Masami's ftrace/hist triggers kselftest patches. > > - Added Masami's Tested-by: to the series. > > > > Every time you send a patch set it's always when I'm about to leave for > a conference (last one was for Dubin and I think plumbers was before > that). Tomorrow I leave for Kernel Summit (takes me three days to get > there :-p). > > Maybe, just maybe (I'm not going to hold my breath), I'll get to look > at them while there. But between jet lag and concentrating on the > topics (there's no keynotes to ignore), I don't think I'll get to it. > > Feel free to ping me on Monday Nov 2nd, and hopefully I'll be recovered > by then. > Sure, no problem, I'll ping you then. Have a great trip! Tom > -- Steve -- 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]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-22 20:30 +0200 |
| Subject | [PATCH v11 05/28] tracing: Add get_syscall_name() |
| Message-ID | <qmsMH-299-37@gated-at.bofh.it> |
| In reply to | #1254047 |
Add a utility function to grab the syscall name from the syscall
metadata, given a syscall id.
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
kernel/trace/trace.h | 5 +++++
kernel/trace/trace_syscalls.c | 11 +++++++++++
2 files changed, 16 insertions(+)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index bc039bc..892335a 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1390,8 +1390,13 @@ int perf_ftrace_event_register(struct trace_event_call *call,
#ifdef CONFIG_FTRACE_SYSCALLS
void init_ftrace_syscalls(void);
+const char *get_syscall_name(int syscall);
#else
static inline void init_ftrace_syscalls(void) { }
+static inline const char *get_syscall_name(int syscall)
+{
+ return NULL;
+}
#endif
#ifdef CONFIG_EVENT_TRACING
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 0655afb..50be560 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -106,6 +106,17 @@ static struct syscall_metadata *syscall_nr_to_meta(int nr)
return syscalls_metadata[nr];
}
+const char *get_syscall_name(int syscall)
+{
+ struct syscall_metadata *entry;
+
+ entry = syscall_nr_to_meta(syscall);
+ if (!entry)
+ return NULL;
+
+ return entry->name;
+}
+
static enum print_line_t
print_syscall_enter(struct trace_iterator *iter, int flags,
struct trace_event *event)
--
1.9.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] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-10-29 09:40 +0100 |
| Subject | Re: [PATCH v11 08/28] tracing: Add lock-free tracing_map |
| Message-ID | <qoQUy-6fX-15@gated-at.bofh.it> |
| In reply to | #1254047 |
Hi Tom,
On Thu, Oct 22, 2015 at 01:14:12PM -0500, Tom Zanussi wrote:
> Add tracing_map, a special-purpose lock-free map for tracing.
>
> tracing_map is designed to aggregate or 'sum' one or more values
> associated with a specific object of type tracing_map_elt, which
> is associated by the map to a given key.
>
> It provides various hooks allowing per-tracer customization and is
> separated out into a separate file in order to allow it to be shared
> between multiple tracers, but isn't meant to be generally used outside
> of that context.
>
> The tracing_map implementation was inspired by lock-free map
> algorithms originated by Dr. Cliff Click:
>
> http://www.azulsystems.com/blog/cliff/2007-03-26-non-blocking-hashtable
> http://www.azulsystems.com/events/javaone_2007/2007_LockFreeHash.pdf
>
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
[SNIP]
> +void tracing_map_array_free(struct tracing_map_array *a)
> +{
> + unsigned int i;
> +
> + if (!a->pages)
> + return;
You need to free 'a' anyway.. Or did you want to check 'a' being NULL?
> +
> + for (i = 0; i < a->n_pages; i++)
> + free_page((unsigned long)a->pages[i]);
> +
> + kfree(a);
> +}
> +
> +struct tracing_map_array *tracing_map_array_alloc(unsigned int n_elts,
> + unsigned int entry_size)
> +{
> + struct tracing_map_array *a;
> + unsigned int i;
> +
> + a = kzalloc(sizeof(*a), GFP_KERNEL);
> + if (!a)
> + return NULL;
> +
> + a->entry_size_shift = fls(roundup_pow_of_two(entry_size) - 1);
> + a->entries_per_page = PAGE_SIZE / (1 << a->entry_size_shift);
> + a->n_pages = n_elts / a->entries_per_page;
> + if (!a->n_pages)
> + a->n_pages = 1;
> + a->entry_shift = fls(a->entries_per_page) - 1;
> + a->entry_mask = (1 << a->entry_shift) - 1;
> +
> + a->pages = kcalloc(a->n_pages, sizeof(void *), GFP_KERNEL);
> + if (!a->pages)
> + goto free;
> +
> + for (i = 0; i < a->n_pages; i++) {
> + a->pages[i] = (void *)get_zeroed_page(GFP_KERNEL);
> + if (!a->pages)
if (!a->pages[i])
> + goto free;
> + }
> + out:
> + return a;
> + free:
> + tracing_map_array_free(a);
> + a = NULL;
> +
> + goto out;
> +}
> +
[SNIP]
> +/**
> + * tracing_map_insert - Insert key and/or retrieve val from a tracing_map
> + * @map: The tracing_map to insert into
> + * @key: The key to insert
> + *
> + * Inserts a key into a tracing_map and creates and returns a new
> + * tracing_map_elt for it, or if the key has already been inserted by
> + * a previous call, returns the tracing_map_elt already associated
> + * with it. When the map was created, the number of elements to be
> + * allocated for the map was specified (internally maintained as
> + * 'max_elts' in struct tracing_map), and that number of
> + * tracing_map_elts was created by tracing_map_init(). This is the
> + * pre-allocated pool of tracing_map_elts that tracing_map_insert()
> + * will allocate from when adding new keys. Once that pool is
> + * exhausted, tracing_map_insert() is useless and will return NULL to
> + * signal that state.
> + *
> + * This is a lock-free tracing map insertion function implementing a
> + * modified form of Cliff Click's basic insertion algorithm. It
> + * requires the table size be a power of two. To prevent any
> + * possibility of an infinite loop we always make the internal table
> + * size double the size of the requested table size (max_elts * 2).
> + * Likewise, we never reuse a slot or resize or delete elements - when
> + * we've reached max_elts entries, we simply return NULL once we've
> + * run out of entries. Readers can at any point in time traverse the
> + * tracing map and safely access the key/val pairs.
> + *
> + * Return: the tracing_map_elt pointer val associated with the key.
> + * If this was a newly inserted key, the val will be a newly allocated
> + * and associated tracing_map_elt pointer val. If the key wasn't
> + * found and the pool of tracing_map_elts has been exhausted, NULL is
> + * returned and no further insertions will succeed.
> + */
> +struct tracing_map_elt *tracing_map_insert(struct tracing_map *map, void *key)
> +{
> + u32 idx, key_hash, test_key;
> + struct tracing_map_entry *entry;
> +
> + key_hash = jhash(key, map->key_size, 0);
> + if (key_hash == 0)
> + key_hash = 1;
> + idx = key_hash >> (32 - (map->map_bits + 1));
> +
> + while (1) {
> + idx &= (map->map_size - 1);
> + entry = TRACING_MAP_ENTRY(map->map, idx);
> + test_key = entry->key;
> +
> + if (test_key && test_key == key_hash && entry->val &&
> + keys_match(key, entry->val->key, map->key_size))
> + return entry->val;
> +
> + if (!test_key && !cmpxchg(&entry->key, 0, key_hash)) {
> + struct tracing_map_elt *elt;
> +
> + elt = get_free_elt(map);
> + if (!elt)
> + break;
> + memcpy(elt->key, key, map->key_size);
> + entry->val = elt;
> +
> + return entry->val;
> + }
> + idx++;
> + }
> +
> + return NULL;
> +}
IIUC this always insert new entry if no matching key found. And if
the map is full, it only fails after walking through the entries to
find an empty one, mark the entry with the key and call to
get_free_elt() returns NULL. As more key added, it worsenes the
problem since more entries will be marked with no value IMHO.
I can see you checked hist_data->drops in the next patch to work
around this problem. But IMHO it's suboptimal since it cannot update
the existing entries too. I think it'd be better having lookup-only
version of this function and use it after it sees drops. The lookup
function can bail out from the loop if the insert doesn't mark empty
entry anymore IMHO.
Thoughts?
Thanks,
Namhyung
--
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]
| From | Tom Zanussi <tom.zanussi@linux.intel.com> |
|---|---|
| Date | 2015-10-29 19:40 +0100 |
| Subject | Re: [PATCH v11 08/28] tracing: Add lock-free tracing_map |
| Message-ID | <qp0hb-3KA-7@gated-at.bofh.it> |
| In reply to | #1258655 |
Hi Namhyung,
On Thu, 2015-10-29 at 17:31 +0900, Namhyung Kim wrote:
> Hi Tom,
>
> On Thu, Oct 22, 2015 at 01:14:12PM -0500, Tom Zanussi wrote:
> > Add tracing_map, a special-purpose lock-free map for tracing.
> >
> > tracing_map is designed to aggregate or 'sum' one or more values
> > associated with a specific object of type tracing_map_elt, which
> > is associated by the map to a given key.
> >
> > It provides various hooks allowing per-tracer customization and is
> > separated out into a separate file in order to allow it to be shared
> > between multiple tracers, but isn't meant to be generally used outside
> > of that context.
> >
> > The tracing_map implementation was inspired by lock-free map
> > algorithms originated by Dr. Cliff Click:
> >
> > http://www.azulsystems.com/blog/cliff/2007-03-26-non-blocking-hashtable
> > http://www.azulsystems.com/events/javaone_2007/2007_LockFreeHash.pdf
> >
> > Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> > Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > ---
>
> [SNIP]
> > +void tracing_map_array_free(struct tracing_map_array *a)
> > +{
> > + unsigned int i;
> > +
> > + if (!a->pages)
> > + return;
>
> You need to free 'a' anyway.. Or did you want to check 'a' being NULL?
>
Both, I think. Thanks for pointing this out, will fix in the next
version.
>
> > +
> > + for (i = 0; i < a->n_pages; i++)
> > + free_page((unsigned long)a->pages[i]);
> > +
> > + kfree(a);
> > +}
> > +
> > +struct tracing_map_array *tracing_map_array_alloc(unsigned int n_elts,
> > + unsigned int entry_size)
> > +{
> > + struct tracing_map_array *a;
> > + unsigned int i;
> > +
> > + a = kzalloc(sizeof(*a), GFP_KERNEL);
> > + if (!a)
> > + return NULL;
> > +
> > + a->entry_size_shift = fls(roundup_pow_of_two(entry_size) - 1);
> > + a->entries_per_page = PAGE_SIZE / (1 << a->entry_size_shift);
> > + a->n_pages = n_elts / a->entries_per_page;
> > + if (!a->n_pages)
> > + a->n_pages = 1;
> > + a->entry_shift = fls(a->entries_per_page) - 1;
> > + a->entry_mask = (1 << a->entry_shift) - 1;
> > +
> > + a->pages = kcalloc(a->n_pages, sizeof(void *), GFP_KERNEL);
> > + if (!a->pages)
> > + goto free;
> > +
> > + for (i = 0; i < a->n_pages; i++) {
> > + a->pages[i] = (void *)get_zeroed_page(GFP_KERNEL);
> > + if (!a->pages)
>
> if (!a->pages[i])
>
And this obvious error too, thanks.
>
> > + goto free;
> > + }
> > + out:
> > + return a;
> > + free:
> > + tracing_map_array_free(a);
> > + a = NULL;
> > +
> > + goto out;
> > +}
> > +
>
> [SNIP]
> > +/**
> > + * tracing_map_insert - Insert key and/or retrieve val from a tracing_map
> > + * @map: The tracing_map to insert into
> > + * @key: The key to insert
> > + *
> > + * Inserts a key into a tracing_map and creates and returns a new
> > + * tracing_map_elt for it, or if the key has already been inserted by
> > + * a previous call, returns the tracing_map_elt already associated
> > + * with it. When the map was created, the number of elements to be
> > + * allocated for the map was specified (internally maintained as
> > + * 'max_elts' in struct tracing_map), and that number of
> > + * tracing_map_elts was created by tracing_map_init(). This is the
> > + * pre-allocated pool of tracing_map_elts that tracing_map_insert()
> > + * will allocate from when adding new keys. Once that pool is
> > + * exhausted, tracing_map_insert() is useless and will return NULL to
> > + * signal that state.
> > + *
> > + * This is a lock-free tracing map insertion function implementing a
> > + * modified form of Cliff Click's basic insertion algorithm. It
> > + * requires the table size be a power of two. To prevent any
> > + * possibility of an infinite loop we always make the internal table
> > + * size double the size of the requested table size (max_elts * 2).
> > + * Likewise, we never reuse a slot or resize or delete elements - when
> > + * we've reached max_elts entries, we simply return NULL once we've
> > + * run out of entries. Readers can at any point in time traverse the
> > + * tracing map and safely access the key/val pairs.
> > + *
> > + * Return: the tracing_map_elt pointer val associated with the key.
> > + * If this was a newly inserted key, the val will be a newly allocated
> > + * and associated tracing_map_elt pointer val. If the key wasn't
> > + * found and the pool of tracing_map_elts has been exhausted, NULL is
> > + * returned and no further insertions will succeed.
> > + */
> > +struct tracing_map_elt *tracing_map_insert(struct tracing_map *map, void *key)
> > +{
> > + u32 idx, key_hash, test_key;
> > + struct tracing_map_entry *entry;
> > +
> > + key_hash = jhash(key, map->key_size, 0);
> > + if (key_hash == 0)
> > + key_hash = 1;
> > + idx = key_hash >> (32 - (map->map_bits + 1));
> > +
> > + while (1) {
> > + idx &= (map->map_size - 1);
> > + entry = TRACING_MAP_ENTRY(map->map, idx);
> > + test_key = entry->key;
> > +
> > + if (test_key && test_key == key_hash && entry->val &&
> > + keys_match(key, entry->val->key, map->key_size))
> > + return entry->val;
> > +
> > + if (!test_key && !cmpxchg(&entry->key, 0, key_hash)) {
> > + struct tracing_map_elt *elt;
> > +
> > + elt = get_free_elt(map);
> > + if (!elt)
> > + break;
> > + memcpy(elt->key, key, map->key_size);
> > + entry->val = elt;
> > +
> > + return entry->val;
> > + }
> > + idx++;
> > + }
> > +
> > + return NULL;
> > +}
>
> IIUC this always insert new entry if no matching key found. And if
> the map is full, it only fails after walking through the entries to
> find an empty one, mark the entry with the key and call to
> get_free_elt() returns NULL. As more key added, it worsenes the
> problem since more entries will be marked with no value IMHO.
>
> I can see you checked hist_data->drops in the next patch to work
> around this problem. But IMHO it's suboptimal since it cannot update
> the existing entries too. I think it'd be better having lookup-only
> version of this function and use it after it sees drops. The lookup
> function can bail out from the loop if the insert doesn't mark empty
> entry anymore IMHO.
>
> Thoughts?
>
The assumption has always been that once you have drops (i.e.
tracing_map_insert() returns NULL), the data can't really be trusted any
more and tracing should just stop (and presumably be restarted with a
bigger table). It doesn't mean that the data is completely useless,
just that it no longer can be assumed to have captured all the events
over the tracing run. Having a lookup-only version for the purpose of
updating only existing entries sort of illustrates the problem even
better - in that case only the events that already have entries in the
table will be included while the events that don't yet have entries will
be ignored, skewing the value of the data.
On the other hand, if users do end up calling this even after it's
returned NULL, we should make sure it doesn't result in an infinite
loop, and cap the number of iterations through the loop. tracing_map
wasn't really meant to be generally reusable - it was separated out so
it could be shared between two tracers - but it wouldn't hurt to add
that check just in case...
Thanks,
Tom
> Thanks,
> Namhyung
--
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