tracing: Last minute updates for 4.10
Subsystems (7)
Steven Rostedt
maintainercover tracing: Last minute updates for 4.10
There are two patches that are marked for stable. But since Linus will hopefully open the merge window very soon, I figured I let those changes sit in linux-next for a week before pushing to Linus. They are not that critical as they are old bugs, and require root privilege to abuse.
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next
Head SHA1: 1a41442864e35bff859582fe9c5d051d0b1040ba
Steven Rostedt (Red Hat) (8):
tracing: Have the reg function allow to fail
tracing: Do not start benchmark on boot up
tracing: Have system enable return error if one of the events fail
tracing: Allow benchmark to be enabled at early_initcall() ftrace/x86_32: Set ftrace_stub to weak to prevent gcc from using short jumps to it
tracing: Replace kmap with copy_from_user() in trace_marker writing
fgraph: Handle a case where a tracer ignores set_graph_notrace tracing/fgraph: Have wakeup and irqsoff tracers ignore graph functions too
----
arch/powerpc/include/asm/trace.h | 4 +- arch/powerpc/platforms/powernv/opal-tracepoints.c | 6 +- arch/powerpc/platforms/pseries/lpar.c | 6 +- arch/x86/entry/entry_32.S | 4 +- arch/x86/include/asm/trace/exceptions.h | 2 +- arch/x86/include/asm/trace/irq_vectors.h | 2 +- arch/x86/kernel/tracepoint.c | 3 +- drivers/i2c/i2c-core.c | 3 +- include/linux/tracepoint-defs.h | 2 +- include/linux/tracepoint.h | 2 +- include/trace/events/i2c.h | 2 +- kernel/trace/trace.c | 139 ++++++---------------- kernel/trace/trace.h | 11 ++ kernel/trace/trace_benchmark.c | 26 +++- kernel/trace/trace_benchmark.h | 2 +- kernel/trace/trace_events.c | 13 +- kernel/trace/trace_functions_graph.c | 31 +++-- kernel/trace/trace_irqsoff.c | 12 ++ kernel/trace/trace_sched_wakeup.c | 12 ++ kernel/tracepoint.c | 12 +- samples/trace_events/trace-events-sample.c | 3 +- samples/trace_events/trace-events-sample.h | 2 +- 22 files changed, 162 insertions(+), 137 deletions(-)
Steven Rostedt
maintainerPATCH 1/8 tracing: Have the reg function allow to fail
From: "Steven Rostedt (Red Hat)" <[email protected]>
Some tracepoints have a registration function that gets enabled when the tracepoint is enabled. There may be cases that the registraction function must fail (for example, can't allocate enough memory). In this case, the tracepoint should also fail to register, otherwise the user would not know why the tracepoint is not working.
--- arch/powerpc/include/asm/trace.h | 4 ++-- arch/powerpc/platforms/powernv/opal-tracepoints.c | 6 ++++-- arch/powerpc/platforms/pseries/lpar.c | 6 ++++-- arch/x86/include/asm/trace/exceptions.h | 2 +- arch/x86/include/asm/trace/irq_vectors.h | 2 +- arch/x86/kernel/tracepoint.c | 3 ++- drivers/i2c/i2c-core.c | 3 ++- include/linux/tracepoint-defs.h | 2 +- include/linux/tracepoint.h | 2 +- include/trace/events/i2c.h | 2 +- kernel/trace/trace_benchmark.c | 3 ++- kernel/trace/trace_benchmark.h | 2 +- kernel/tracepoint.c | 12 +++++++++--- samples/trace_events/trace-events-sample.c | 3 ++- samples/trace_events/trace-events-sample.h | 2 +- 15 files changed, 34 insertions(+), 20 deletions(-)
arch/powerpc/include/asm/trace.h +2 -2
@@ -54,7 +54,7 @@ DEFINE_EVENT(ppc64_interrupt_class, timer_interrupt_exit, );#ifdef CONFIG_PPC_PSERIES -extern void hcall_tracepoint_regfunc (void ); +extern int hcall_tracepoint_regfunc (void );extern void hcall_tracepoint_unregfunc (void ); TRACE_EVENT_FN_COND(hcall_entry,
@@ -104,7 +104,7 @@ TRACE_EVENT_FN_COND(hcall_exit,#endif #ifdef CONFIG_PPC_POWERNV -extern void opal_tracepoint_regfunc (void ); +extern int opal_tracepoint_regfunc (void );extern void opal_tracepoint_unregfunc (void ); TRACE_EVENT_FN(opal_entry,
arch/powerpc/platforms/powernv/opal-tracepoints.c +4 -2
@@ -6,9 +6,10 @@#ifdef HAVE_JUMP_LABEL struct static_key opal_tracepoint_key =STATIC_KEY_INIT ; -void opal_tracepoint_regfunc (void ) +int opal_tracepoint_regfunc (void ) {static_key_slow_inc (&opal_tracepoint_key); +return 0 ; }void opal_tracepoint_unregfunc (void )
@@ -25,9 +26,10 @@ void opal_tracepoint_unregfunc(void)/* NB: reg/unreg are called while guarded with the tracepoints_mutex */ extern long opal_tracepoint_refcount; -void opal_tracepoint_regfunc (void ) +int opal_tracepoint_regfunc (void ) { opal_tracepoint_refcount++; +return 0 ; }void opal_tracepoint_unregfunc (void )
arch/powerpc/platforms/pseries/lpar.c +4 -2
@@ -661,9 +661,10 @@ EXPORT_SYMBOL(arch_free_page);#ifdef HAVE_JUMP_LABEL struct static_key hcall_tracepoint_key =STATIC_KEY_INIT ; -void hcall_tracepoint_regfunc (void ) +int hcall_tracepoint_regfunc (void ) {static_key_slow_inc (&hcall_tracepoint_key); +return 0 ; }void hcall_tracepoint_unregfunc (void )
@@ -680,9 +681,10 @@ void hcall_tracepoint_unregfunc(void)/* NB: reg/unreg are called while guarded with the tracepoints_mutex */ extern long hcall_tracepoint_refcount; -void hcall_tracepoint_regfunc (void ) +int hcall_tracepoint_regfunc (void ) { hcall_tracepoint_refcount++; +return 0 ; }void hcall_tracepoint_unregfunc (void )
arch/x86/include/asm/trace/exceptions.h +1 -1
@@ -6,7 +6,7 @@#include <linux/tracepoint.h> -extern void trace_irq_vector_regfunc (void ); +extern int trace_irq_vector_regfunc (void );extern void trace_irq_vector_unregfunc (void ); DECLARE_EVENT_CLASS(x86_exceptions,
arch/x86/include/asm/trace/irq_vectors.h +1 -1
@@ -6,7 +6,7 @@#include <linux/tracepoint.h> -extern void trace_irq_vector_regfunc (void ); +extern int trace_irq_vector_regfunc (void );extern void trace_irq_vector_unregfunc (void ); DECLARE_EVENT_CLASS(x86_irq_vector,
arch/x86/kernel/tracepoint.c +2 -1
@@ -34,7 +34,7 @@ static void switch_idt(void *arg)local_irq_restore (flags); } -void trace_irq_vector_regfunc (void ) +int trace_irq_vector_regfunc (void ) {mutex_lock (&irq_vector_mutex);if (!trace_irq_vector_refcount) {
@@ -44,6 +44,7 @@ void trace_irq_vector_regfunc(void) } trace_irq_vector_refcount++;mutex_unlock (&irq_vector_mutex); +return 0 ; }void trace_irq_vector_unregfunc (void )
drivers/i2c/i2c-core.c +2 -1
@@ -77,9 +77,10 @@ static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver);static struct static_key i2c_trace_msg =STATIC_KEY_INIT_FALSE ;static bool is_registered; -void i2c_transfer_trace_reg (void ) +int i2c_transfer_trace_reg (void ) {static_key_slow_inc (&i2c_trace_msg); +return 0 ; }void i2c_transfer_trace_unreg (void )
include/linux/tracepoint-defs.h +1 -1
@@ -29,7 +29,7 @@ struct tracepoint_func {struct tracepoint {const char *name;/* Tracepoint name */ struct static_key key; -void (*regfunc)(void ); +int (*regfunc)(void );void (*unregfunc)(void );struct tracepoint_func __rcu *funcs; };
include/linux/tracepoint.h +1 -1
@@ -81,7 +81,7 @@ static inline void tracepoint_synchronize_unregister(void) }#ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS -extern void syscall_regfunc (void ); +extern int syscall_regfunc (void );extern void syscall_unregfunc (void );#endif /* CONFIG_HAVE_SYSCALL_TRACEPOINTS */
include/trace/events/i2c.h +1 -1
@@ -20,7 +20,7 @@/* * drivers/i2c/i2c-core.c */ -extern void i2c_transfer_trace_reg (void ); +extern int i2c_transfer_trace_reg (void );extern void i2c_transfer_trace_unreg (void );/*
kernel/trace/trace_benchmark.c +2 -1
@@ -164,11 +164,12 @@ static int benchmark_event_kthread(void *arg)* When the benchmark tracepoint is enabled, it calls this * function and the thread that calls the tracepoint is created. */ -void trace_benchmark_reg (void ) +int trace_benchmark_reg (void ) { bm_event_thread =kthread_run (benchmark_event_kthread,NULL ,"event_benchmark" );WARN_ON (!bm_event_thread); +return 0 ; }/*
kernel/trace/trace_benchmark.h +1 -1
@@ -6,7 +6,7 @@#include <linux/tracepoint.h> -extern void trace_benchmark_reg (void ); +extern int trace_benchmark_reg (void );extern void trace_benchmark_unreg (void );#define BENCHMARK_EVENT_STRLEN 128
kernel/tracepoint.c +9 -3
@@ -194,9 +194,13 @@ static int tracepoint_add_func(struct tracepoint *tp, struct tracepoint_func *func, int prio) {struct tracepoint_func *old, *tp_funcs; +int ret; -if (tp->regfunc && !static_key_enabled (&tp->key)) - tp->regfunc (); +if (tp->regfunc && !static_key_enabled (&tp->key)) { + ret = tp->regfunc (); +if (ret <0 ) +return ret; + } tp_funcs =rcu_dereference_protected (tp->funcs,lockdep_is_held (&tracepoints_mutex));
@@ -529,7 +533,7 @@ EXPORT_SYMBOL_GPL(for_each_kernel_tracepoint);/* NB: reg/unreg are called while guarded with the tracepoints_mutex */ static int sys_tracepoint_refcount; -void syscall_regfunc (void ) +int syscall_regfunc (void ) {struct task_struct *p, *t;
@@ -541,6 +545,8 @@ void syscall_regfunc(void)read_unlock (&tasklist_lock); } sys_tracepoint_refcount++; + +return 0 ; }void syscall_unregfunc (void )
samples/trace_events/trace-events-sample.c +2 -1
@@ -79,7 +79,7 @@ static int simple_thread_fn(void *arg)static DEFINE_MUTEX (thread_mutex); -void foo_bar_reg (void ) +int foo_bar_reg (void ) {pr_info ("Starting thread for foo_bar_fn\n" );/*
@@ -90,6 +90,7 @@ void foo_bar_reg(void)mutex_lock (&thread_mutex); simple_tsk_fn =kthread_run (simple_thread_fn,NULL ,"event-sample-fn" );mutex_unlock (&thread_mutex); +return 0 ; }void foo_bar_unreg (void )
samples/trace_events/trace-events-sample.h +1 -1
@@ -354,7 +354,7 @@ TRACE_EVENT_CONDITION(foo_bar_with_cond,TP_printk ("foo %s %d" ,__get_str (foo), __entry->bar) ); -void foo_bar_reg (void ); +int foo_bar_reg (void );void foo_bar_unreg (void );/*
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 2/8 tracing: Do not start benchmark on boot up
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
Trace events are enabled very early on boot up via the boot command line parameter. The benchmark tool creates a new thread to perform the trace event benchmarking. But at start up, it is called before scheduling is set up and because it creates a new thread before the init thread is created, this crashes the kernel.
Have the benchmark fail to register when started via the kernel command line.
Also, since the registering of a tracepoint now can handle failure cases, return -ENOMEM instead of warning if the thread cannot be created.
--- kernel/trace/trace_benchmark.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
kernel/trace/trace_benchmark.c +11 -1
@@ -166,9 +166,18 @@ static int benchmark_event_kthread(void *arg)*/ int trace_benchmark_reg (void ) { +if (system_state !=SYSTEM_RUNNING ) { +pr_warning ("trace benchmark cannot be started via kernel command line\n" ); +return -EBUSY ; + } + bm_event_thread =kthread_run (benchmark_event_kthread,NULL ,"event_benchmark" ); -WARN_ON (!bm_event_thread); +if (!bm_event_thread) { +pr_warning ("trace benchmark failed to create kernel thread\n" ); +return -ENOMEM ; + } +return 0 ; }
@@ -183,6 +192,7 @@ void trace_benchmark_unreg(void)return ;kthread_stop (bm_event_thread); + bm_event_thread =NULL ;strcpy (bm_str,"START" ); bm_total =0 ;
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 3/8 tracing: Have system enable return error if one of the events fail
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
If one of the events within a system fails to enable when "1" is written to the system "enable" file, it should return an error. Note, some events may still be enabled, but the user should know that something did go wrong.
--- kernel/trace/trace_events.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
kernel/trace/trace_events.c +11 -2
@@ -702,6 +702,7 @@ __ftrace_set_clr_event_nolock(struct trace_array *tr, const char *match,struct trace_event_call *call;const char *name;int ret = -EINVAL ; +int eret =0 ;list_for_each_entry (file, &tr->events, list) {
@@ -725,9 +726,17 @@ __ftrace_set_clr_event_nolock(struct trace_array *tr, const char *match,if (event &&strcmp (event, name) !=0 )continue ; -ftrace_event_enable_disable (file, set); + ret =ftrace_event_enable_disable (file, set); - ret =0 ; +/* +* Save the first error and return that. Some events +* may still have been enabled, but let the user +* know that something went wrong. +*/ +if (ret && !eret) + eret = ret; + + ret = eret; }return ret;
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 4/8 tracing: Allow benchmark to be enabled at early_initcall()
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
The trace event start up selftests fails when the trace benchmark is enabled, because it is disabled during boot. It really only needs to be disabled before scheduling is set up, as it creates a thread.
--- kernel/trace/trace_benchmark.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
kernel/trace/trace_benchmark.c +12 -1
@@ -21,6 +21,8 @@ static u64 bm_stddev;static unsigned int bm_avg;static unsigned int bm_std; +static bool ok_to_run; +/* * This gets called in a loop recording the time it took to write * the tracepoint. What it writes is the time statistics of the last
@@ -166,7 +168,7 @@ static int benchmark_event_kthread(void *arg)*/ int trace_benchmark_reg (void ) { -if (system_state !=SYSTEM_RUNNING ) { +if (!ok_to_run) {pr_warning ("trace benchmark cannot be started via kernel command line\n" );return -EBUSY ; }
@@ -207,3 +209,12 @@ void trace_benchmark_unreg(void) bm_avg =0 ; bm_stddev =0 ; } + +static __init int ok_to_run_trace_benchmark (void ) +{ + ok_to_run = true; + +return 0 ; +} + +early_initcall (ok_to_run_trace_benchmark);
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 5/8 ftrace/x86_32: Set ftrace_stub to weak to prevent gcc from using short jumps to it
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
With new binutils, gcc may get smart with its optimization and change a jmp from a 5 byte jump to a 2 byte one even though it was jumping to a global function. But that global function existed within a 2 byte radius, and gcc was able to optimize it. Unfortunately, that jump was also being modified when function graph tracing begins. Since ftrace expected that jump to be 5 bytes, but it was only two, it overwrote code after the jump, causing a crash.
This was fixed for x86_64 with commit 8329e818f149, with the same subject as this commit, but nothing was done for x86_32.
--- arch/x86/entry/entry_32.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/entry/entry_32.S +2 -2
@@ -889,8 +889,8 @@ ftrace_graph_call: jmp ftrace_stub #endif -.globl ftrace_stub -ftrace_stub: +/* This is weak to keep gas from relaxing the jumps */ +WEAK(ftrace_stub) ret END(ftrace_caller)
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 6/8 tracing: Replace kmap with copy_from_user() in trace_marker writing
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
Instead of using get_user_pages_fast() and kmap_atomic() when writing to the trace_marker file, just allocate enough space on the ring buffer directly, and write into it via copy_from_user().
Writing into the trace_marker file use to allocate a temporary buffer to perform the copy_from_user(), as we didn't want to write into the ring buffer if the copy failed. But as a trace_marker write is suppose to be extremely fast, and allocating memory causes other tracepoints to trigger, Peter Zijlstra suggested using get_user_pages_fast() and kmap_atomic() to keep the user space pages in memory and reading it directly. But Henrik Austad had issues with this because it required taking the mm->mmap_sem and causing long delays with the write.
Instead, just allocate the space in the ring buffer and use copy_from_user() directly. If it faults, return -EFAULT and write "<faulted>" into the ring buffer.
Cc: Ingo Molnar <[email protected]>
Cc: Henrik Austad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Updates: d696b58ca2c3ca "tracing: Do not allocate buffer for trace_marker"
--- kernel/trace/trace.c | 139 ++++++++++++++------------------------------------- 1 file changed, 37 insertions(+), 102 deletions(-)
kernel/trace/trace.c +37 -102
@@ -5738,61 +5738,6 @@ tracing_free_buffer_release(struct inode *inode, struct file *filp)return 0 ; } -static inline int lock_user_pages (const char __user *ubuf,size_t cnt, -struct page **pages,void **map_page, -int *offset) -{ -unsigned long addr = (unsigned long )ubuf; -int nr_pages =1 ; -int ret; -int i; - -/* -* Userspace is injecting traces into the kernel trace buffer. -* We want to be as non intrusive as possible. -* To do so, we do not want to allocate any special buffers -* or take any locks, but instead write the userspace data -* straight into the ring buffer. -* -* First we need to pin the userspace buffer into memory, -* which, most likely it is, because it just referenced it. -* But there's no guarantee that it is. By using get_user_pages_fast() -* and kmap_atomic/kunmap_atomic() we can get access to the -* pages directly. We then write the data directly into the -* ring buffer. -*/ - -/* check if we cross pages */ -if ((addr &PAGE_MASK ) != ((addr + cnt) &PAGE_MASK )) - nr_pages =2 ; - - *offset = addr & (PAGE_SIZE -1 ); - addr &=PAGE_MASK ; - - ret =get_user_pages_fast (addr, nr_pages,0 , pages); -if (ret < nr_pages) { -while (--ret >=0 ) -put_page (pages[ret]); -return -EFAULT ; - } - -for (i =0 ; i < nr_pages; i++) - map_page[i] =kmap_atomic (pages[i]); - -return nr_pages; -} - -static inline void unlock_user_pages (struct page **pages, -void **map_page,int nr_pages) -{ -int i; - -for (i = nr_pages -1 ; i >=0 ; i--) { -kunmap_atomic (map_page[i]); -put_page (pages[i]); - } -} -static ssize_t tracing_mark_write (struct file *filp,const char __user *ubuf,size_t cnt,loff_t *fpos)
@@ -5802,14 +5747,14 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,struct ring_buffer *buffer;struct print_entry *entry;unsigned long irq_flags; -struct page *pages[2 ]; -void *map_page[2 ]; -int nr_pages =1 ; +const char faulted[] ="<faulted>" ;ssize_t written; -int offset;int size;int len; +/* Used in tracing_mark_raw_write() as well */ +#define FAULTED_SIZE (sizeof(faulted) - 1)/* '\0' is already accounted for */ +if (tracing_disabled)return -EINVAL ;
@@ -5821,30 +5766,31 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,BUILD_BUG_ON (TRACE_BUF_SIZE >=PAGE_SIZE ); - nr_pages =lock_user_pages (ubuf, cnt, pages, map_page, &offset); -if (nr_pages <0 ) -return nr_pages; -local_save_flags (irq_flags); - size =sizeof (*entry) + cnt +2 ;/* possible \n added */ + size =sizeof (*entry) + cnt +2 ;/* add '\0' and possible '\n' */ + +/* If less than "<faulted>", then make sure we can still add that */ +if (cnt <FAULTED_SIZE ) + size +=FAULTED_SIZE - cnt; + buffer = tr->trace_buffer.buffer; event =__trace_buffer_lock_reserve (buffer,TRACE_PRINT , size, irq_flags,preempt_count ()); -if (!event) { +if (unlikely (!event))/* Ring buffer disabled, return as if not open for write */ - written = -EBADF ; - goto out_unlock; - } +return -EBADF ; entry =ring_buffer_event_data (event); entry->ip = _THIS_IP_; -if (nr_pages ==2 ) { - len =PAGE_SIZE - offset; -memcpy (&entry->buf, map_page[0 ] + offset, len); -memcpy (&entry->buf[len], map_page[1 ], cnt - len); + len =__copy_from_user_inatomic (&entry->buf, ubuf, cnt); +if (len) { +memcpy (&entry->buf, faulted,FAULTED_SIZE ); + cnt =FAULTED_SIZE ; + written = -EFAULT ; }else -memcpy (&entry->buf, map_page[0 ] + offset, cnt); + written = cnt; + len = cnt;if (entry->buf[cnt -1 ] !='\n' ) { entry->buf[cnt] ='\n' ;
@@ -5854,12 +5800,8 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,__buffer_unlock_commit (buffer, event); - written = cnt; - - *fpos += written; - - out_unlock: -unlock_user_pages (pages, map_page, nr_pages); +if (written >0 ) + *fpos += written;return written; }
@@ -5875,15 +5817,14 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,struct ring_buffer_event *event;struct ring_buffer *buffer;struct raw_data_entry *entry; +const char faulted[] ="<faulted>" ;unsigned long irq_flags; -struct page *pages[2 ]; -void *map_page[2 ]; -int nr_pages =1 ;ssize_t written; -int offset;int size;int len; +#define FAULT_SIZE_ID (FAULTED_SIZE + sizeof(int)) +if (tracing_disabled)return -EINVAL ;
@@ -5899,38 +5840,32 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,BUILD_BUG_ON (TRACE_BUF_SIZE >=PAGE_SIZE ); - nr_pages =lock_user_pages (ubuf, cnt, pages, map_page, &offset); -if (nr_pages <0 ) -return nr_pages; -local_save_flags (irq_flags); size =sizeof (*entry) + cnt; +if (cnt <FAULT_SIZE_ID ) + size +=FAULT_SIZE_ID - cnt; + buffer = tr->trace_buffer.buffer; event =__trace_buffer_lock_reserve (buffer,TRACE_RAW_DATA , size, irq_flags,preempt_count ()); -if (!event) { +if (!event)/* Ring buffer disabled, return as if not open for write */ - written = -EBADF ; - goto out_unlock; - } +return -EBADF ; entry =ring_buffer_event_data (event); -if (nr_pages ==2 ) { - len =PAGE_SIZE - offset; -memcpy (&entry->id, map_page[0 ] + offset, len); -memcpy (((char *)&entry->id) + len, map_page[1 ], cnt - len); + len =__copy_from_user_inatomic (&entry->id, ubuf, cnt); +if (len) { + entry->id =-1 ; +memcpy (&entry->buf, faulted,FAULTED_SIZE ); + written = -EFAULT ; }else -memcpy (&entry->id, map_page[0 ] + offset, cnt); + written = cnt;__buffer_unlock_commit (buffer, event); - written = cnt; - - *fpos += written; - - out_unlock: -unlock_user_pages (pages, map_page, nr_pages); +if (written >0 ) + *fpos += written;return written; }
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 7/8 fgraph: Handle a case where a tracer ignores set_graph_notrace
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
Both the wakeup and irqsoff tracers can use the function graph tracer when the display-graph option is set. The problem is that they ignore the notrace file, and record the entry of functions that would be ignored by the function_graph tracer. This causes the trace->depth to be recorded into the ring buffer. The set_graph_notrace uses a trick by adding a large negative number to the trace->depth when a graph function is to be ignored.
On trace output, the graph function uses the depth to record a stack of functions. But since the depth is negative, it accesses the array with a negative number and causes an out of bounds access that can cause a kernel oops or corrupt data.
Have the print functions handle cases where a tracer still records functions even when they are in set_graph_notrace.
Also add warnings if the depth is below zero before accessing the array.
Note, the function graph logic will still prevent the return of these functions from being recorded, which means that they will be left hanging without a return. For example:
# echo '*spin*' > set_graph_notrace
# echo 1 > options/display-graph
# echo wakeup > current_tracer
# cat trace
[...]
_raw_spin_lock() {
preempt_count_add() {
do_raw_spin_lock() {
update_rq_clock();Where it should look like:
_raw_spin_lock() {
preempt_count_add();
do_raw_spin_lock();
}
update_rq_clock();--- kernel/trace/trace_functions_graph.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
kernel/trace/trace_functions_graph.c +14 -3
@@ -842,6 +842,10 @@ print_graph_entry_leaf(struct trace_iterator *iter, cpu_data =per_cpu_ptr (data->cpu_data, cpu); +/* If a graph tracer ignored set_graph_notrace */ +if (call->depth <-1 ) + call->depth +=FTRACE_NOTRACE_DEPTH ; +/* * Comments display at + 1 to depth. Since * this is a leaf function, keep the comments
@@ -850,7 +854,8 @@ print_graph_entry_leaf(struct trace_iterator *iter, cpu_data->depth = call->depth -1 ;/* No need to keep this function around for this depth */ -if (call->depth <FTRACE_RETFUNC_DEPTH ) +if (call->depth <FTRACE_RETFUNC_DEPTH && + !WARN_ON_ONCE (call->depth <0 )) cpu_data->enter_funcs[call->depth] =0 ; }
@@ -880,11 +885,16 @@ print_graph_entry_nested(struct trace_iterator *iter,struct fgraph_cpu_data *cpu_data;int cpu = iter->cpu; +/* If a graph tracer ignored set_graph_notrace */ +if (call->depth <-1 ) + call->depth +=FTRACE_NOTRACE_DEPTH ; + cpu_data =per_cpu_ptr (data->cpu_data, cpu); cpu_data->depth = call->depth;/* Save this function pointer to see if the exit matches */ -if (call->depth <FTRACE_RETFUNC_DEPTH ) +if (call->depth <FTRACE_RETFUNC_DEPTH && + !WARN_ON_ONCE (call->depth <0 )) cpu_data->enter_funcs[call->depth] = call->func; }
@@ -1114,7 +1124,8 @@ print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,*/ cpu_data->depth = trace->depth -1 ; -if (trace->depth <FTRACE_RETFUNC_DEPTH ) { +if (trace->depth <FTRACE_RETFUNC_DEPTH && + !WARN_ON_ONCE (trace->depth <0 )) {if (cpu_data->enter_funcs[trace->depth] != trace->func) func_match =0 ; cpu_data->enter_funcs[trace->depth] =0 ;
Signature · 1 line
2.10.2
A message that is not in the archive (sent off-list or not yet received): CADWwUUbhD0ZQbg6zN-A7A+f+jToadTx63UMQESoM04B75S+hvg@mail.gmail.com
A message that is not in the archive (sent off-list or not yet received): CADWwUUarxm+ACf1WqDo1B+RFCDMtGXzFYK45kbTbRm6s+QGyUA@mail.gmail.com
PATCH 7/8 fgraph: Handle a case where a tracer ignores set_graph_notrace
Hi Steve,
Quoted 9 lines · Steven Rostedt
From: "Steven Rostedt (Red Hat)" <[email protected]>
Both the wakeup and irqsoff tracers can use the function graph tracer when the display-graph option is set. The problem is that they ignore the notrace file, and record the entry of functions that would be ignored by the
function_graphtracer. This causes thetrace->depthto be recorded into the ring buffer. Theset_graph_notraceuses a trick by adding a large negative number to thetrace->depthwhen a graph function is to be ignored.
Quoted 4 lines
On trace output, the graph function uses the depth to record a stack of functions. But since the depth is negative, it accesses the array with a negative number and causes an out of bounds access that can cause a kernel oops or corrupt data.
Sorry to miss updating those tracers. I guess it's no more necessary once the patch 8 is applied so that functions in the notrace filter will not be recorded.
Or maybe we need to change the prepare_ftrace_return() so that the graph_entry callback should be called after ftrace_push_return_trace() as some archs do.
Quoted 3 lines
Have the print functions handle cases where a tracer still records functions even when they are in
set_graph_notrace.
I think it'd be better (or consistent, at least) not printing negative index records rather than showing entry only.
Quoted 17 lines
Also add warnings if the depth is below zero before accessing the array.
Note, the function graph logic will still prevent the return of these functions from being recorded, which means that they will be left hanging without a return. For example:
# echo '*spin*' > set_graph_notrace # echo 1 > options/display-graph # echo wakeup > current_tracer # cat trace [...] _raw_spin_lock() { preempt_count_add() { do_raw_spin_lock() { update_rq_clock();
Quoted 7 lines
Where it should look like:
_raw_spin_lock() { preempt_count_add(); do_raw_spin_lock(); } update_rq_clock();
If set_graph_notrace works correctly, it should be just:
update_rq_clock();
Thanks,
Namhyung
Quoted 66 lines
Cc Namhyung Kim <[email protected]>Fixes 29ad23b00474 ("ftrace: Add set_graph_notrace filter")Signed-off-by Steven Rostedt <[email protected]>--- kernel/trace/trace_functions_graph.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)kernel/trace/trace_functions_graph.c +14
-4@@ -842,6 +842,10 @@ print_graph_entry_leaf(struct trace_iterator *iter, cpu_data =per_cpu_ptr (data->cpu_data, cpu); +/* If a graph tracer ignored set_graph_notrace */ +if (call->depth <-1 ) + call->depth +=FTRACE_NOTRACE_DEPTH ; +/* * Comments display at + 1 to depth. Since * this is a leaf function, keep the comments @@ -850,7 +854,8 @@ print_graph_entry_leaf(struct trace_iterator *iter, cpu_data->depth = call->depth -1 ;/* No need to keep this function around for this depth */ -if (call->depth <FTRACE_RETFUNC_DEPTH ) +if (call->depth <FTRACE_RETFUNC_DEPTH && + !WARN_ON_ONCE (call->depth <0 )) cpu_data->enter_funcs[call->depth] =0 ; }@@ -880,11 +885,16 @@ print_graph_entry_nested(struct trace_iterator *iter,struct fgraph_cpu_data *cpu_data;int cpu = iter->cpu; +/* If a graph tracer ignored set_graph_notrace */ +if (call->depth <-1 ) + call->depth +=FTRACE_NOTRACE_DEPTH ; + cpu_data =per_cpu_ptr (data->cpu_data, cpu); cpu_data->depth = call->depth;/* Save this function pointer to see if the exit matches */ -if (call->depth <FTRACE_RETFUNC_DEPTH ) +if (call->depth <FTRACE_RETFUNC_DEPTH && + !WARN_ON_ONCE (call->depth <0 )) cpu_data->enter_funcs[call->depth] = call->func; }@@ -1114,7 +1124,8 @@ print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,*/ cpu_data->depth = trace->depth -1 ; -if (trace->depth <FTRACE_RETFUNC_DEPTH ) { +if (trace->depth <FTRACE_RETFUNC_DEPTH && + !WARN_ON_ONCE (trace->depth <0 )) {if (cpu_data->enter_funcs[trace->depth] != trace->func) func_match =0 ; cpu_data->enter_funcs[trace->depth] =0 ; --2.10.2
Steven Rostedt
maintainerRe: [for-next][PATCH 7/8] fgraph: Handle a case where a tracer ignores set_graph_notrace
Quoted 7 lines · On Tue, 13 Dec 2016 01:30:01 +0900 Namhyung Kim <[email protected]> wrote:
Sorry to miss updating those tracers. I guess it's no more necessary once the patch 8 is applied so that functions in the notrace filter will not be recorded.
Or maybe we need to change the
prepare_ftrace_return()so that thegraph_entrycallback should be called afterftrace_push_return_trace()as some archs do.
I plan on updating fgraph in general so this should all be handled then.
Quoted 7 lines
Quoted 4 lines
Have the print functions handle cases where a tracer still records functions even when they are in
set_graph_notrace.I think it'd be better (or consistent, at least) not printing negative index records rather than showing entry only.
I thought about this too, but I'm more concerned about it not crashing the kernel than to show a proper trace. The fix will just make sure it doesn't crash.
Quoted 26 lines
Quoted 18 lines
Also add warnings if the depth is below zero before accessing the array.
Note, the function graph logic will still prevent the return of these functions from being recorded, which means that they will be left hanging without a return. For example:
# echo '*spin*' > set_graph_notrace # echo 1 > options/display-graph # echo wakeup > current_tracer # cat trace [...] _raw_spin_lock() { preempt_count_add() { do_raw_spin_lock() { update_rq_clock();Quoted 7 lines
Where it should look like:
_raw_spin_lock() { preempt_count_add(); do_raw_spin_lock(); } update_rq_clock();
Quoted 3 lines
If
set_graph_notraceworks correctly, it should be just:update_rq_clock();
Which is what it should look like after patch 8. But I didn't mark 8 as stable as that's more of a feature. As wakeup and irqsoff doesn't use notrace yet. Yeah, notrace may break it a bit, but since this is the first someone noticed it, I don't think it's used much.
I wanted the simplest fix for stable.
-- Steve
Re: [for-next][PATCH 7/8] fgraph: Handle a case where a tracer ignores set_graph_notrace
Quoted 8 lines · Steven Rostedt
Quoted 4 lines · On Tue, 13 Dec 2016 01:30:01 +0900 Namhyung Kim <[email protected]> wrote:
Sorry to miss updating those tracers. I guess it's no more necessary once the patch 8 is applied so that functions in the notrace filter will not be recorded.
Quoted 5 lines
Quoted 3 lines
Or maybe we need to change the
prepare_ftrace_return()so that thegraph_entrycallback should be called afterftrace_push_return_trace()as some archs do.I plan on updating fgraph in general so this should all be handled then.
ok
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Have the print functions handle cases where a tracer still records functions even when they are in
set_graph_notrace.
Quoted 6 lines
Quoted 2 lines
I think it'd be better (or consistent, at least) not printing negative index records rather than showing entry only.
I thought about this too, but I'm more concerned about it not crashing the kernel than to show a proper trace. The fix will just make sure it doesn't crash.
ok
Quoted 29 lines
Quoted 29 lines
Quoted 19 lines
Also add warnings if the depth is below zero before accessing the array.
Note, the function graph logic will still prevent the return of these functions from being recorded, which means that they will be left hanging without a return. For example:
# echo '*spin*' > set_graph_notrace # echo 1 > options/display-graph # echo wakeup > current_tracer # cat trace [...] _raw_spin_lock() { preempt_count_add() { do_raw_spin_lock() { update_rq_clock();Quoted 7 lines
Where it should look like:
_raw_spin_lock() { preempt_count_add(); do_raw_spin_lock(); } update_rq_clock();If
set_graph_notraceworks correctly, it should be just:
Quoted 8 lines
Quoted 1 line
update_rq_clock();Which is what it should look like after patch 8. But I didn't mark 8 as stable as that's more of a feature. As wakeup and irqsoff doesn't use notrace yet. Yeah, notrace may break it a bit, but since this is the first someone noticed it, I don't think it's used much.
I wanted the simplest fix for stable.
I think a simpler fix is just to return when it sees a negative record..
kernel/trace/trace_functions_graph.c +2 -2
@@ -844,7 +844,7 @@ print_graph_entry_leaf(struct trace_iterator *iter,/* If a graph tracer ignored set_graph_notrace */ if (call->depth <-1 ) - call->depth +=FTRACE_NOTRACE_DEPTH ; +return TRACE_TYPE_HANDLED ;/* * Comments display at + 1 to depth. Since
@@ -887,7 +887,7 @@ print_graph_entry_nested(struct trace_iterator *iter,/* If a graph tracer ignored set_graph_notrace */ if (call->depth <-1 ) - call->depth +=FTRACE_NOTRACE_DEPTH ; +return TRACE_TYPE_HANDLED ; cpu_data =per_cpu_ptr (data->cpu_data, cpu); cpu_data->depth = call->depth;
Thanks,
Namhyung
Steven Rostedt
maintainerRe: [for-next][PATCH 7/8] fgraph: Handle a case where a tracer ignores set_graph_notrace
Quoted 3 lines · On Tue, 13 Dec 2016 02:09:04 +0900 Namhyung Kim <[email protected]> wrote:
Quoted 1 line
I wanted the simplest fix for stable.
I think a simpler fix is just to return when it sees a negative record..
You're right, but I guess I was trying to get it someone closer to the final change too.
-- Steve
Quoted 28 lines
kernel/trace/trace_functions_graph.c +2
-2@@ -844,7 +844,7 @@ print_graph_entry_leaf(struct trace_iterator *iter,/* If a graph tracer ignored set_graph_notrace */ if (call->depth <-1 ) - call->depth +=FTRACE_NOTRACE_DEPTH ; +return TRACE_TYPE_HANDLED ;/* * Comments display at + 1 to depth. Since @@ -887,7 +887,7 @@ print_graph_entry_nested(struct trace_iterator *iter,/* If a graph tracer ignored set_graph_notrace */ if (call->depth <-1 ) - call->depth +=FTRACE_NOTRACE_DEPTH ; +return TRACE_TYPE_HANDLED ; cpu_data =per_cpu_ptr (data->cpu_data, cpu); cpu_data->depth = call->depth;Thanks,
Namhyung
Steven Rostedt
maintainerPATCH 8/8 tracing/fgraph: Have wakeup and irqsoff tracers ignore graph functions too
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
Currently both the wakeup and irqsoff traces do not handle set_graph_notrace well. The ftrace infrastructure will ignore the return paths of all functions leaving them hanging without an end:
# echo '*spin*' > set_graph_notrace
# cat trace
[...]
_raw_spin_lock() {
preempt_count_add() {
do_raw_spin_lock() {
update_rq_clock();Where the '*spin*' functions should have looked like this:
_raw_spin_lock() {
preempt_count_add();
do_raw_spin_lock();
}
update_rq_clock();Instead, have the wakeup and irqsoff tracers ignore the functions that are set by the set_graph_notrace like the function_graph tracer does. Move the logic in the function_graph tracer into a header to allow wakeup and irqsoff tracers to use it as well.
--- kernel/trace/trace.h | 11 +++++++++++ kernel/trace/trace_functions_graph.c | 14 +++++++------- kernel/trace/trace_irqsoff.c | 12 ++++++++++++ kernel/trace/trace_sched_wakeup.c | 12 ++++++++++++ 4 files changed, 42 insertions(+), 7 deletions(-)
kernel/trace/trace.h +11 -0
@@ -846,6 +846,17 @@ static inline int ftrace_graph_notrace_addr(unsigned long addr) return 0; }#endif /* CONFIG_DYNAMIC_FTRACE */ + +extern unsigned int fgraph_max_depth; + +static inline bool ftrace_graph_ignore_func (struct ftrace_graph_ent *trace) +{ +/* trace it when it is-nested-in or is a function enabled. */ +return !(trace->depth ||ftrace_graph_addr (trace->func)) || + (trace->depth <0 ) || + (fgraph_max_depth && trace->depth >= fgraph_max_depth); +} +#else /* CONFIG_FUNCTION_GRAPH_TRACER */ static inline enum print_line_t print_graph_function_flags (struct trace_iterator *iter,u32 flags)
kernel/trace/trace_functions_graph.c +7 -7
@@ -65,7 +65,7 @@ struct fgraph_data {#define TRACE_GRAPH_INDENT 2 -static unsigned int max_depth; +unsigned int fgraph_max_depth;static struct tracer_opt trace_opts[] = {/* Display overruns? (for self-debug purpose) */
@@ -384,10 +384,10 @@ int trace_graph_entry(struct ftrace_graph_ent *trace)if (!ftrace_trace_task (tr))return 0 ; -/* trace it when it is-nested-in or is a function enabled. */ -if ((!(trace->depth ||ftrace_graph_addr (trace->func)) || -ftrace_graph_ignore_irqs ()) || (trace->depth <0 ) || - (max_depth && trace->depth >= max_depth)) +if (ftrace_graph_ignore_func (trace)) +return 0 ; + +if (ftrace_graph_ignore_irqs ())return 0 ;/*
@@ -1500,7 +1500,7 @@ graph_depth_write(struct file *filp, const char __user *ubuf, size_t cnt,if (ret)return ret; - max_depth = val; + fgraph_max_depth = val; *ppos += cnt;
@@ -1514,7 +1514,7 @@ graph_depth_read(struct file *filp, char __user *ubuf, size_t cnt,char buf[15 ];/* More than enough to hold UINT_MAX + "\n"*/ int n; - n =sprintf (buf,"%d\n" , max_depth); + n =sprintf (buf,"%d\n" , fgraph_max_depth);return simple_read_from_buffer (ubuf, cnt, ppos, buf, n); }
kernel/trace/trace_irqsoff.c +12 -0
@@ -175,6 +175,18 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace)int ret;int pc; +if (ftrace_graph_ignore_func (trace)) +return 0 ; +/* +* Do not trace a function if it's filtered by set_graph_notrace. +* Make the index of ret stack negative to indicate that it should +* ignore further functions. But it needs its own ret stack entry +* to recover the original index in order to continue tracing after +* returning from the function. +*/ +if (ftrace_graph_notrace_addr (trace->func)) +return 1 ; +if (!func_prolog_dec (tr, &data, &flags))return 0 ;
kernel/trace/trace_sched_wakeup.c +12 -0
@@ -239,6 +239,18 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace)unsigned long flags;int pc, ret =0 ; +if (ftrace_graph_ignore_func (trace)) +return 0 ; +/* +* Do not trace a function if it's filtered by set_graph_notrace. +* Make the index of ret stack negative to indicate that it should +* ignore further functions. But it needs its own ret stack entry +* to recover the original index in order to continue tracing after +* returning from the function. +*/ +if (ftrace_graph_notrace_addr (trace->func)) +return 1 ; +if (!func_prolog_preempt_disable (tr, &data, &pc))return 0 ;
Signature · 1 line
2.10.2
Re: [for-next][PATCH 8/8] tracing/fgraph: Have wakeup and irqsoff tracers ignore graph functions too
Quoted 22 lines · Namhyung Kim
From: "Steven Rostedt (Red Hat)" <[email protected]>
Currently both the wakeup and irqsoff traces do not handle
set_graph_notracewell. The ftrace infrastructure will ignore the return paths of all functions leaving them hanging without an end:# echo '*spin*' > set_graph_notrace # cat trace [...] _raw_spin_lock() { preempt_count_add() { do_raw_spin_lock() { update_rq_clock();Where the '*spin*' functions should have looked like this:
_raw_spin_lock() { preempt_count_add(); do_raw_spin_lock(); } update_rq_clock();
Quoted 6 lines
Instead, have the wakeup and irqsoff tracers ignore the functions that are set by the
set_graph_notracelike thefunction_graphtracer does. Move the logic in thefunction_graphtracer into a header to allow wakeup and irqsoff tracers to use it as well.Signed-off-by Steven Rostedt <[email protected]>
Thanks,
Namhyung
Quoted 130 lines
--- kernel/trace/trace.h | 11 +++++++++++ kernel/trace/trace_functions_graph.c | 14 +++++++------- kernel/trace/trace_irqsoff.c | 12 ++++++++++++ kernel/trace/trace_sched_wakeup.c | 12 ++++++++++++ 4 files changed, 42 insertions(+), 7 deletions(-)kernel/trace/trace.h +11
-0@@ -846,6 +846,17 @@ static inline intftrace_graph_notrace_addr (unsigned long addr ) return 0; }#endif /* CONFIG_DYNAMIC_FTRACE */ + +extern unsigned int fgraph_max_depth; + +static inline bool ftrace_graph_ignore_func (struct ftrace_graph_ent *trace) +{ +/* trace it when it is-nested-in or is a function enabled. */ +return !(trace->depth ||ftrace_graph_addr (trace->func)) || + (trace->depth <0 ) || + (fgraph_max_depth && trace->depth >= fgraph_max_depth); +} +#else /* CONFIG_FUNCTION_GRAPH_TRACER */ static inline enum print_line_t
print_graph_function_flags(struct trace_iterator *iter, u32 flags)kernel/trace/trace_functions_graph.c +7
-7@@ -65,7 +65,7 @@ struct fgraph_data {#define TRACE_GRAPH_INDENT 2 -static unsigned int max_depth; +unsigned int fgraph_max_depth;static struct tracer_opt trace_opts[] = {/* Display overruns? (for self-debug purpose) */ @@ -384,10 +384,10 @@ int trace_graph_entry(struct ftrace_graph_ent *trace)if (!ftrace_trace_task (tr))return 0 ; -/* trace it when it is-nested-in or is a function enabled. */ -if ((!(trace->depth ||ftrace_graph_addr (trace->func)) || -ftrace_graph_ignore_irqs ()) || (trace->depth <0 ) || - (max_depth && trace->depth >= max_depth)) +if (ftrace_graph_ignore_func (trace)) +return 0 ; + +if (ftrace_graph_ignore_irqs ())return 0 ;/* @@ -1500,7 +1500,7 @@ graph_depth_write(struct file *filp, const char __user *ubuf, size_t cnt, if (ret)return ret; - max_depth = val; + fgraph_max_depth = val; *ppos += cnt;@@ -1514,7 +1514,7 @@ graph_depth_read(struct file *filp, char __user *ubuf, size_t cnt, char buf[15]; /* More than enough to hold UINT_MAX + "\n"*/int n; - n =sprintf (buf,"%d\n" , max_depth); + n =sprintf (buf,"%d\n" , fgraph_max_depth);return simple_read_from_buffer (ubuf, cnt, ppos, buf, n);}
kernel/trace/trace_irqsoff.c +12
-0@@ -175,6 +175,18 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace)int ret;int pc; +if (ftrace_graph_ignore_func (trace)) +return 0 ; +/* +* Do not trace a function if it's filtered by set_graph_notrace. +* Make the index of ret stack negative to indicate that it should +* ignore further functions. But it needs its own ret stack entry +* to recover the original index in order to continue tracing after +* returning from the function. +*/ +if (ftrace_graph_notrace_addr (trace->func)) +return 1 ; +if (!func_prolog_dec (tr, &data, &flags))return 0 ;kernel/trace/trace_sched_wakeup.c +12
-1@@ -239,6 +239,18 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace)unsigned long flags;int pc, ret =0 ; +if (ftrace_graph_ignore_func (trace)) +return 0 ; +/* +* Do not trace a function if it's filtered by set_graph_notrace. +* Make the index of ret stack negative to indicate that it should +* ignore further functions. But it needs its own ret stack entry +* to recover the original index in order to continue tracing after +* returning from the function. +*/ +if (ftrace_graph_notrace_addr (trace->func)) +return 1 ; +if (!func_prolog_preempt_disable (tr, &data, &pc))return 0 ; --2.10.2