tracing: Updates for 5.17-rc
Subsystems (6)
Steven Rostedt
maintainercover tracing: Updates for 5.17-rc
Tracing fixes for 5.17:
- rtla (Real-Time Linux Analysis tool): fix typo in man page
- rtla: Update API -e to -E before it is released
- rlla: Error message fix and memory leak fix
- Partially unline trace event soft disable to shrink text
- Fix function graph start up test
- Have triggers affect the trace instance they are in and not top level
- Have osnoise sleep in the units it says it uses
- Remove unused ftrace stub function
- Remove event probe reduntant info from event in the buffer
- Fix group ownership setting in tracefs
- Ensure trace buffer is minimum size to prevent crashes
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
ftrace/urgent
Head SHA1: f4b8715634f04fa03e730f1493ee919b56468de8
Arnaldo Carvalho de Melo (1):
rtla: Fix systme -> system typo on man page
Christophe Leroy (2):
tracing: Uninline trace_trigger_soft_disabled() partly
tracing: Fix selftest config check for function graph start up test
Daniel Bristot de Oliveira (5):
tracing: Dump stacktrace trigger to the corresponding instance tracing/osnoise: Make osnoise_main to sleep for microseconds rtla/hist: Make -E the short version of --entries rtla/osnoise: Free params at the exit
rtla/osnoise: Fix error message when failing to enable trace instance
Nathan Chancellor (1):
ftrace: Remove unused ftrace_startup_enable() stub
Steven Rostedt (Google) (3):
tracing: Have traceon and traceoff trigger honor the instance
eprobes: Remove redundant event type information
tracefs: Set the group ownership in apply_options() not parse_options()
Sven Schnelle (1):
tracing: Ensure trace buffer is at least 4096 bytes large
----
Documentation/tools/rtla/common_hist_options.rst | 2 +- .../tools/rtla/common_osnoise_description.rst | 2 +- Documentation/tools/rtla/rtla-osnoise-hist.rst | 2 +- fs/tracefs/inode.c | 4 +- include/linux/trace_events.h | 22 ++++--- kernel/trace/ftrace.c | 1 - kernel/trace/trace.c | 10 +-- kernel/trace/trace.h | 1 - kernel/trace/trace_eprobe.c | 16 +++-- kernel/trace/trace_events_trigger.c | 73 +++++++++++++++++++--- kernel/trace/trace_osnoise.c | 53 +++++++++------- kernel/trace/trace_probe.c | 10 +-- kernel/trace/trace_probe.h | 1 - kernel/trace/trace_selftest.c | 6 +- tools/tracing/rtla/src/osnoise.c | 2 +- tools/tracing/rtla/src/osnoise_hist.c | 10 +-- tools/tracing/rtla/src/osnoise_top.c | 1 + tools/tracing/rtla/src/timerlat_hist.c | 10 +-- 18 files changed, 148 insertions(+), 78 deletions(-)
Steven Rostedt
maintainerPATCH 1/13 rtla: Fix systme -> system typo on man page
From: Arnaldo Carvalho de Melo <[email protected]>
--- Documentation/tools/rtla/common_osnoise_description.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/tools/rtla/common_osnoise_description.rst +1 -1
@@ -1,7 +1,7 @@ The **rtla osnoise** tool is an interface for the *osnoise* tracer. The *osnoise* tracer dispatches a kernel thread per-cpu. These threads read the time in a loop while with preemption, softirq and IRQs enabled, thus -allowing all the sources of operating systme noise during its execution. +allowing all the sources of operating system noise during its execution. The *osnoise*'s tracer threads take note of the delta between each time read, along with an interference counter of all sources of interference. At the end of each period, the *osnoise* tracer displays a summary of
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 2/13 tracing: Dump stacktrace trigger to the corresponding instance
In reply to Steven RostedtFrom: Daniel Bristot de Oliveira <[email protected]>
The stacktrace event trigger is not dumping the stacktrace to the instance where it was enabled, but to the global "instance."
Use the private_data, pointing to the trigger file, to figure out the corresponding trace instance, and use it in the trigger action, like snapshot_trigger does.
--- kernel/trace/trace_events_trigger.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
kernel/trace/trace_events_trigger.c +6 -1
@@ -1540,7 +1540,12 @@ stacktrace_trigger(struct event_trigger_data *data, struct trace_buffer *buffer, void *rec, struct ring_buffer_event *event) { -trace_dump_stack (STACK_SKIP ); + struct trace_event_file *file = data->private_data; + +if (file) +__trace_stack (file->tr,tracing_gen_ctx (),STACK_SKIP ); +else +trace_dump_stack (STACK_SKIP ); } static void
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 3/13 tracing: Have traceon and traceoff trigger honor the instance
In reply to Steven RostedtFrom: "Steven Rostedt (Google)" <[email protected]>
If a trigger is set on an event to disable or enable tracing within an instance, then tracing should be disabled or enabled in the instance and not at the top level, which is confusing to users.
--- kernel/trace/trace_events_trigger.c | 52 +++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 6 deletions(-)
kernel/trace/trace_events_trigger.c +46 -6
@@ -1295,6 +1295,16 @@ traceon_trigger(struct event_trigger_data *data, struct trace_buffer *buffer, void *rec, struct ring_buffer_event *event) { + struct trace_event_file *file = data->private_data; + +if (file) { +if (tracer_tracing_is_on (file->tr)) +return ; + +tracer_tracing_on (file->tr); +return ; + } +if (tracing_is_on ())return ;
@@ -1306,8 +1316,15 @@ traceon_count_trigger(struct event_trigger_data *data, struct trace_buffer *buffer, void *rec, struct ring_buffer_event *event) { - if (tracing_is_on()) - return; + struct trace_event_file *file = data->private_data; + +if (file) { +if (tracer_tracing_is_on (file->tr)) +return ; + }else { +if (tracing_is_on ()) +return ; + }if (!data->count)return ;
@@ -1315,7 +1332,10 @@ traceon_count_trigger(struct event_trigger_data *data,if (data->count !=-1 ) (data->count)--; -tracing_on (); +if (file) +tracer_tracing_on (file->tr); +else +tracing_on (); } static void
@@ -1323,6 +1343,16 @@ traceoff_trigger(struct event_trigger_data *data, struct trace_buffer *buffer, void *rec, struct ring_buffer_event *event) { + struct trace_event_file *file = data->private_data; + +if (file) { +if (!tracer_tracing_is_on (file->tr)) +return ; + +tracer_tracing_off (file->tr); +return ; + } +if (!tracing_is_on ())return ;
@@ -1334,8 +1364,15 @@ traceoff_count_trigger(struct event_trigger_data *data, struct trace_buffer *buffer, void *rec, struct ring_buffer_event *event) { - if (!tracing_is_on()) - return; + struct trace_event_file *file = data->private_data; + +if (file) { +if (!tracer_tracing_is_on (file->tr)) +return ; + }else { +if (!tracing_is_on ()) +return ; + }if (!data->count)return ;
@@ -1343,7 +1380,10 @@ traceoff_count_trigger(struct event_trigger_data *data,if (data->count !=-1 ) (data->count)--; -tracing_off (); +if (file) +tracer_tracing_off (file->tr); +else +tracing_off (); } static int
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 4/13 eprobes: Remove redundant event type information
In reply to Steven RostedtFrom: "Steven Rostedt (Google)" <[email protected]>
Currently, the event probes save the type of the event they are attached to when recording the event. For example:
# echo 'e:switch sched/sched_switch prev_state=$prev_state prev_prio=$prev_prio next_pid=$next_pid next_prio=$next_prio' > dynamic_events # cat events/eprobes/switch/format
name: switch
ID: 1717
format:
field:unsigned short common_type; offset:0; size:2; signed:0;
field:unsigned char common_flags; offset:2; size:1; signed:0;
field:unsigned char common_preempt_count; offset:3; size:1; signed:0;
field:int common_pid; offset:4; size:4; signed:1;
field:unsigned int __probe_type; offset:8; size:4; signed:0;
field:u64 prev_state; offset:12; size:8; signed:0;
field:u64 prev_prio; offset:20; size:8; signed:0;
field:u64 next_pid; offset:28; size:8; signed:0;
field:u64 next_prio; offset:36; size:8; signed:0; print fmt: "(%u) prev_state=0x%Lx prev_prio=0x%Lx next_pid=0x%Lx next_prio=0x%Lx", REC->__probe_type, REC->prev_state, REC->prev_prio, REC->next_pid, REC->next_prio
The __probe_type adds 4 bytes to every event.
One of the reasons for creating eprobes is to limit what is traced in an event to be able to limit what is written into the ring buffer. Having this redundant 4 bytes to every event takes away from this.
The event that is recorded can be retrieved from the event probe itself, that is available when the trace is happening. For user space tools, it could simply read the dynamic_event file to find the event they are for. So there is really no reason to write this information into the ring buffer for every event.
--- kernel/trace/trace.h | 1 - kernel/trace/trace_eprobe.c | 16 +++++++--------- kernel/trace/trace_probe.c | 10 +++++----- kernel/trace/trace_probe.h | 1 - 4 files changed, 12 insertions(+), 16 deletions(-)
kernel/trace/trace.h +0 -1
@@ -136,7 +136,6 @@ struct kprobe_trace_entry_head {struct eprobe_trace_entry_head {struct trace_entry ent; -unsigned int type; };struct kretprobe_trace_entry_head {
kernel/trace/trace_eprobe.c +7 -9
@@ -242,7 +242,6 @@ static int trace_eprobe_tp_arg_update(struct trace_eprobe *ep, int i)static int eprobe_event_define_fields (struct trace_event_call *event_call) { -int ret;struct eprobe_trace_entry_head field;struct trace_probe *tp;
@@ -250,8 +249,6 @@ static int eprobe_event_define_fields(struct trace_event_call *event_call)if (WARN_ON_ONCE (!tp))return -ENOENT ; - DEFINE_FIELD(unsigned int, type, FIELD_STRING_TYPE, 0); -return traceprobe_define_arg_fields (event_call,sizeof (field), tp); }
@@ -270,7 +267,9 @@ print_eprobe_event(struct trace_iterator *iter, int flags,struct trace_event_call *pevent;struct trace_event *probed_event;struct trace_seq *s = &iter->seq; +struct trace_eprobe *ep;struct trace_probe *tp; +unsigned int type; field = (struct eprobe_trace_entry_head *)iter->ent; tp = trace_probe_primary_from_call(
@@ -278,15 +277,18 @@ print_eprobe_event(struct trace_iterator *iter, int flags,if (WARN_ON_ONCE (!tp)) goto out; + ep = container_of(tp, struct trace_eprobe, tp); + type = ep->event->event.type; +trace_seq_printf (s,"%s: (" ,trace_probe_name (tp)); - probed_event =ftrace_find_event (field->type); + probed_event =ftrace_find_event (type);if (probed_event) { pevent = container_of(probed_event, struct trace_event_call, event);trace_seq_printf (s,"%s.%s" , pevent->class->system,trace_event_name (pevent)); }else { -trace_seq_printf (s,"%u" , field->type); +trace_seq_printf (s,"%u" , type); }trace_seq_putc (s,')' );
@@ -498,10 +500,6 @@ __eprobe_trace_func(struct eprobe_data *edata, void *rec)return ; entry = fbuffer.entry =ring_buffer_event_data (fbuffer.event); -if (edata->ep->event) - entry->type = edata->ep->event->event.type; -else - entry->type =0 ;store_trace_args (&entry[1 ], &edata->ep->tp, rec,sizeof (*entry), dsize);trace_event_buffer_commit (&fbuffer);
kernel/trace/trace_probe.c +5 -5
@@ -871,15 +871,15 @@ static int __set_print_fmt(struct trace_probe *tp, char *buf, int len,switch (ptype) {case PROBE_PRINT_NORMAL : fmt ="(%lx)" ; - arg ="REC->" FIELD_STRING_IP ; + arg =", REC->" FIELD_STRING_IP ;break ;case PROBE_PRINT_RETURN : fmt ="(%lx <- %lx)" ; - arg ="REC->" FIELD_STRING_FUNC ", REC->" FIELD_STRING_RETIP ; + arg =", REC->" FIELD_STRING_FUNC ", REC->" FIELD_STRING_RETIP ;break ;case PROBE_PRINT_EVENT : - fmt ="(%u)" ; - arg ="REC->" FIELD_STRING_TYPE ; + fmt ="" ; + arg ="" ;break ;default :WARN_ON_ONCE (1 );
@@ -903,7 +903,7 @@ static int __set_print_fmt(struct trace_probe *tp, char *buf, int len, parg->type->fmt); } - pos +=snprintf (buf + pos,LEN_OR_ZERO ,"\", %s" , arg); + pos +=snprintf (buf + pos,LEN_OR_ZERO ,"\"%s" , arg);for (i =0 ; i < tp->nr_args; i++) { parg = tp->args + i;
kernel/trace/trace_probe.h +0 -1
@@ -38,7 +38,6 @@#define FIELD_STRING_IP "__probe_ip"#define FIELD_STRING_RETIP "__probe_ret_ip"#define FIELD_STRING_FUNC "__probe_func" -#define FIELD_STRING_TYPE "__probe_type"#undef DEFINE_FIELD#define DEFINE_FIELD (type, item, name, is_signed) \
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 5/13 tracing: Uninline trace_trigger_soft_disabled() partly
In reply to Steven RostedtFrom: Christophe Leroy <[email protected]>
On a powerpc32 build with CONFIG_CC_OPTIMISE_FOR_SIZE, the inline keyword is not honored and trace_trigger_soft_disabled() appears approx 50 times in vmlinux.
Adding -Winline to the build, the following message appears:
./include/linux/trace_events.h:712:1: error: inlining failed in call to 'trace_trigger_soft_disabled': call is unlikely and code size would grow [-Werror=inline]
That function is rather big for an inlined function:
c003df60 <trace_trigger_soft_disabled>: c003df60: 94 21 ff f0 stwu r1,-16(r1) c003df64: 7c 08 02 a6 mflr r0 c003df68: 90 01 00 14 stw r0,20(r1) c003df6c: bf c1 00 08 stmw r30,8(r1) c003df70: 83 e3 00 24 lwz r31,36(r3) c003df74: 73 e9 01 00 andi. r9,r31,256 c003df78: 41 82 00 10 beq c003df88 <trace_trigger_soft_disabled+0x28> c003df7c: 38 60 00 00 li r3,0 c003df80: 39 61 00 10 addi r11,r1,16 c003df84: 4b fd 60 ac b c0014030 <_rest32gpr_30_x> c003df88: 73 e9 00 80 andi. r9,r31,128 c003df8c: 7c 7e 1b 78 mr r30,r3 c003df90: 41 a2 00 14 beq c003dfa4 <trace_trigger_soft_disabled+0x44> c003df94: 38 c0 00 00 li r6,0 c003df98: 38 a0 00 00 li r5,0 c003df9c: 38 80 00 00 li r4,0 c003dfa0: 48 05 c5 f1 bl c009a590 <event_triggers_call> c003dfa4: 73 e9 00 40 andi. r9,r31,64 c003dfa8: 40 82 00 28 bne c003dfd0 <trace_trigger_soft_disabled+0x70> c003dfac: 73 ff 02 00 andi. r31,r31,512 c003dfb0: 41 82 ff cc beq c003df7c <trace_trigger_soft_disabled+0x1c> c003dfb4: 80 01 00 14 lwz r0,20(r1) c003dfb8: 83 e1 00 0c lwz r31,12(r1) c003dfbc: 7f c3 f3 78 mr r3,r30 c003dfc0: 83 c1 00 08 lwz r30,8(r1) c003dfc4: 7c 08 03 a6 mtlr r0 c003dfc8: 38 21 00 10 addi r1,r1,16 c003dfcc: 48 05 6f 6c b c0094f38 <trace_event_ignore_this_pid> c003dfd0: 38 60 00 01 li r3,1 c003dfd4: 4b ff ff ac b c003df80 <trace_trigger_soft_disabled+0x20>
However it is located in a hot path so inlining it is important. But forcing inlining of the entire function by using __always_inline leads to increasing the text size by approx 20 kbytes.
Instead, split the fonction in two parts, one part with the likely fast path, flagged __always_inline, and a second part out of line.
With this change, on a powerpc32 with CONFIG_CC_OPTIMISE_FOR_SIZE vmlinux text increases by only 1,4 kbytes, which is partly compensated by a decrease of vmlinux data by 7 kbytes.
On ppc64_defconfig which has CONFIG_CC_OPTIMISE_FOR_SPEED, this change reduces vmlinux text by more than 30 kbytes.
--- include/linux/trace_events.h | 22 ++++++++++++---------- kernel/trace/trace_events_trigger.c | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 10 deletions(-)
include/linux/trace_events.h +12 -10
@@ -699,6 +699,8 @@ event_triggers_post_call(struct trace_event_file *file,bool trace_event_ignore_this_pid (struct trace_event_file *trace_file); +bool __trace_trigger_soft_disabled (struct trace_event_file *file); +/** * trace_trigger_soft_disabled - do triggers and test if soft disabled * @file: The file pointer of the event to test
@@ -708,20 +710,20 @@ bool trace_event_ignore_this_pid(struct trace_event_file *trace_file);* triggers that require testing the fields, it will return true, * otherwise false. */ -static inline bool +static __always_inline booltrace_trigger_soft_disabled (struct trace_event_file *file) {unsigned long eflags = file->flags; -if (!(eflags &EVENT_FILE_FL_TRIGGER_COND )) { -if (eflags &EVENT_FILE_FL_TRIGGER_MODE ) -event_triggers_call (file,NULL ,NULL ,NULL ); -if (eflags &EVENT_FILE_FL_SOFT_DISABLED ) -return true; -if (eflags &EVENT_FILE_FL_PID_FILTER ) -return trace_event_ignore_this_pid (file); - } -return false; +if (likely (!(eflags & (EVENT_FILE_FL_TRIGGER_MODE | +EVENT_FILE_FL_SOFT_DISABLED | +EVENT_FILE_FL_PID_FILTER )))) +return false; + +if (likely (eflags &EVENT_FILE_FL_TRIGGER_COND )) +return false; + +return __trace_trigger_soft_disabled (file); }#ifdef CONFIG_BPF_EVENTS
kernel/trace/trace_events_trigger.c +14 -0
@@ -84,6 +84,20 @@ event_triggers_call(struct trace_event_file *file, }EXPORT_SYMBOL_GPL (event_triggers_call); +bool __trace_trigger_soft_disabled (struct trace_event_file *file) +{ +unsigned long eflags = file->flags; + +if (eflags &EVENT_FILE_FL_TRIGGER_MODE ) +event_triggers_call (file,NULL ,NULL ,NULL ); +if (eflags &EVENT_FILE_FL_SOFT_DISABLED ) +return true; +if (eflags &EVENT_FILE_FL_PID_FILTER ) +return trace_event_ignore_this_pid (file); +return false; +} +EXPORT_SYMBOL_GPL (__trace_trigger_soft_disabled); +/** * event_triggers_post_call - Call 'post_triggers' for a trace event * @file: The trace_event_file associated with the event
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 6/13 tracing: Ensure trace buffer is at least 4096 bytes large
In reply to Steven RostedtFrom: Sven Schnelle <[email protected]>
Booting the kernel with 'trace_buf_size=1' give a warning at boot during the ftrace selftests:
[ 0.892809] Running postponed tracer tests: [ 0.892893] Testing tracer function: [ 0.901899] Callback from call_rcu_tasks_trace() invoked. [ 0.983829] Callback from call_rcu_tasks_rude() invoked. [ 1.072003] .. bad ring buffer .. corrupted trace buffer .. [ 1.091944] Callback from call_rcu_tasks() invoked. [ 1.097695] PASSED [ 1.097701] Testing dynamic ftrace: .. filter failed count=0 ..FAILED! [ 1.353474] ------------[ cut here ]------------ [ 1.353478] WARNING: CPU: 0 PID: 1 at kernel/trace/trace.c:1951 run_tracer_selftest+0x13c/0x1b0
Therefore enforce a minimum of 4096 bytes to make the selftest pass.
--- kernel/trace/trace.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
kernel/trace/trace.c +6 -4
@@ -1474,10 +1474,12 @@ static int __init set_buf_size(char *str)if (!str)return 0 ; buf_size =memparse (str, &str); -/* nr_entries can not be zero */ -if (buf_size ==0 ) -return 0 ; - trace_buf_size = buf_size; +/* +* nr_entries can not be zero and the startup +* tests require some buffer space. Therefore +* ensure we have at least 4096 bytes of buffer. +*/ + trace_buf_size =max (4096UL , buf_size);return 1 ; }__setup ("trace_buf_size=" , set_buf_size);
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 7/13 ftrace: Remove unused ftrace_startup_enable() stub
In reply to Steven RostedtFrom: Nathan Chancellor <[email protected]>
When building with clang + CONFIG_DYNAMIC_FTRACE=n + W=1, there is a warning:
kernel/trace/ftrace.c:7194:20: error: unused function 'ftrace_startup_enable' [-Werror,-Wunused-function] static inline void ftrace_startup_enable(int command) { }
^
1 error generated.
Clang warns on instances of static inline functions in .c files with W=1 after commit 6863f5643dd7 ("kbuild: allow Clang to find unused static inline functions for W=1 build").
The ftrace_startup_enable() stub has been unused since commit e1effa0144a1 ("ftrace: Annotate the ops operation on update"), where its use outside of the CONFIG_DYNAMIC_TRACE section was replaced by ftrace_startup_all(). Remove it to resolve the warning.
--- kernel/trace/ftrace.c | 1 - 1 file changed, 1 deletion(-)
kernel/trace/ftrace.c +0 -1
@@ -7191,7 +7191,6 @@ static int __init ftrace_nodyn_init(void)core_initcall (ftrace_nodyn_init);static inline int ftrace_init_dyn_tracefs (struct dentry *d_tracer) {return 0 ; } -static inline void ftrace_startup_enable (int command) { }static inline void ftrace_startup_all (int command) { }# define ftrace_startup_sysctl () do { } while (0)
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 8/13 tracing/osnoise: Make osnoise_main to sleep for microseconds
In reply to Steven RostedtFrom: Daniel Bristot de Oliveira <[email protected]>
osnoise's runtime and period are in the microseconds scale, but it is currently sleeping in the millisecond's scale. This behavior roots in the usage of hwlat as the skeleton for osnoise.
Make osnoise to sleep in the microseconds scale. Also, move the sleep to a specialized function.
--- kernel/trace/trace_osnoise.c | 53 ++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 21 deletions(-)
kernel/trace/trace_osnoise.c +32 -21
@@ -1436,6 +1436,37 @@ static int run_osnoise(void)static struct cpumask osnoise_cpumask;static struct cpumask save_cpumask; +/* +* osnoise_sleep - sleep until the next period +*/ +static void osnoise_sleep (void ) +{ +u64 interval; +ktime_t wake_time; + +mutex_lock (&interface_lock); + interval = osnoise_data.sample_period - osnoise_data.sample_runtime; +mutex_unlock (&interface_lock); + +/* +* differently from hwlat_detector, the osnoise tracer can run +* without a pause because preemption is on. +*/ +if (!interval) { +/* Let synchronize_rcu_tasks() make progress */ +cond_resched_tasks_rcu_qs (); +return ; + } + + wake_time =ktime_add_us (ktime_get (), interval); +__set_current_state (TASK_INTERRUPTIBLE ); + +while (schedule_hrtimeout_range (&wake_time,0 ,HRTIMER_MODE_ABS )) { +if (kthread_should_stop ()) +break ; + } +} +/* * osnoise_main - The osnoise detection kernel thread *
@@ -1444,30 +1475,10 @@ static struct cpumask save_cpumask;*/ static int osnoise_main (void *data) { -u64 interval;while (!kthread_should_stop ()) { -run_osnoise (); - -mutex_lock (&interface_lock); - interval = osnoise_data.sample_period - osnoise_data.sample_runtime; -mutex_unlock (&interface_lock); - -do_div (interval,USEC_PER_MSEC ); - -/* -* differently from hwlat_detector, the osnoise tracer can run -* without a pause because preemption is on. -*/ -if (interval <1 ) { -/* Let synchronize_rcu_tasks() make progress */ -cond_resched_tasks_rcu_qs (); -continue ; - } - -if (msleep_interruptible (interval)) -break ; +osnoise_sleep (); }return 0 ;
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 9/13 tracefs: Set the group ownership in apply_options() not parse_options()
In reply to Steven RostedtFrom: "Steven Rostedt (Google)" <[email protected]>
Al Viro brought it to my attention that the dentries may not be filled when the parse_options() is called, causing the call to set_gid() to possibly crash. It should only be called if parse_options() succeeds totally anyway.
He suggested the logical place to do the update is in apply_options().
--- fs/tracefs/inode.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
fs/tracefs/inode.c +3 -1
@@ -264,7 +264,6 @@ static int tracefs_parse_options(char *data, struct tracefs_mount_opts *opts)if (!gid_valid (gid))return -EINVAL ; opts->gid = gid; -set_gid (tracefs_mount->mnt_root, gid);break ;case Opt_mode:if (match_octal (&args[0 ], &option))
@@ -293,6 +292,9 @@ static int tracefs_apply_options(struct super_block *sb) inode->i_uid = opts->uid; inode->i_gid = opts->gid; +if (tracefs_mount && tracefs_mount->mnt_root) +set_gid (tracefs_mount->mnt_root, opts->gid); +return 0 ; }
Signature · 1 line
2.34.1
Re: [for-linus][PATCH 09/13] tracefs: Set the group ownership in apply_options() not parse_options()
fs/tracefs/inode.c static int tracefs_apply_options(struct super_block *sb) · line 297
Quoted 7 lines · Steven Rostedt
fs/tracefs/inode.c +3
-0@@ -293,6 +292,9 @@ static int tracefs_apply_options(struct super_block *sb) inode->i_uid = opts->uid; inode->i_gid = opts->gid; +if (tracefs_mount && tracefs_mount->mnt_root) +set_gid (tracefs_mount->mnt_root, opts->gid); +
Umm... Why bother with tracefs_mount here in the first place? You have the superblock the operation is applied for - right in the arguments.
Just make that
set_gid(sb->s_root, opts->gid);
and be done with that. Same place in tracefs_apply_options()...
Incidentally, I'm not sure you need to keep ->i_gid assignment - set_gid() won't miss that inode, so...
Steven Rostedt
maintainerRe: [for-linus][PATCH 09/13] tracefs: Set the group ownership in apply_options() not parse_options()
Quoted 6 lines · On Fri, 25 Feb 2022 17:11:00 +0000 Al Viro <[email protected]> wrote:
Quoted 4 lines · Steven Rostedt
fs/tracefs/inode.c +0
-0@@ -293,6 +292,9 @@ static int tracefs_apply_options(struct super_block *sb) inode->i_uid = opts->uid; inode->i_gid = opts->gid;
Quoted 7 lines
fs/tracefs/inode.c static int tracefs_apply_options(struct super_block *sb) · line 297
Quoted 3 lines
fs/tracefs/inode.c +3
-0+if (tracefs_mount && tracefs_mount->mnt_root) +set_gid (tracefs_mount->mnt_root, opts->gid); +Umm... Why bother with
tracefs_mounthere in the first place? You have the superblock the operation is applied for - right in the arguments.
Ah, because I didn't realize they were one and the same ;-)
Quoted 7 lines
Just make that
set_gid(sb->s_root, opts->gid);and be done with that. Same place in
tracefs_apply_options()...Incidentally, I'm not sure you need to keep ->i_gid assignment -
set_gid()won't miss that inode, so...
Makes sense. Thanks.
I guess I wont be sending my pull request to Linus now :-/
-- Steve
Steven Rostedt
maintainerPATCH 10/13 tracing: Fix selftest config check for function graph start up test
In reply to Steven RostedtFrom: Christophe Leroy <[email protected]>
CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is required to test direct tramp.
--- kernel/trace/trace_selftest.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
kernel/trace/trace_selftest.c +2 -4
@@ -784,9 +784,7 @@ static struct fgraph_ops fgraph_ops __initdata = { .retfunc = &trace_graph_return, }; -#if defined(CONFIG_DYNAMIC_FTRACE ) && \ - defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS ) -#define TEST_DIRECT_TRAMP +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS noinline __noclone static void trace_direct_tramp(void) { }#endif
@@ -849,7 +847,7 @@ trace_selftest_startup_function_graph(struct tracer *trace, goto out; } -#ifdef TEST_DIRECT_TRAMP +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS tracing_reset_online_cpus (&tr->array_buffer);set_graph_array (tr);
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 11/13 rtla/hist: Make -E the short version of --entries
In reply to Steven RostedtFrom: Daniel Bristot de Oliveira <[email protected]>
Currently, --entries uses -e as the short version in the hist mode of timerlat and osnoise tools. But as -e is already used to enable events on trace sessions by other tools, thus let's keep it available for the same usage for all rtla tools.
Make -E the short version of --entries for hist mode on all tools.
Note: rtla was merged in this merge window, so rtla was not released yet.
--- Documentation/tools/rtla/common_hist_options.rst | 2 +- Documentation/tools/rtla/rtla-osnoise-hist.rst | 2 +- tools/tracing/rtla/src/osnoise_hist.c | 10 +++++----- tools/tracing/rtla/src/timerlat_hist.c | 10 +++++----- 4 files changed, 12 insertions(+), 12 deletions(-)
Documentation/tools/rtla/common_hist_options.rst +1 -1
@@ -2,7 +2,7 @@ Set the histogram bucket size (default *1*). -**-e**, **--entries** *N* +**-E**, **--entries** *N* Set the number of entries of the histogram (default 256).
Documentation/tools/rtla/rtla-osnoise-hist.rst +1 -1
@@ -36,7 +36,7 @@ default). The reason for reducing the runtime is to avoid starving the **rtla** tool. The tool is also set to run for *one minute*. The output histogram is set to group outputs in buckets of *10us* and *25* entries:: - [root@f34 ~/]# rtla osnoise hist -P F:1 -c 0-11 -r 900000 -d 1M -b 10 -e 25 + [root@f34 ~/]# rtla osnoise hist -P F:1 -c 0-11 -r 900000 -d 1M -b 10 -E 25 # RTLA osnoise histogram # Time unit is microseconds (us) # Duration: 0 00:01:00
tools/tracing/rtla/src/osnoise_hist.c +5 -5
@@ -426,7 +426,7 @@ static void osnoise_hist_usage(char *usage)static const char *const msg[] = {"" ," usage: rtla osnoise hist [-h] [-D] [-d s] [-p us] [-r us] [-s us] [-S us] [-t[=file]] \\" , -" [-c cpu-list] [-P priority] [-b N] [-e N] [--no-header] [--no-summary] \\" , +" [-c cpu-list] [-P priority] [-b N] [-E N] [--no-header] [--no-summary] \\" ," [--no-index] [--with-zeros]" ,"" ," -h/--help: print this menu" ,
@@ -439,7 +439,7 @@ static void osnoise_hist_usage(char *usage)" -D/--debug: print debug info" ," -t/--trace[=file]: save the stopped trace to [file|osnoise_trace.txt]" ," -b/--bucket-size N: set the histogram bucket size (default 1)" , -" -e/--entries N: set the number of entries of the histogram (default 256)" , +" -E/--entries N: set the number of entries of the histogram (default 256)" ," --no-header: do not print header" ," --no-summary: do not print summary" , " --no-index: do not print index",
@@ -486,7 +486,7 @@ static struct osnoise_hist_paramswhile (1 ) {static struct option long_options[] = { {"bucket-size" , required_argument,0 ,'b' }, - {"entries" , required_argument,0 ,'e' }, + {"entries" , required_argument,0 ,'E' }, {"cpus" , required_argument,0 ,'c' }, {"debug" , no_argument,0 ,'D' }, {"duration" , required_argument,0 ,'d' },
@@ -507,7 +507,7 @@ static struct osnoise_hist_params/* getopt_long stores the option index here. */ int option_index =0 ; - c =getopt_long (argc, argv,"c:b:d:e:Dhp:P:r:s:S:t::0123" , + c =getopt_long (argc, argv,"c:b:d:E:Dhp:P:r:s:S:t::0123" , long_options, &option_index);/* detect the end of the options. */
@@ -534,7 +534,7 @@ static struct osnoise_hist_paramsif (!params->duration)osnoise_hist_usage ("Invalid -D duration\n" );break ; -case 'e' : +case 'E' : params->entries =get_llong_from_str (optarg);if ((params->entries <10 ) || (params->entries >9999999 ))osnoise_hist_usage ("Entries must be > 10 and < 9999999\n" );
tools/tracing/rtla/src/timerlat_hist.c +5 -5
@@ -429,7 +429,7 @@ static void timerlat_hist_usage(char *usage)char *msg[] = {"" ," usage: [rtla] timerlat hist [-h] [-q] [-d s] [-D] [-n] [-p us] [-i us] [-T us] [-s us] [-t[=file]] \\" , -" [-c cpu-list] [-P priority] [-e N] [-b N] [--no-irq] [--no-thread] [--no-header] [--no-summary] \\" , +" [-c cpu-list] [-P priority] [-E N] [-b N] [--no-irq] [--no-thread] [--no-header] [--no-summary] \\" ," [--no-index] [--with-zeros]" ,"" ," -h/--help: print this menu" ,
@@ -443,7 +443,7 @@ static void timerlat_hist_usage(char *usage)" -T/--trace[=file]: save the stopped trace to [file|timerlat_trace.txt]" ," -n/--nano: display data in nanoseconds" ," -b/--bucket-size N: set the histogram bucket size (default 1)" , -" -e/--entries N: set the number of entries of the histogram (default 256)" , +" -E/--entries N: set the number of entries of the histogram (default 256)" ," --no-irq: ignore IRQ latencies" ," --no-thread: ignore thread latencies" , " --no-header: do not print header",
@@ -494,7 +494,7 @@ static struct timerlat_hist_params {"cpus", required_argument, 0, 'c'}, {"bucket-size", required_argument, 0, 'b'}, {"debug", no_argument, 0, 'D'}, - {"entries", required_argument, 0, 'e'}, + {"entries", required_argument, 0, 'E'}, {"duration", required_argument, 0, 'd'}, {"help", no_argument, 0, 'h'}, {"irq", required_argument, 0, 'i'},
@@ -516,7 +516,7 @@ static struct timerlat_hist_params/* getopt_long stores the option index here. */ int option_index =0 ; - c =getopt_long (argc, argv,"c:b:d:e:Dhi:np:P:s:t::T:012345" , + c =getopt_long (argc, argv,"c:b:d:E:Dhi:np:P:s:t::T:012345" , long_options, &option_index);/* detect the end of the options. */
@@ -543,7 +543,7 @@ static struct timerlat_hist_paramsif (!params->duration)timerlat_hist_usage ("Invalid -D duration\n" );break ; -case 'e' : +case 'E' : params->entries =get_llong_from_str (optarg);if ((params->entries <10 ) || (params->entries >9999999 ))timerlat_hist_usage ("Entries must be > 10 and < 9999999\n" );
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 12/13 rtla/osnoise: Free params at the exit
In reply to Steven RostedtFrom: Daniel Bristot de Oliveira <[email protected]>
The variable that stores the parsed command line arguments are not being free()d at the rtla osnoise top exit path.
Free params variable before exiting.
--- tools/tracing/rtla/src/osnoise_top.c | 1 + 1 file changed, 1 insertion(+)
tools/tracing/rtla/src/osnoise_top.c +1 -0
@@ -573,6 +573,7 @@ int osnoise_top_main(int argc, char **argv)osnoise_free_top (tool->data);osnoise_destroy_tool (record);osnoise_destroy_tool (tool); +free (params); out_exit:exit (return_value); }
Signature · 1 line
2.34.1
Steven Rostedt
maintainerPATCH 13/13 rtla/osnoise: Fix error message when failing to enable trace instance
In reply to Steven RostedtFrom: Daniel Bristot de Oliveira <[email protected]>
When a trace instance creation fails, tools are printing:
Could not enable -> osnoiser <- tracer for tracing
Print the actual (and correct) name of the tracer it fails to enable.
--- tools/tracing/rtla/src/osnoise.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/tracing/rtla/src/osnoise.c +1 -1
@@ -810,7 +810,7 @@ struct osnoise_tool *osnoise_init_trace_tool(char *tracer) retval =enable_tracer_by_name (trace->trace.inst, tracer);if (retval) { -err_msg ("Could not enable osnoiser tracer for tracing\n" ); +err_msg ("Could not enable %s tracer for tracing\n" , tracer); goto out_err; }
Signature · 1 line
2.34.1