tracing: Updates for 3.5
Subsystems (5)
Steven Rostedt
maintainerIngo,
This should be my last set of changes for the next merge window.
Please pull the latest tip/perf/core tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
tip/perf/core
Head SHA1: b732d439cb43336cd6d7e804ecb2c81193ef63b0
Namhyung Kim (1):
tracing: Check return value of tracing_dentry_percpu()
Steven Rostedt (11):
tracing: Clean up tracing_mark_write()
ring-buffer: Add integrity check at end of iter read
ring-buffer: Reset head page before running self test
ftrace: Sort all function addresses, not just per page
ftrace: Remove extra helper functions
ftrace: Speed up search by skipping pages by address
ftrace: Consolidate ftrace_location() and ftrace_text_reserved()
ftrace: Return record ip addr for ftrace_location()
ftrace: Make ftrace_modify_all_code() global for archs to use ftrace/x86: Have x86 ftrace use the ftrace_modify_all_code()
ftrace: Remove selecting FRAME_POINTER with FUNCTION_TRACER
Vaibhav Nagarnaik (3):
ring-buffer: Make removal of ring buffer pages atomic
ring-buffer: Make addition of pages in ring buffer atomic
tracing: change CPU ring buffer state from tracing_cpumask
----
arch/x86/kernel/ftrace.c | 15 +- include/asm-generic/vmlinux.lds.h | 2 +- include/linux/ftrace.h | 5 +- kernel/trace/Kconfig | 1 - kernel/trace/ftrace.c | 198 ++++++++++--------- kernel/trace/ring_buffer.c | 394 +++++++++++++++++++++++++++++-------- kernel/trace/trace.c | 49 ++--- 7 files changed, 446 insertions(+), 218 deletions(-)
Steven Rostedt
maintainerPATCH 1/15 tracing: Clean up tracing_mark_write()
From: Steven Rostedt <[email protected]>
On gcc 4.5 the function tracing_mark_write() would give a warning of page2 being uninitialized. This is due to a bug in gcc because the logic prevents page2 from being used uninitialized, and gcc 4.6+ does not complain (correctly).
Instead of adding a "unitialized" around page2, which could show a bug later on, I combined page1 and page2 into an array map_pages[]. This binds the two and the two are modified according to nr_pages (what gcc 4.5 seems to ignore). This no longer gives a warning with gcc 4.5 nor with gcc 4.6.
--- kernel/trace/trace.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-)
kernel/trace/trace.c +11 -13
@@ -3875,14 +3875,14 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,struct print_entry *entry;unsigned long irq_flags;struct page *pages[2 ]; +void *map_page[2 ];int nr_pages =1 ;ssize_t written; -void *page1; -void *page2;int offset;int size;int len;int ret; +int i;if (tracing_disabled)return -EINVAL ;
@@ -3921,9 +3921,8 @@ tracing_mark_write(struct file *filp, const char __user *ubuf, goto out; } - page1 =kmap_atomic (pages[0 ]); -if (nr_pages ==2 ) - page2 =kmap_atomic (pages[1 ]); +for (i =0 ; i < nr_pages; i++) + map_page[i] =kmap_atomic (pages[i]);local_save_flags (irq_flags); size =sizeof (*entry) + cnt +2 ;/* possible \n added */
@@ -3941,10 +3940,10 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,if (nr_pages ==2 ) { len =PAGE_SIZE - offset; -memcpy (&entry->buf, page1 + offset, len); -memcpy (&entry->buf[len], page2, cnt - len); +memcpy (&entry->buf, map_page[0 ] + offset, len); +memcpy (&entry->buf[len], map_page[1 ], cnt - len); }else -memcpy (&entry->buf, page1 + offset, cnt); +memcpy (&entry->buf, map_page[0 ] + offset, cnt);if (entry->buf[cnt -1 ] !='\n' ) { entry->buf[cnt] ='\n' ;
@@ -3959,11 +3958,10 @@ tracing_mark_write(struct file *filp, const char __user *ubuf, *fpos += written; out_unlock: -if (nr_pages ==2 ) -kunmap_atomic (page2); -kunmap_atomic (page1); -while (nr_pages >0 ) -put_page (pages[--nr_pages]); +for (i =0 ; i < nr_pages; i++){ +kunmap_atomic (map_page[i]); +put_page (pages[i]); + } out:return written; }
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 2/15 ring-buffer: Make removal of ring buffer pages atomic
In reply to Steven RostedtFrom: Vaibhav Nagarnaik <[email protected]>
This patch adds the capability to remove pages from a ring buffer without destroying any existing data in it.
This is done by removing the pages after the tail page. This makes sure that first all the empty pages in the ring buffer are removed. If the head page is one in the list of pages to be removed, then the page after the removed ones is made the head page. This removes the oldest data from the ring buffer and keeps the latest data around to be read.
To do this in a non-racey manner, tracing is stopped for a very short time while the pages to be removed are identified and unlinked from the ring buffer. The pages are freed after the tracing is restarted to minimize the time needed to stop tracing.
The context in which the pages from the per-cpu ring buffer are removed runs on the respective CPU. This minimizes the events not traced to only NMI trace contexts.
--- kernel/trace/ring_buffer.c | 265 ++++++++++++++++++++++++++++++++++---------- kernel/trace/trace.c | 20 +--- 2 files changed, 209 insertions(+), 76 deletions(-)
kernel/trace/ring_buffer.c +208 -57
@@ -23,6 +23,8 @@#include <asm/local.h> #include "trace.h" +static void update_pages_handler (struct work_struct *work); +/* * The ring buffer header is special. We must manually up keep it. */
@@ -470,12 +472,15 @@ struct ring_buffer_per_cpu {/* ring buffer pages to update, > 0 to add, < 0 to remove */ int nr_pages_to_update;struct list_head new_pages;/* new pages to add */ +struct work_struct update_pages_work; +struct completion update_completion; };struct ring_buffer {unsigned flags;int cpus;atomic_t record_disabled; +atomic_t resize_disabled;cpumask_var_t cpumask;struct lock_class_key *reader_lock_key;
@@ -1048,6 +1053,8 @@ rb_allocate_cpu_buffer(struct ring_buffer *buffer, int nr_pages, int cpu)raw_spin_lock_init (&cpu_buffer->reader_lock);lockdep_set_class (&cpu_buffer->reader_lock, buffer->reader_lock_key); cpu_buffer->lock = (arch_spinlock_t )__ARCH_SPIN_LOCK_UNLOCKED; +INIT_WORK (&cpu_buffer->update_pages_work, update_pages_handler); +init_completion (&cpu_buffer->update_completion); bpage =kzalloc_node (ALIGN (sizeof (*bpage),cache_line_size ()),GFP_KERNEL ,cpu_to_node (cpu));
@@ -1235,32 +1242,123 @@ void ring_buffer_set_clock(struct ring_buffer *buffer,static void rb_reset_cpu (struct ring_buffer_per_cpu *cpu_buffer); +static inline unsigned long rb_page_entries (struct buffer_page *bpage) +{ +return local_read (&bpage->entries) &RB_WRITE_MASK ; +} + +static inline unsigned long rb_page_write (struct buffer_page *bpage) +{ +return local_read (&bpage->write) &RB_WRITE_MASK ; +} +static void -rb_remove_pages (struct ring_buffer_per_cpu *cpu_buffer,unsigned nr_pages) +rb_remove_pages (struct ring_buffer_per_cpu *cpu_buffer,unsigned int nr_pages) { -struct buffer_page *bpage; -struct list_head *p; -unsigned i; +struct list_head *tail_page, *to_remove, *next_page; +struct buffer_page *to_remove_page, *tmp_iter_page; +struct buffer_page *last_page, *first_page; +unsigned int nr_removed; +unsigned long head_bit; +int page_entries; + + head_bit =0 ;raw_spin_lock_irq (&cpu_buffer->reader_lock); -rb_head_page_deactivate (cpu_buffer); +atomic_inc (&cpu_buffer->record_disabled); +/* +* We don't race with the readers since we have acquired the reader +* lock. We also don't race with writers after disabling recording. +* This makes it easy to figure out the first and the last page to be +* removed from the list. We unlink all the pages in between including +* the first and last pages. This is done in a busy loop so that we +* lose the least number of traces. +* The pages are freed after we restart recording and unlock readers. +*/ + tail_page = &cpu_buffer->tail_page->list; -for (i =0 ; i < nr_pages; i++) { -if (RB_WARN_ON (cpu_buffer,list_empty (cpu_buffer->pages))) - goto out; - p = cpu_buffer->pages->next; - bpage = list_entry(p, struct buffer_page, list); -list_del_init (&bpage->list); -free_buffer_page (bpage); +/* +* tail page might be on reader page, we remove the next page +* from the ring buffer +*/ +if (cpu_buffer->tail_page == cpu_buffer->reader_page) + tail_page =rb_list_head (tail_page->next); + to_remove = tail_page; + +/* start of pages to remove */ + first_page =list_entry (rb_list_head (to_remove->next), + struct buffer_page, list); + +for (nr_removed =0 ; nr_removed < nr_pages; nr_removed++) { + to_remove =rb_list_head (to_remove)->next; + head_bit |= (unsigned long )to_remove &RB_PAGE_HEAD ; } -if (RB_WARN_ON (cpu_buffer,list_empty (cpu_buffer->pages))) - goto out; -rb_reset_cpu (cpu_buffer); -rb_check_pages (cpu_buffer); + next_page =rb_list_head (to_remove)->next; -out: +/* +* Now we remove all pages between tail_page and next_page. +* Make sure that we have head_bit value preserved for the +* next page +*/ + tail_page->next = (struct list_head *)((unsigned long )next_page | + head_bit); + next_page =rb_list_head (next_page); + next_page->prev = tail_page; + +/* make sure pages points to a valid page in the ring buffer */ + cpu_buffer->pages = next_page; + +/* update head page */ +if (head_bit) + cpu_buffer->head_page =list_entry (next_page, + struct buffer_page, list); + +/* +* change read pointer to make sure any read iterators reset +* themselves +*/ + cpu_buffer->read =0 ; + +/* pages are removed, resume tracing and then free the pages */ +atomic_dec (&cpu_buffer->record_disabled);raw_spin_unlock_irq (&cpu_buffer->reader_lock); + +RB_WARN_ON (cpu_buffer,list_empty (cpu_buffer->pages)); + +/* last buffer page to remove */ + last_page = list_entry(rb_list_head(to_remove), struct buffer_page, + list); + tmp_iter_page = first_page; + +do { + to_remove_page = tmp_iter_page; +rb_inc_page (cpu_buffer, &tmp_iter_page); + +/* update the counters */ + page_entries =rb_page_entries (to_remove_page); +if (page_entries) { +/* +* If something was added to this page, it was full +* since it is not the tail page. So we deduct the +* bytes consumed in ring buffer from here. +* No need to update overruns, since this page is +* deleted from ring buffer and its entries are +* already accounted for. +*/ +local_sub (BUF_PAGE_SIZE , &cpu_buffer->entries_bytes); + } + +/* +* We have already removed references to this list item, just +* free up the buffer_page and its page +*/ +free_buffer_page (to_remove_page); + nr_removed--; + + }while (to_remove_page != last_page); + +RB_WARN_ON (cpu_buffer, nr_removed); } static void
@@ -1272,6 +1370,8 @@ rb_insert_pages(struct ring_buffer_per_cpu *cpu_buffer,unsigned i;raw_spin_lock_irq (&cpu_buffer->reader_lock); +/* stop the writers while inserting pages */ +atomic_inc (&cpu_buffer->record_disabled);rb_head_page_deactivate (cpu_buffer);for (i =0 ; i < nr_pages; i++) {
@@ -1286,19 +1386,27 @@ rb_insert_pages(struct ring_buffer_per_cpu *cpu_buffer,rb_check_pages (cpu_buffer); out: +atomic_dec (&cpu_buffer->record_disabled);raw_spin_unlock_irq (&cpu_buffer->reader_lock); } -static void update_pages_handler (struct ring_buffer_per_cpu *cpu_buffer) +static void rb_update_pages (struct ring_buffer_per_cpu *cpu_buffer) {if (cpu_buffer->nr_pages_to_update >0 )rb_insert_pages (cpu_buffer, &cpu_buffer->new_pages, cpu_buffer->nr_pages_to_update);else rb_remove_pages (cpu_buffer, -cpu_buffer->nr_pages_to_update); + cpu_buffer->nr_pages += cpu_buffer->nr_pages_to_update; -/* reset this value */ - cpu_buffer->nr_pages_to_update =0 ; +} + +static void update_pages_handler (struct work_struct *work) +{ +struct ring_buffer_per_cpu *cpu_buffer =container_of (work, + struct ring_buffer_per_cpu, update_pages_work); +rb_update_pages (cpu_buffer); +complete (&cpu_buffer->update_completion); }/**
@@ -1308,14 +1416,14 @@ static void update_pages_handler(struct ring_buffer_per_cpu *cpu_buffer)* * Minimum size is 2 * BUF_PAGE_SIZE. * -* Returns -1 on failure. +* Returns 0 on success and < 0 on failure. */ int ring_buffer_resize (struct ring_buffer *buffer,unsigned long size,int cpu_id) {struct ring_buffer_per_cpu *cpu_buffer;unsigned nr_pages; -int cpu; +int cpu, err =0 ;/* * Always succeed at resizing a non-existent buffer:
@@ -1330,15 +1438,18 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size,if (size <BUF_PAGE_SIZE *2 ) size =BUF_PAGE_SIZE *2 ; -atomic_inc (&buffer->record_disabled); + nr_pages =DIV_ROUND_UP (size,BUF_PAGE_SIZE ); -/* Make sure all writers are done with this buffer. */ -synchronize_sched (); +/* +* Don't succeed if resizing is disabled, as a reader might be +* manipulating the ring buffer and is expecting a sane state while +* this is true. +*/ +if (atomic_read (&buffer->resize_disabled)) +return -EBUSY ; +/* prevent another thread from changing buffer sizes */ mutex_lock (&buffer->mutex); -get_online_cpus (); - - nr_pages =DIV_ROUND_UP (size,BUF_PAGE_SIZE );if (cpu_id ==RING_BUFFER_ALL_CPUS ) {/* calculate the pages to update */
@@ -1347,33 +1458,67 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size, cpu_buffer->nr_pages_to_update = nr_pages - cpu_buffer->nr_pages; -/* * nothing more to do for removing pages or no update */ if (cpu_buffer->nr_pages_to_update <=0 )continue ; -/* * to add pages, make sure all new pages can be * allocated without receiving ENOMEM */ INIT_LIST_HEAD (&cpu_buffer->new_pages);if (__rb_allocate_pages (cpu_buffer->nr_pages_to_update, - &cpu_buffer->new_pages, cpu)) + &cpu_buffer->new_pages, cpu)) {/* not enough memory for new pages */ - goto no_mem; + err = -ENOMEM ; + goto out_err; + } + } + +get_online_cpus (); +/* +* Fire off all the required work handlers +* Look out for offline CPUs +*/ +for_each_buffer_cpu (buffer, cpu) { + cpu_buffer = buffer->buffers[cpu]; +if (!cpu_buffer->nr_pages_to_update || + !cpu_online (cpu)) +continue ; + +schedule_work_on (cpu, &cpu_buffer->update_pages_work); + } +/* +* This loop is for the CPUs that are not online. +* We can't schedule anything on them, but it's not necessary +* since we can change their buffer sizes without any race. +*/ +for_each_buffer_cpu (buffer, cpu) { + cpu_buffer = buffer->buffers[cpu]; +if (!cpu_buffer->nr_pages_to_update || +cpu_online (cpu)) +continue ; + +rb_update_pages (cpu_buffer); }/* wait for all the updates to complete */ for_each_buffer_cpu (buffer, cpu) { cpu_buffer = buffer->buffers[cpu]; -if (cpu_buffer->nr_pages_to_update) { -update_pages_handler (cpu_buffer); - } +if (!cpu_buffer->nr_pages_to_update || + !cpu_online (cpu)) +continue ; + +wait_for_completion (&cpu_buffer->update_completion); +/* reset this value */ + cpu_buffer->nr_pages_to_update =0 ; } + +put_online_cpus (); }else { cpu_buffer = buffer->buffers[cpu_id]; +if (nr_pages == cpu_buffer->nr_pages) goto out;
@@ -1383,38 +1528,47 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size,INIT_LIST_HEAD (&cpu_buffer->new_pages);if (cpu_buffer->nr_pages_to_update >0 &&__rb_allocate_pages (cpu_buffer->nr_pages_to_update, - &cpu_buffer->new_pages, cpu_id)) - goto no_mem; + &cpu_buffer->new_pages, cpu_id)) { + err = -ENOMEM ; + goto out_err; + } -update_pages_handler (cpu_buffer); +get_online_cpus (); + +if (cpu_online (cpu_id)) { +schedule_work_on (cpu_id, + &cpu_buffer->update_pages_work); +wait_for_completion (&cpu_buffer->update_completion); + }else +rb_update_pages (cpu_buffer); + +put_online_cpus (); +/* reset this value */ + cpu_buffer->nr_pages_to_update =0 ; } out: -put_online_cpus ();mutex_unlock (&buffer->mutex); - -atomic_dec (&buffer->record_disabled); -return size; - no_mem: + out_err:for_each_buffer_cpu (buffer, cpu) {struct buffer_page *bpage, *tmp; + cpu_buffer = buffer->buffers[cpu]; -/* reset this number regardless */ cpu_buffer->nr_pages_to_update =0 ; +if (list_empty (&cpu_buffer->new_pages))continue ; +list_for_each_entry_safe (bpage, tmp, &cpu_buffer->new_pages, list) {list_del_init (&bpage->list);free_buffer_page (bpage); } } -put_online_cpus ();mutex_unlock (&buffer->mutex); -atomic_dec (&buffer->record_disabled); -return -ENOMEM ; +return err; }EXPORT_SYMBOL_GPL (ring_buffer_resize);
@@ -1453,21 +1607,11 @@ rb_iter_head_event(struct ring_buffer_iter *iter)return __rb_page_index (iter->head_page, iter->head); } -static inline unsigned long rb_page_write (struct buffer_page *bpage) -{ -return local_read (&bpage->write) &RB_WRITE_MASK ; -} -static inline unsigned rb_page_commit (struct buffer_page *bpage) {return local_read (&bpage->page->commit); } -static inline unsigned long rb_page_entries (struct buffer_page *bpage) -{ -return local_read (&bpage->entries) &RB_WRITE_MASK ; -} -/* Size is determined by what has been committed */ static inline unsigned rb_page_size (struct buffer_page *bpage) {
@@ -3492,6 +3636,7 @@ ring_buffer_read_prepare(struct ring_buffer *buffer, int cpu) iter->cpu_buffer = cpu_buffer; +atomic_inc (&buffer->resize_disabled);atomic_inc (&cpu_buffer->record_disabled);return iter;
@@ -3555,6 +3700,7 @@ ring_buffer_read_finish(struct ring_buffer_iter *iter)struct ring_buffer_per_cpu *cpu_buffer = iter->cpu_buffer;atomic_dec (&cpu_buffer->record_disabled); +atomic_dec (&cpu_buffer->buffer->resize_disabled);kfree (iter); }EXPORT_SYMBOL_GPL (ring_buffer_read_finish);
@@ -3662,8 +3808,12 @@ void ring_buffer_reset_cpu(struct ring_buffer *buffer, int cpu)if (!cpumask_test_cpu (cpu, buffer->cpumask))return ; +atomic_inc (&buffer->resize_disabled);atomic_inc (&cpu_buffer->record_disabled); +/* Make sure all commits have finished */ +synchronize_sched (); +raw_spin_lock_irqsave (&cpu_buffer->reader_lock, flags);if (RB_WARN_ON (cpu_buffer,local_read (&cpu_buffer->committing)))
@@ -3679,6 +3829,7 @@ void ring_buffer_reset_cpu(struct ring_buffer *buffer, int cpu)raw_spin_unlock_irqrestore (&cpu_buffer->reader_lock, flags);atomic_dec (&cpu_buffer->record_disabled); +atomic_dec (&buffer->resize_disabled); }EXPORT_SYMBOL_GPL (ring_buffer_reset_cpu);
kernel/trace/trace.c +1 -19
@@ -3076,20 +3076,10 @@ static int __tracing_resize_ring_buffer(unsigned long size, int cpu)static ssize_t tracing_resize_ring_buffer (unsigned long size,int cpu_id) { -int cpu, ret = size; +int ret = size;mutex_lock (&trace_types_lock); -tracing_stop (); - -/* disable all cpu buffers */ -for_each_tracing_cpu (cpu) { -if (global_trace.data[cpu]) -atomic_inc (&global_trace.data[cpu]->disabled); -if (max_tr.data[cpu]) -atomic_inc (&max_tr.data[cpu]->disabled); - } -if (cpu_id !=RING_BUFFER_ALL_CPUS ) {/* make sure, this cpu is enabled in the mask */ if (!cpumask_test_cpu (cpu_id, tracing_buffer_mask)) {
@@ -3103,14 +3093,6 @@ static ssize_t tracing_resize_ring_buffer(unsigned long size, int cpu_id) ret = -ENOMEM ; out: -for_each_tracing_cpu (cpu) { -if (global_trace.data[cpu]) -atomic_dec (&global_trace.data[cpu]->disabled); -if (max_tr.data[cpu]) -atomic_dec (&max_tr.data[cpu]->disabled); - } - -tracing_start ();mutex_unlock (&trace_types_lock);return ret;
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 3/15 ring-buffer: Make addition of pages in ring buffer atomic
In reply to Steven RostedtFrom: Vaibhav Nagarnaik <[email protected]>
This patch adds the capability to add new pages to a ring buffer atomically while write operations are going on. This makes it possible to expand the ring buffer size without reinitializing the ring buffer.
The new pages are attached between the head page and its previous page.
--- kernel/trace/ring_buffer.c | 102 +++++++++++++++++++++++++++++++++----------- 1 file changed, 77 insertions(+), 25 deletions(-)
kernel/trace/ring_buffer.c +77 -25
@@ -1252,7 +1252,7 @@ static inline unsigned long rb_page_write(struct buffer_page *bpage)return local_read (&bpage->write) &RB_WRITE_MASK ; } -static void +static int rb_remove_pages (struct ring_buffer_per_cpu *cpu_buffer,unsigned int nr_pages) {struct list_head *tail_page, *to_remove, *next_page;
@@ -1359,46 +1359,97 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned int nr_pages) }while (to_remove_page != last_page);RB_WARN_ON (cpu_buffer, nr_removed); + +return nr_removed ==0 ; } -static void -rb_insert_pages (struct ring_buffer_per_cpu *cpu_buffer, -struct list_head *pages,unsigned nr_pages) +static int +rb_insert_pages (struct ring_buffer_per_cpu *cpu_buffer) { -struct buffer_page *bpage; -struct list_head *p; -unsigned i; +struct list_head *pages = &cpu_buffer->new_pages; +int retries, success;raw_spin_lock_irq (&cpu_buffer->reader_lock); -/* stop the writers while inserting pages */ -atomic_inc (&cpu_buffer->record_disabled); -rb_head_page_deactivate (cpu_buffer); +/* +* We are holding the reader lock, so the reader page won't be swapped +* in the ring buffer. Now we are racing with the writer trying to +* move head page and the tail page. +* We are going to adapt the reader page update process where: +* 1. We first splice the start and end of list of new pages between +* the head page and its previous page. +* 2. We cmpxchg the prev_page->next to point from head page to the +* start of new pages list. +* 3. Finally, we update the head->prev to the end of new list. +* +* We will try this process 10 times, to make sure that we don't keep +* spinning. +*/ + retries =10 ; + success =0 ; +while (retries--) { +struct list_head *head_page, *prev_page, *r; +struct list_head *last_page, *first_page; +struct list_head *head_page_with_bit; -for (i =0 ; i < nr_pages; i++) { -if (RB_WARN_ON (cpu_buffer,list_empty (pages))) - goto out; - p = pages->next; - bpage = list_entry(p, struct buffer_page, list); -list_del_init (&bpage->list); -list_add_tail (&bpage->list, cpu_buffer->pages); + head_page = &rb_set_head_page (cpu_buffer)->list; + prev_page = head_page->prev; + + first_page = pages->next; + last_page = pages->prev; + + head_page_with_bit = (struct list_head *) + ((unsigned long )head_page |RB_PAGE_HEAD ); + + last_page->next = head_page_with_bit; + first_page->prev = prev_page; + + r =cmpxchg (&prev_page->next, head_page_with_bit, first_page); + +if (r == head_page_with_bit) { +/* +* yay, we replaced the page pointer to our new list, +* now, we just have to update to head page's prev +* pointer to point to end of list +*/ + head_page->prev = last_page; + success =1 ; +break ; + } } -rb_reset_cpu (cpu_buffer); -rb_check_pages (cpu_buffer); -out: -atomic_dec (&cpu_buffer->record_disabled); +if (success) +INIT_LIST_HEAD (pages); +/* +* If we weren't successful in adding in new pages, warn and stop +* tracing +*/ +RB_WARN_ON (cpu_buffer, !success);raw_spin_unlock_irq (&cpu_buffer->reader_lock); + +/* free pages if they weren't inserted */ +if (!success) { +struct buffer_page *bpage, *tmp; +list_for_each_entry_safe (bpage, tmp, &cpu_buffer->new_pages, + list) { +list_del_init (&bpage->list); +free_buffer_page (bpage); + } + } +return success; }static void rb_update_pages (struct ring_buffer_per_cpu *cpu_buffer) { +int success; +if (cpu_buffer->nr_pages_to_update >0 ) -rb_insert_pages (cpu_buffer, &cpu_buffer->new_pages, - cpu_buffer->nr_pages_to_update); + success =rb_insert_pages (cpu_buffer);else -rb_remove_pages (cpu_buffer, -cpu_buffer->nr_pages_to_update); + success =rb_remove_pages (cpu_buffer, + -cpu_buffer->nr_pages_to_update); - cpu_buffer->nr_pages += cpu_buffer->nr_pages_to_update; +if (success) + cpu_buffer->nr_pages += cpu_buffer->nr_pages_to_update; }static void update_pages_handler (struct work_struct *work)
@@ -3772,6 +3823,7 @@ rb_reset_cpu(struct ring_buffer_per_cpu *cpu_buffer) cpu_buffer->commit_page = cpu_buffer->head_page;INIT_LIST_HEAD (&cpu_buffer->reader_page->list); +INIT_LIST_HEAD (&cpu_buffer->new_pages);local_set (&cpu_buffer->reader_page->write,0 );local_set (&cpu_buffer->reader_page->entries,0 );local_set (&cpu_buffer->reader_page->page->commit,0 );
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 4/15 ring-buffer: Add integrity check at end of iter read
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
There use to be ring buffer integrity checks after updating the size of the ring buffer. But now that the ring buffer can modify the size while the system is running, the integrity checks were removed, as they require the ring buffer to be disabed to perform the check.
Move the integrity check to the reading of the ring buffer via the iterator reads (the "trace" file). As reading via an iterator requires disabling the ring buffer, it is a perfect place to have it.
If the ring buffer happens to be disabled when updating the size, we still perform the integrity check.
--- kernel/trace/ring_buffer.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
kernel/trace/ring_buffer.c +29 -0
@@ -1599,6 +1599,29 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size, } out: +/* +* The ring buffer resize can happen with the ring buffer +* enabled, so that the update disturbs the tracing as little +* as possible. But if the buffer is disabled, we do not need +* to worry about that, and we can take the time to verify +* that the buffer is not corrupt. +*/ +if (atomic_read (&buffer->record_disabled)) { +atomic_inc (&buffer->record_disabled); +/* +* Even though the buffer was disabled, we must make sure +* that it is truly disabled before calling rb_check_pages. +* There could have been a race between checking +* record_disable and incrementing it. +*/ +synchronize_sched (); +for_each_buffer_cpu (buffer, cpu) { + cpu_buffer = buffer->buffers[cpu]; +rb_check_pages (cpu_buffer); + } +atomic_dec (&buffer->record_disabled); + } +mutex_unlock (&buffer->mutex);return size;
@@ -3750,6 +3773,12 @@ ring_buffer_read_finish(struct ring_buffer_iter *iter) {struct ring_buffer_per_cpu *cpu_buffer = iter->cpu_buffer; +/* +* Ring buffer is disabled from recording, here's a good place +* to check the integrity of the ring buffer. +*/ +rb_check_pages (cpu_buffer); +atomic_dec (&cpu_buffer->record_disabled);atomic_dec (&cpu_buffer->buffer->resize_disabled);kfree (iter);
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 5/15 ring-buffer: Reset head page before running self test
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
When the ring buffer does its consistency test on itself, it removes the head page, runs the tests, and then adds it back to what the "head_page" pointer was. But because the head_page pointer may lack behind the real head page (held by the link list pointer). The reset may be incorrect.
Instead, if the head_page exists (it does not on first allocation) reset it back to the real head page before running the consistency tests. Then it will be put back to its original location after the tests are complete.
--- kernel/trace/ring_buffer.c | 4 ++++ 1 file changed, 4 insertions(+)
kernel/trace/ring_buffer.c +4 -0
@@ -945,6 +945,10 @@ static int rb_check_pages(struct ring_buffer_per_cpu *cpu_buffer)struct list_head *head = cpu_buffer->pages;struct buffer_page *bpage, *tmp; +/* Reset the head page if it exists */ +if (cpu_buffer->head_page) +rb_set_head_page (cpu_buffer); +rb_head_page_deactivate (cpu_buffer);if (RB_WARN_ON (cpu_buffer, head->next->prev != head))
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 6/15 tracing: Check return value of tracing_dentry_percpu()
In reply to Steven RostedtFrom: Namhyung Kim <[email protected]>
If tracing_dentry_percpu() failed, tracing_init_debugfs_percpu() will try to create each cpu directories on debugfs' root directory as d_percpu is NULL.
--- kernel/trace/trace.c | 3 +++ 1 file changed, 3 insertions(+)
kernel/trace/trace.c +3 -0
@@ -4474,6 +4474,9 @@ static void tracing_init_debugfs_percpu(long cpu)struct dentry *d_cpu;char cpu_dir[30 ];/* 30 characters should be more than enough */ +if (!d_percpu) +return ; +snprintf (cpu_dir,30 ,"cpu%ld" , cpu); d_cpu =debugfs_create_dir (cpu_dir, d_percpu);if (!d_cpu) {
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 7/15 tracing: change CPU ring buffer state from tracing_cpumask
In reply to Steven RostedtFrom: Vaibhav Nagarnaik <[email protected]>
According to Documentation/trace/ftrace.txt:
tracing_cpumask:
This is a mask that lets the user only trace on specified CPUS. The format is a hex string representing the CPUS.
The tracing_cpumask currently doesn't affect the tracing state of per-CPU ring buffers.
This patch enables/disables CPU recording as its corresponding bit in tracing_cpumask is set/unset.
--- kernel/trace/trace.c | 2 ++ 1 file changed, 2 insertions(+)
kernel/trace/trace.c +2 -0
@@ -2669,10 +2669,12 @@ tracing_cpumask_write(struct file *filp, const char __user *ubuf,if (cpumask_test_cpu (cpu, tracing_cpumask) && !cpumask_test_cpu (cpu, tracing_cpumask_new)) {atomic_inc (&global_trace.data[cpu]->disabled); +ring_buffer_record_disable_cpu (global_trace.buffer, cpu); }if (!cpumask_test_cpu (cpu, tracing_cpumask) &&cpumask_test_cpu (cpu, tracing_cpumask_new)) {atomic_dec (&global_trace.data[cpu]->disabled); +ring_buffer_record_enable_cpu (global_trace.buffer, cpu); } }arch_spin_unlock (&ftrace_max_lock);
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 8/15 ftrace: Sort all function addresses, not just per page
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Instead of just sorting the ip's of the functions per ftrace page, sort the entire list before adding them to the ftrace pages.
This will allow the bsearch algorithm to be sped up as it can also sort by pages, not just records within a page.
--- include/asm-generic/vmlinux.lds.h | 2 +- kernel/trace/ftrace.c | 34 ++++++++++++++++++++++------------ 2 files changed, 23 insertions(+), 13 deletions(-)
include/asm-generic/vmlinux.lds.h +1 -1
@@ -486,8 +486,8 @@ CPU_DISCARD(init.data) \ MEM_DISCARD(init.data) \ KERNEL_CTORS() \ - *(.init.rodata) \MCOUNT_REC () \ + *(.init.rodata) \ FTRACE_EVENTS() \ TRACE_SYSCALLS() \DEV_DISCARD (init.rodata) \
kernel/trace/ftrace.c +22 -12
@@ -3666,15 +3666,27 @@ static __init int ftrace_init_dyn_debugfs(struct dentry *d_tracer)return 0 ; } -static void ftrace_swap_recs (void *a,void *b,int size) +static int ftrace_cmp_ips (const void *a,const void *b) { -struct dyn_ftrace *reca = a; -struct dyn_ftrace *recb = b; -struct dyn_ftrace t; +const unsigned long *ipa = a; +const unsigned long *ipb = b; - t = *reca; - *reca = *recb; - *recb = t; +if (*ipa > *ipb) +return 1 ; +if (*ipa < *ipb) +return -1 ; +return 0 ; +} + +static void ftrace_swap_ips (void *a,void *b,int size) +{ +unsigned long *ipa = a; +unsigned long *ipb = b; +unsigned long t; + + t = *ipa; + *ipa = *ipb; + *ipb = t; } static int ftrace_process_locs(struct module *mod,
@@ -3693,6 +3705,9 @@ static int ftrace_process_locs(struct module *mod,if (!count)return 0 ; +sort (start, count,sizeof (*start), + ftrace_cmp_ips, ftrace_swap_ips); + pg =ftrace_allocate_pages (count);if (!pg)return -ENOMEM ;
@@ -3740,11 +3755,6 @@ static int ftrace_process_locs(struct module *mod,/* These new locations need to be initialized */ ftrace_new_pgs = pg; -/* Make each individual set of pages sorted by ips */ -for (; pg; pg = pg->next) -sort (pg->records, pg->index,sizeof (struct dyn_ftrace ), - ftrace_cmp_recs, ftrace_swap_recs); -/* * We only need to disable interrupts on start up * because we are modifying code that an interrupt
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 9/15 ftrace: Remove extra helper functions
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
The ftrace_record_ip() and ftrace_alloc_dyn_node() were from the time of the ftrace daemon. Although they were still used, they still make things a bit more complex than necessary.
Move the code into the one function that uses it, and remove the helper functions.
--- kernel/trace/ftrace.c | 61 +++++++++++++++++++------------------------------ 1 file changed, 24 insertions(+), 37 deletions(-)
kernel/trace/ftrace.c +24 -37
@@ -1520,35 +1520,6 @@ static void ftrace_hash_rec_enable(struct ftrace_ops *ops,__ftrace_hash_rec_update (ops, filter_hash,1 ); } -static struct dyn_ftrace *ftrace_alloc_dyn_node (unsigned long ip) -{ -if (ftrace_pages->index == ftrace_pages->size) { -/* We should have allocated enough */ -if (WARN_ON (!ftrace_pages->next)) -return NULL ; - ftrace_pages = ftrace_pages->next; - } - -return &ftrace_pages->records[ftrace_pages->index++]; -} - -static struct dyn_ftrace * -ftrace_record_ip (unsigned long ip) -{ -struct dyn_ftrace *rec; - -if (ftrace_disabled) -return NULL ; - - rec =ftrace_alloc_dyn_node (ip); -if (!rec) -return NULL ; - - rec->ip = ip; - -return rec; -} -static void print_ip_ins (const char *fmt,unsigned char *p) {int i;
@@ -3693,7 +3664,9 @@ static int ftrace_process_locs(struct module *mod,unsigned long *start, unsigned long *end) { + struct ftrace_page *start_pg;struct ftrace_page *pg; +struct dyn_ftrace *rec;unsigned long count;unsigned long *p;unsigned long addr;
@@ -3708,8 +3681,8 @@ static int ftrace_process_locs(struct module *mod,sort (start, count,sizeof (*start), ftrace_cmp_ips, ftrace_swap_ips); - pg =ftrace_allocate_pages (count); -if (!pg) + start_pg =ftrace_allocate_pages (count); +if (!start_pg)return -ENOMEM ;mutex_lock (&ftrace_lock);
@@ -3722,7 +3695,7 @@ static int ftrace_process_locs(struct module *mod,if (!mod) {WARN_ON (ftrace_pages || ftrace_pages_start);/* First initialization */ - ftrace_pages = ftrace_pages_start = pg; + ftrace_pages = ftrace_pages_start = start_pg; }else {if (!ftrace_pages) goto out;
@@ -3733,11 +3706,11 @@ static int ftrace_process_locs(struct module *mod, ftrace_pages = ftrace_pages->next; } - ftrace_pages->next = pg; - ftrace_pages = pg; + ftrace_pages->next = start_pg; } p = start; + pg = start_pg;while (p < end) { addr =ftrace_call_adjust (*p++);/*
@@ -3748,12 +3721,26 @@ static int ftrace_process_locs(struct module *mod,*/ if (!addr)continue ; -if (!ftrace_record_ip (addr)) -break ; + +if (pg->index == pg->size) { +/* We should have allocated enough */ +if (WARN_ON (!pg->next)) +break ; + pg = pg->next; + } + + rec = &pg->records[pg->index++]; + rec->ip = addr; } +/* We should have used all pages */ +WARN_ON (pg->next); + +/* Assign the last page to ftrace_pages */ + ftrace_pages = pg; +/* These new locations need to be initialized */ - ftrace_new_pgs = pg; + ftrace_new_pgs = start_pg;/* * We only need to disable interrupts on start up
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 10/15 ftrace: Speed up search by skipping pages by address
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
As all records in a page of the ftrace table are sorted, we can speed up the search algorithm by checking if the address to look for falls in between the first and last record ip on the page.
This speeds up both the ftrace_location() and ftrace_text_reserved() algorithms, as it can skip full pages when the search address is not in them.
--- kernel/trace/ftrace.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
kernel/trace/ftrace.c +16 -6
@@ -1411,6 +1411,8 @@ int ftrace_location(unsigned long ip) key.ip = ip;for (pg = ftrace_pages_start; pg; pg = pg->next) { +if (ip < pg->records[0 ].ip || ip > pg->records[pg->index -1 ].ip) +continue ; rec =bsearch (&key, pg->records, pg->index,sizeof (struct dyn_ftrace ), ftrace_cmp_recs);
@@ -1571,16 +1573,24 @@ void ftrace_bug(int failed, unsigned long ip)/* Return 1 if the address range is reserved for ftrace */ -int ftrace_text_reserved (void *start,void *end) +int ftrace_text_reserved (void *s,void *e) {struct dyn_ftrace *rec;struct ftrace_page *pg; +unsigned long start = (unsigned long )s; +unsigned long end = (unsigned long )e; +int i; -do_for_each_ftrace_rec (pg, rec) { -if (rec->ip <= (unsigned long )end && - rec->ip +MCOUNT_INSN_SIZE > (unsigned long )start) -return 1 ; - }while_for_each_ftrace_rec (); +for (pg = ftrace_pages_start; pg; pg = pg->next) { +if (end < pg->records[0 ].ip || + start >= (pg->records[pg->index -1 ].ip +MCOUNT_INSN_SIZE )) +continue ; +for (i =0 ; i < pg->index; i++) { + rec = &pg->records[i]; +if (rec->ip <= end && rec->ip +MCOUNT_INSN_SIZE > start) +return 1 ; + } + }return 0 ; }
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 11/15 ftrace: Consolidate ftrace_location() and ftrace_text_reserved()
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Both ftrace_location() and ftrace_text_reserved() do basically the same thing. They search to see if an address is in the ftace table (contains an address that may change from nop to call ftrace_caller). The difference is that ftrace_location() searches a single address, but ftrace_text_reserved() searches a range.
This also makes the ftrace_text_reserved() faster as it now uses a bsearch() instead of linearly searching all the addresses within a page.
--- kernel/trace/ftrace.c | 80 ++++++++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 40 deletions(-)
kernel/trace/ftrace.c +40 -40
@@ -1383,35 +1383,28 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip)static int ftrace_cmp_recs (const void *a,const void *b) { -const struct dyn_ftrace *reca = a; -const struct dyn_ftrace *recb = b; +const struct dyn_ftrace *key = a; +const struct dyn_ftrace *rec = b; -if (reca->ip > recb->ip) -return 1 ; -if (reca->ip < recb->ip) +if (key->flags < rec->ip)return -1 ; +if (key->ip >= rec->ip +MCOUNT_INSN_SIZE ) +return 1 ;return 0 ; } -/** -* ftrace_location - return true if the ip giving is a traced location -* @ip: the instruction pointer to check -* -* Returns 1 if @ip given is a pointer to a ftrace location. -* That is, the instruction that is either a NOP or call to -* the function tracer. It checks the ftrace internal tables to -* determine if the address belongs or not. -*/ -int ftrace_location (unsigned long ip) +static int ftrace_location_range (unsigned long start,unsigned long end) {struct ftrace_page *pg;struct dyn_ftrace *rec;struct dyn_ftrace key; - key.ip = ip; + key.ip = start; + key.flags = end;/* overload flags, as it is unsigned long */ for (pg = ftrace_pages_start; pg; pg = pg->next) { -if (ip < pg->records[0 ].ip || ip > pg->records[pg->index -1 ].ip) +if (end < pg->records[0 ].ip || + start >= (pg->records[pg->index -1 ].ip +MCOUNT_INSN_SIZE ))continue ; rec = bsearch(&key, pg->records, pg->index, sizeof(struct dyn_ftrace),
@@ -1423,6 +1416,36 @@ int ftrace_location(unsigned long ip)return 0 ; } +/** +* ftrace_location - return true if the ip giving is a traced location +* @ip: the instruction pointer to check +* +* Returns 1 if @ip given is a pointer to a ftrace location. +* That is, the instruction that is either a NOP or call to +* the function tracer. It checks the ftrace internal tables to +* determine if the address belongs or not. +*/ +int ftrace_location (unsigned long ip) +{ +return ftrace_location_range (ip, ip); +} + +/** +* ftrace_text_reserved - return true if range contains an ftrace location +* @start: start of range to search +* @end: end of range to search (inclusive). @end points to the last byte to check. +* +* Returns 1 if @start and @end contains a ftrace location. +* That is, the instruction that is either a NOP or call to +* the function tracer. It checks the ftrace internal tables to +* determine if the address belongs or not. +*/ +int ftrace_text_reserved (void *start,void *end) +{ +return ftrace_location_range ((unsigned long )start, + (unsigned long )end); +} +static void __ftrace_hash_rec_update (struct ftrace_ops *ops,int filter_hash,bool inc)
@@ -1571,29 +1594,6 @@ void ftrace_bug(int failed, unsigned long ip) } } - -/* Return 1 if the address range is reserved for ftrace */ -int ftrace_text_reserved (void *s,void *e) -{ -struct dyn_ftrace *rec; -struct ftrace_page *pg; -unsigned long start = (unsigned long )s; -unsigned long end = (unsigned long )e; -int i; - -for (pg = ftrace_pages_start; pg; pg = pg->next) { -if (end < pg->records[0 ].ip || - start >= (pg->records[pg->index -1 ].ip +MCOUNT_INSN_SIZE )) -continue ; -for (i =0 ; i < pg->index; i++) { - rec = &pg->records[i]; -if (rec->ip <= end && rec->ip +MCOUNT_INSN_SIZE > start) -return 1 ; - } - } -return 0 ; -} -static int ftrace_check_record (struct dyn_ftrace *rec,int enable,int update) {unsigned long flag =0UL ;
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 12/15 ftrace: Return record ip addr for ftrace_location()
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
ftrace_location() is passed an addr, and returns 1 if the addr is on a ftrace nop (or caller to ftrace_caller), and 0 otherwise.
To let kprobes know if it should move a breakpoint or not, it must return the actual addr that is the start of the ftrace nop. This way a kprobe placed on the location of a ftrace nop, can instead be placed on the instruction after the nop. Even if the probe addr is on the second or later byte of the nop, it can simply be moved forward.
--- include/linux/ftrace.h | 2 +- kernel/trace/ftrace.c | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-)
include/linux/ftrace.h +1 -1
@@ -295,7 +295,7 @@ struct dyn_ftrace *ftrace_rec_iter_record(struct ftrace_rec_iter *iter);int ftrace_update_record (struct dyn_ftrace *rec,int enable);int ftrace_test_record (struct dyn_ftrace *rec,int enable);void ftrace_run_stop_machine (int command); -int ftrace_location (unsigned long ip); +unsigned long ftrace_location (unsigned long ip);extern ftrace_func_t ftrace_trace_function;
kernel/trace/ftrace.c +10 -6
@@ -1393,7 +1393,7 @@ static int ftrace_cmp_recs(const void *a, const void *b)return 0 ; } -static int ftrace_location_range (unsigned long start,unsigned long end) +static unsigned long ftrace_location_range (unsigned long start,unsigned long end) {struct ftrace_page *pg;struct dyn_ftrace *rec;
@@ -1410,7 +1410,7 @@ static int ftrace_location_range(unsigned long start, unsigned long end)sizeof (struct dyn_ftrace ), ftrace_cmp_recs);if (rec) -return 1 ; +return rec->ip; }return 0 ;
@@ -1420,12 +1420,12 @@ static int ftrace_location_range(unsigned long start, unsigned long end)* ftrace_location - return true if the ip giving is a traced location * @ip: the instruction pointer to check * -* Returns 1 if @ip given is a pointer to a ftrace location. +* Returns rec->ip if @ip given is a pointer to a ftrace location. * That is, the instruction that is either a NOP or call to * the function tracer. It checks the ftrace internal tables to * determine if the address belongs or not. */ -int ftrace_location (unsigned long ip) +unsigned long ftrace_location (unsigned long ip) {return ftrace_location_range (ip, ip); }
@@ -1442,8 +1442,12 @@ int ftrace_location(unsigned long ip)*/ int ftrace_text_reserved (void *start,void *end) { -return ftrace_location_range ((unsigned long )start, - (unsigned long )end); +unsigned long ret; + + ret =ftrace_location_range ((unsigned long )start, + (unsigned long )end); + +return (int )!!ret; } static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
Signature · 1 line
1.7.10
Re: [PATCH 12/15] ftrace: Return record ip addr for ftrace_location()
Quoted 12 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
ftrace_location()is passed an addr, and returns 1 if the addr is on a ftrace nop (or caller toftrace_caller), and 0 otherwise.To let kprobes know if it should move a breakpoint or not, itH must return the actual addr that is the start of the ftrace nop. This way a kprobe placed on the location of a ftrace nop, can instead be placed on the instruction after the nop. Even if the probe addr is on the second or later byte of the nop, it can simply be moved forward.
Quoted 2 lines
Cc Masami Hiramatsu <[email protected]>Signed-off-by Steven Rostedt <[email protected]>
This looks good to me:)
Thanks!
P.S.
Steven, ftrace-based optimization is under development with both i386/x86-64 support :)
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
Steven Rostedt
maintainerPATCH 13/15 ftrace: Make ftrace_modify_all_code() global for archs to use
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Rename __ftrace_modify_code() to ftrace_modify_all_code() and make it global for all archs to use. This will remove the duplication of code, as archs that can modify code without stop_machine() can use it directly outside of the stop_machine() call.
--- include/linux/ftrace.h | 2 ++ kernel/trace/ftrace.c | 21 +++++++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-)
include/linux/ftrace.h +2 -0
@@ -319,6 +319,8 @@ extern void ftrace_caller(void);extern void ftrace_call (void );extern void mcount_call (void ); +void ftrace_modify_all_code (int command); +#ifndef FTRACE_ADDR #define FTRACE_ADDR ((unsigned long)ftrace_caller)#endif
kernel/trace/ftrace.c +13 -8
@@ -1811,22 +1811,27 @@ int __weak ftrace_arch_code_modify_post_process(void)return 0 ; } -static int __ftrace_modify_code (void *data) +void ftrace_modify_all_code (int command) { -int *command = data; - -if (*command &FTRACE_UPDATE_CALLS ) +if (command &FTRACE_UPDATE_CALLS )ftrace_replace_code (1 ); -else if (*command &FTRACE_DISABLE_CALLS ) +else if (command &FTRACE_DISABLE_CALLS )ftrace_replace_code (0 ); -if (*command &FTRACE_UPDATE_TRACE_FUNC ) +if (command &FTRACE_UPDATE_TRACE_FUNC )ftrace_update_ftrace_func (ftrace_trace_function); -if (*command &FTRACE_START_FUNC_RET ) +if (command &FTRACE_START_FUNC_RET )ftrace_enable_ftrace_graph_caller (); -else if (*command &FTRACE_STOP_FUNC_RET ) +else if (command &FTRACE_STOP_FUNC_RET )ftrace_disable_ftrace_graph_caller (); +} + +static int __ftrace_modify_code (void *data) +{ +int *command = data; + +ftrace_modify_all_code (*command);return 0 ; }
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 14/15 ftrace/x86: Have x86 ftrace use the ftrace_modify_all_code()
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
To remove duplicate code, have the ftrace arch_ftrace_update_code() use the generic ftrace_modify_all_code(). This requires that the default ftrace_replace_code() becomes a weak function so that an arch may override it.
--- arch/x86/kernel/ftrace.c | 15 ++------------- include/linux/ftrace.h | 1 + kernel/trace/ftrace.c | 4 ++-- 3 files changed, 5 insertions(+), 15 deletions(-)
arch/x86/kernel/ftrace.c +2 -13
@@ -435,7 +435,7 @@ static void run_sync(void)local_irq_disable (); } -static void ftrace_replace_code (int enable) +void ftrace_replace_code (int enable) {struct ftrace_rec_iter *iter;struct dyn_ftrace *rec;
@@ -493,18 +493,7 @@ void arch_ftrace_update_code(int command) { modifying_ftrace_code++; -if (command &FTRACE_UPDATE_CALLS ) -ftrace_replace_code (1 ); -else if (command &FTRACE_DISABLE_CALLS ) -ftrace_replace_code (0 ); - -if (command &FTRACE_UPDATE_TRACE_FUNC ) -ftrace_update_ftrace_func (ftrace_trace_function); - -if (command &FTRACE_START_FUNC_RET ) -ftrace_enable_ftrace_graph_caller (); -else if (command &FTRACE_STOP_FUNC_RET ) -ftrace_disable_ftrace_graph_caller (); +ftrace_modify_all_code (command); modifying_ftrace_code--; }
include/linux/ftrace.h +1 -0
@@ -314,6 +314,7 @@ ftrace_set_early_filter(struct ftrace_ops *ops, char *buf, int enable);/* defined in arch */ extern int ftrace_ip_converted (unsigned long ip);extern int ftrace_dyn_arch_init (void *data); +extern void ftrace_replace_code (int enable);extern int ftrace_update_ftrace_func (ftrace_func_t func);extern void ftrace_caller (void );extern void ftrace_call (void );
kernel/trace/ftrace.c +2 -2
@@ -1683,7 +1683,7 @@ __ftrace_replace_code(struct dyn_ftrace *rec, int enable)return -1 ;/* unknow ftrace bug */ } -static void ftrace_replace_code (int update) +void __weak ftrace_replace_code (int enable) {struct dyn_ftrace *rec;struct ftrace_page *pg;
@@ -1693,7 +1693,7 @@ static void ftrace_replace_code(int update)return ;do_for_each_ftrace_rec (pg, rec) { - failed =__ftrace_replace_code (rec, update); + failed =__ftrace_replace_code (rec, enable);if (failed) {ftrace_bug (failed, rec->ip);/* Stop processing */
Signature · 1 line
1.7.10
Steven Rostedt
maintainerPATCH 15/15 ftrace: Remove selecting FRAME_POINTER with FUNCTION_TRACER
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
The function tracer will enable the -pg option with gcc, which requires that frame pointers. When FRAME_POINTER is defined in the kernel config it adds the gcc option -fno-omit-frame-pointer which causes some problems on some architectures. For those architectures, the FRAME_POINTER select was not set.
When FUNCTION_TRACER was selected on these architectures that can not have -fno-omit-frame-pointer, the -pg option is still set. But when FRAME_POINTER is not selected, the kernel config would add the gcc option -fomit-frame-pointer. Adding this option is incompatible with -pg even on archs that do not need frame pointers with -pg.
The answer to this was to just not add either -fno-omit-frame-pointer or -fomit-frame-pointer on these archs that want function tracing but do not set FRAME_POINTER.
As it turns out, for archs that require frame pointers for function tracing, the same can be used. If gcc requires frame pointers with -pg, it will simply add it. The best thing to do is not select FRAME_POINTER when function tracing is selected, and let gcc add it if needed.
Only add the -fno-omit-frame-pointer when something else selects FRAME_POINTER, but do not add -fomit-frame-pointer if function tracing is selected.
--- kernel/trace/Kconfig | 1 - 1 file changed, 1 deletion(-)
kernel/trace/Kconfig +0 -1
@@ -141,7 +141,6 @@ if FTRACE config FUNCTION_TRACER bool "Kernel Function Tracer" depends on HAVE_FUNCTION_TRACER - select FRAME_POINTER if !ARM_UNWIND && !PPC && !S390 && !MICROBLAZE select KALLSYMS select GENERIC_TRACER select CONTEXT_SWITCH_TRACER
Signature · 1 line
1.7.10
Steven Rostedt
maintainerQuoted 3 lines · Steven Rostedt
Ingo,
This should be my last set of changes for the next merge window.
Not quite :-)
You can hold off on pulling this. I want to add Masami's Acked-by tag, and Vaibhav gave me two more patches. I'll run it through my own tests and then do another pull request.
Thanks!
-- Steve
Quoted 39 lines
Please pull the latest tip/perf/core tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
tip/perf/coreHead SHA1:
b732d439cb43336cd6d7e804ecb2c81193ef63b0Namhyung Kim (1):
tracing: Check return value oftracing_dentry_percpu()Steven Rostedt (11):
tracing: Clean uptracing_mark_write()
ring-buffer: Add integrity check at end of iter read
ring-buffer: Reset head page before running self test
ftrace: Sort all function addresses, not just per page
ftrace: Remove extra helper functions
ftrace: Speed up search by skipping pages by address
ftrace: Consolidateftrace_location()andftrace_text_reserved()
ftrace: Return record ip addr forftrace_location()
ftrace: Makeftrace_modify_all_code()global for archs to use ftrace/x86: Have x86 ftrace use theftrace_modify_all_code()
ftrace: Remove selectingFRAME_POINTERwithFUNCTION_TRACERVaibhav Nagarnaik (3):
ring-buffer: Make removal of ring buffer pages atomic
ring-buffer: Make addition of pages in ring buffer atomic
tracing: change CPU ring buffer state fromtracing_cpumask----
arch/x86/kernel/ftrace.c | 15 +- include/asm-generic/vmlinux.lds.h | 2 +- include/linux/ftrace.h | 5 +- kernel/trace/Kconfig | 1 - kernel/trace/ftrace.c | 198 ++++++++++--------- kernel/trace/ring_buffer.c | 394 +++++++++++++++++++++++++++++-------- kernel/trace/trace.c | 49 ++--- 7 files changed, 446 insertions(+), 218 deletions(-)
Quoted 10 lines · Steven Rostedt
Quoted 3 lines · Steven Rostedt
Ingo,
This should be my last set of changes for the next merge window.
Not quite :-)
You can hold off on pulling this. I want to add Masami's Acked-by tag, and Vaibhav gave me two more patches. I'll run it through my own tests and then do another pull request.
I already pulled them and they tested out fine. Please queue up the two patches on top.
Thanks,
Ingo
Steven Rostedt
maintainerQuoted 7 lines · Ingo Molnar
Quoted 5 lines · Steven Rostedt
Quoted 3 lines · Steven Rostedt
Ingo,
This should be my last set of changes for the next merge window.
Quoted 8 lines
Quoted 5 lines
Not quite :-)
You can hold off on pulling this. I want to add Masami's Acked-by tag, and Vaibhav gave me two more patches. I'll run it through my own tests and then do another pull request.
I already pulled them and they tested out fine. Please queue up the two patches on top.
OK, will do. I actually found an old patch that should have gone in a long time ago too. It's a minor clean up, so I'll add that one too.
-- Steve