Hi Steven, On Wed, May 07, 2014 at 04:12:38AM +0100, Steven Rostedt wrote: > > Being able to show a cpumask of events can be useful as some events > may affect only some CPUs. There is no standard way to record the > cpumask and converting it to a string is rather expensive during > the trace as traces happen in hotpaths. It would be better to record > the raw event mask and be able to parse it at print time. > > The following macros were added for use with the TRACE_EVENT() macro: > > __bitmask() > __assign_bitmask() > __get_bitmask() > > To test this, I added this to the sched_migrate_task event, which > looked like this: > > TRACE_EVENT(sched_migrate_task, > > TP_PROTO(struct task_struct *p, int dest_cpu, const struct cpumask > *cpus), > > TP_ARGS(p, dest_cpu, cpus), > > TP_STRUCT__entry( > __array( char, comm, TASK_COMM_LEN ) > __field( pid_t, pid ) > __field( int, prio ) > __field( int, orig_cpu ) > __field( int, dest_cpu ) > __bitmask( cpumask, cpus ) > ), > > TP_fast_assign( > memcpy(__entry->comm, p->comm, TASK_COMM_LEN); > __entry->pid = p->pid; > __entry->prio = p->prio; > __entry->orig_cpu = task_cpu(p); > __entry->dest_cpu = dest_cpu; > __assign_bitmask(cpumask, cpumask_bits(cpus)); > ), > > TP_printk("comm=%s pid=%d prio=%d orig_cpu=%d dest_cpu=%d cpumask=%s", > __entry->comm, __entry->pid, __entry->prio, > __entry->orig_cpu, __entry->dest_cpu, > __get_bitmask(cpumask)) > ); > > With the output of: > > ksmtuned-3613 [003] d..2 485.220508: sched_migrate_task: > comm=ksmtuned pid=3615 prio=120 orig_cpu=3 dest_cpu=2 > cpumask=00000000,0000000f > migration/1-13 [001] d..5 485.221202: sched_migrate_task: > comm=ksmtuned pid=3614 prio=120 orig_cpu=1 dest_cpu=0 > cpumask=00000000,0000000f > awk-3615 [002] d.H5 485.221747: sched_migrate_task: > comm=rcu_preempt pid=7 prio=120 orig_cpu=0 dest_cpu=1 > cpumask=00000000,000000ff > migration/2-18 [002] d..5 485.222062: sched_migrate_task: > comm=ksmtuned pid=3615 prio=120 orig_cpu=2 dest_cpu=3 > cpumask=00000000,0000000f > > Link: > http://lkml.kernel.org/r/1399377998-14870-6-git-send-email-javi.mer...@arm.com > > Suggested-by: Javi Merino <javi.mer...@arm.com> > Signed-off-by: Steven Rostedt <rost...@goodmis.org> > --- > > Changes since v1: Use bitmask name instead of cpumask naming. > > --- > include/linux/ftrace_event.h | 3 +++ > include/linux/trace_seq.h | 10 ++++++++ > include/trace/ftrace.h | 57 > +++++++++++++++++++++++++++++++++++++++++++- > kernel/trace/trace_output.c | 41 +++++++++++++++++++++++++++++++ > 4 files changed, 110 insertions(+), 1 deletion(-) > > diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h > index d16da3e..cff3106 100644 > --- a/include/linux/ftrace_event.h > +++ b/include/linux/ftrace_event.h > @@ -38,6 +38,9 @@ const char *ftrace_print_symbols_seq_u64(struct trace_seq > *p, > *symbol_array); > #endif > > +const char *ftrace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr, > + unsigned int bitmask_size); > + > const char *ftrace_print_hex_seq(struct trace_seq *p, > const unsigned char *buf, int len); > > diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h > index a32d86e..1361169 100644 > --- a/include/linux/trace_seq.h > +++ b/include/linux/trace_seq.h > @@ -46,6 +46,9 @@ extern int trace_seq_putmem_hex(struct trace_seq *s, const > void *mem, > extern void *trace_seq_reserve(struct trace_seq *s, size_t len); > extern int trace_seq_path(struct trace_seq *s, const struct path *path); > > +extern int trace_seq_bitmask(struct trace_seq *s, const unsigned long *maskp, > + int nmaskbits); > + > #else /* CONFIG_TRACING */ > static inline int trace_seq_printf(struct trace_seq *s, const char *fmt, ...) > { > @@ -57,6 +60,13 @@ trace_seq_bprintf(struct trace_seq *s, const char *fmt, > const u32 *binary) > return 0; > } > > +static inline int > +trace_seq_bitmask(struct trace_seq *s, const unsigned long *maskp, > + int nmaskbits) > +{ > + return 0; > +} > + > static inline int trace_print_seq(struct seq_file *m, struct trace_seq *s) > { > return 0; > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h > index 0a1a4f7..d9c44af 100644 > --- a/include/trace/ftrace.h > +++ b/include/trace/ftrace.h > @@ -53,6 +53,9 @@ > #undef __string > #define __string(item, src) __dynamic_array(char, item, -1) > > +#undef __bitmask > +#define __bitmask(item, src) __dynamic_array(char, item, -1) > + > #undef TP_STRUCT__entry > #define TP_STRUCT__entry(args...) args > > @@ -128,6 +131,9 @@ > #undef __string > #define __string(item, src) __dynamic_array(char, item, -1) > > +#undef __string > +#define __string(item, src) __dynamic_array(unsigned long, item, -1) > +
This overrides the previous definition of __string() and looks like it shouldn't be here. > #undef DECLARE_EVENT_CLASS > #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) > \ > struct ftrace_data_offsets_##call { \ > @@ -200,6 +206,15 @@ > #undef __get_str > #define __get_str(field) (char *)__get_dynamic_array(field) > > +#undef __get_bitmask > +#define __get_bitmask(field) \ > + ({ \ > + void *__bitmask = __get_dynamic_array(field); \ > + unsigned int __bitmask_size; \ > + __bitmask_size = (__entry->__data_loc_##field >> 16) & 0xffff; \ > + ftrace_print_bitmask_seq(p, __bitmask, __bitmask_size); \ > + }) > + > #undef __print_flags > #define __print_flags(flag, delim, flag_array...) \ > ({ \ > @@ -322,6 +337,9 @@ static struct trace_event_functions > ftrace_event_type_funcs_##call = { \ > #undef __string > #define __string(item, src) __dynamic_array(char, item, -1) > > +#undef __bitmask > +#define __bitmask(item, src) __dynamic_array(unsigned long, item, -1) > + > > #undef DECLARE_EVENT_CLASS > #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, func, print) \ > static int notrace __init \ > @@ -372,6 +390,29 @@ ftrace_define_fields_##call(struct ftrace_event_call > *event_call) \ > #define __string(item, src) __dynamic_array(char, item, > \ > strlen((src) ? (const char *)(src) : "(null)") + 1) > > +/* > + * __bitmask_size_in_bytes_raw is the number of bytes needed to hold > + * num_possible_cpus(). > + */ > +#define __bitmask_size_in_bytes_raw \ > + ((num_possible_cpus() + 7) / 8) > + > +#define __bitmask_size_in_longs > \ > + ((__bitmask_size_in_bytes_raw + ((BITS_PER_LONG / 8) - 1)) \ > + / (BITS_PER_LONG / 8)) > + > +/* > + * __bitmask_size_in_bytes is the number of bytes needed to hold > + * num_possible_cpus() padded out to the nearest long. This is what > + * is saved in the buffer, just to be consistent. > + */ > +#define __bitmask_size_in_bytes \ > + (__bitmask_size_in_longs * (BITS_PER_LONG / 8)) > + > +#undef __bitmask > +#define __bitmask(item, src) __dynamic_array(unsigned long, item, \ > + __bitmask_size_in_longs) > + > #undef DECLARE_EVENT_CLASS > #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) > \ > static inline notrace int ftrace_get_offsets_##call( \ > @@ -513,12 +554,22 @@ static inline notrace int ftrace_get_offsets_##call( > \ > __entry->__data_loc_##item = __data_offsets.item; > > #undef __string > -#define __string(item, src) __dynamic_array(char, item, -1) \ > +#define __string(item, src) __dynamic_array(char, item, -1) > > #undef __assign_str > #define __assign_str(dst, src) > \ > strcpy(__get_str(dst), (src) ? (const char *)(src) : "(null)"); > > +#undef __bitmask > +#define __bitmask(item, src) __dynamic_array(unsigned long, item, -1) Why src? It's not used in any of the definitions of the __bitmask() macro, can we remove it? Cheers, Javi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/