From fd13f6c85144bb2026c534a35be1d7cb7628a64a Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Sun, 19 Oct 2008 21:00:09 +0200 Subject: oprofile: comment cleanup This fixes the coding style of some comments. Signed-off-by: Robert Richter --- drivers/oprofile/cpu_buffer.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c index 01d38e78cde1..3958107723fb 100644 --- a/drivers/oprofile/cpu_buffer.c +++ b/drivers/oprofile/cpu_buffer.c @@ -127,9 +127,10 @@ void end_cpu_work(void) /* Resets the cpu buffer to a sane state. */ void cpu_buffer_reset(struct oprofile_cpu_buffer *cpu_buf) { - /* reset these to invalid values; the next sample - * collected will populate the buffer with proper - * values to initialize the buffer + /* + * reset these to invalid values; the next sample collected + * will populate the buffer with proper values to initialize + * the buffer */ cpu_buf->last_is_kernel = -1; cpu_buf->last_task = NULL; @@ -151,8 +152,10 @@ static void increment_head(struct oprofile_cpu_buffer *b) { unsigned long new_head = b->head_pos + 1; - /* Ensure anything written to the slot before we - * increment is visible */ + /* + * Ensure anything written to the slot before we increment is + * visible + */ wmb(); if (new_head < b->buffer_size) @@ -253,8 +256,10 @@ void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs, if (!oprofile_begin_trace(cpu_buf)) return; - /* if log_sample() fail we can't backtrace since we lost the source - * of this event */ + /* + * if log_sample() fail we can't backtrace since we lost the + * source of this event + */ if (log_sample(cpu_buf, pc, is_kernel, event)) oprofile_ops.backtrace(regs, backtrace_depth); oprofile_end_trace(cpu_buf); @@ -338,8 +343,10 @@ void oprofile_add_trace(unsigned long pc) return; } - /* broken frame can give an eip with the same value as an escape code, - * abort the trace if we get it */ + /* + * broken frame can give an eip with the same value as an + * escape code, abort the trace if we get it + */ if (pc == ESCAPE_CODE) { cpu_buf->tracing = 0; cpu_buf->backtrace_aborted++; -- cgit v1.2.3 From cdc1834d1aa2e5b574a25e66f82625b44cdd0d8f Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Fri, 26 Sep 2008 22:18:44 -0400 Subject: oprofile: whitspace changes only Signed-off-by: Robert Richter --- drivers/oprofile/cpu_buffer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c index 3958107723fb..2c4d54187b90 100644 --- a/drivers/oprofile/cpu_buffer.c +++ b/drivers/oprofile/cpu_buffer.c @@ -277,8 +277,8 @@ void oprofile_add_sample(struct pt_regs * const regs, unsigned long event) #define MAX_IBS_SAMPLE_SIZE 14 -void oprofile_add_ibs_sample(struct pt_regs *const regs, - unsigned int *const ibs_sample, int ibs_code) +void oprofile_add_ibs_sample(struct pt_regs * const regs, + unsigned int * const ibs_sample, int ibs_code) { int is_kernel = !user_mode(regs); struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(cpu_buffer); -- cgit v1.2.3 From 8dbc50c322619eb821907e8dba75252f5378c712 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Wed, 26 Nov 2008 15:00:52 +0100 Subject: oprofile: fix typo Signed-off-by: Robert Richter --- drivers/oprofile/buffer_sync.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c index b55cd23ffdef..774b081b9b7f 100644 --- a/drivers/oprofile/buffer_sync.c +++ b/drivers/oprofile/buffer_sync.c @@ -272,7 +272,7 @@ static void increment_tail(struct oprofile_cpu_buffer *b) { unsigned long new_tail = b->tail_pos + 1; - rmb(); /* be sure fifo pointers are synchromized */ + rmb(); /* be sure fifo pointers are synchronized */ if (new_tail < b->buffer_size) b->tail_pos = new_tail; -- cgit v1.2.3 From fd7826d56bde11ab142d2431093773ad2b3f0a59 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Fri, 26 Sep 2008 17:50:31 -0400 Subject: oprofile: implement switch/case in buffer_sync.c Signed-off-by: Robert Richter --- drivers/oprofile/buffer_sync.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c index 774b081b9b7f..7d61ae8ee8cf 100644 --- a/drivers/oprofile/buffer_sync.c +++ b/drivers/oprofile/buffer_sync.c @@ -561,6 +561,7 @@ void sync_buffer(int cpu) { struct oprofile_cpu_buffer *cpu_buf = &per_cpu(cpu_buffer, cpu); struct mm_struct *mm = NULL; + struct mm_struct *oldmm; struct task_struct *new; unsigned long cookie = 0; int in_kernel = 1; @@ -586,34 +587,39 @@ void sync_buffer(int cpu) struct op_sample *s = &cpu_buf->buffer[cpu_buf->tail_pos]; if (is_code(s->eip)) { - if (s->event <= CPU_IS_KERNEL) { + switch (s->event) { + case 0: + case CPU_IS_KERNEL: /* kernel/userspace switch */ in_kernel = s->event; if (state == sb_buffer_start) state = sb_sample_start; add_kernel_ctx_switch(s->event); - } else if (s->event == CPU_TRACE_BEGIN) { + break; + case CPU_TRACE_BEGIN: state = sb_bt_start; add_trace_begin(); + break; #ifdef CONFIG_OPROFILE_IBS - } else if (s->event == IBS_FETCH_BEGIN) { + case IBS_FETCH_BEGIN: state = sb_bt_start; add_ibs_begin(cpu_buf, IBS_FETCH_CODE, mm); - } else if (s->event == IBS_OP_BEGIN) { + break; + case IBS_OP_BEGIN: state = sb_bt_start; add_ibs_begin(cpu_buf, IBS_OP_CODE, mm); + break; #endif - } else { - struct mm_struct *oldmm = mm; - + default: /* userspace context switch */ + oldmm = mm; new = (struct task_struct *)s->event; - release_mm(oldmm); mm = take_tasks_mm(new); if (mm != oldmm) cookie = get_exec_dcookie(mm); add_user_ctx_switch(new, cookie); + break; } } else if (state >= sb_bt_start && !add_sample(mm, s, in_kernel)) { -- cgit v1.2.3 From 37ca5eb341711d7aeb9f296873b9d46eb6af33ec Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Tue, 9 Dec 2008 16:56:01 +0100 Subject: oprofile: set values to default when creating oprofilefs This patch restores default values for: /dev/oprofile/cpu_buffer_size /dev/oprofile/buffer_watershed /dev/oprofile/buffer_size when creating the oprofilefs: # opcontrol --deinit # opcontrol --init # cat /dev/oprofile/cpu_buffer_size 8192 # echo 5123 > /dev/oprofile/cpu_buffer_size # cat /dev/oprofile/cpu_buffer_size 5123 # opcontrol --deinit # opcontrol --init # cat /dev/oprofile/cpu_buffer_size 8192 # opcontrol --deinit This sets the values in a defined state. Before, there was no way to restore the defaults without rebooting the system or reloading the module. Signed-off-by: Robert Richter --- drivers/oprofile/oprofile_files.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/oprofile_files.c b/drivers/oprofile/oprofile_files.c index cc106d503ace..d8201998b0b7 100644 --- a/drivers/oprofile/oprofile_files.c +++ b/drivers/oprofile/oprofile_files.c @@ -14,9 +14,13 @@ #include "oprofile_stats.h" #include "oprof.h" -unsigned long fs_buffer_size = 131072; -unsigned long fs_cpu_buffer_size = 8192; -unsigned long fs_buffer_watershed = 32768; /* FIXME: tune */ +#define FS_BUFFER_SIZE_DEFAULT 131072 +#define FS_CPU_BUFFER_SIZE_DEFAULT 8192 +#define FS_BUFFER_WATERSHED_DEFAULT 32768 /* FIXME: tune */ + +unsigned long fs_buffer_size; +unsigned long fs_cpu_buffer_size; +unsigned long fs_buffer_watershed; static ssize_t depth_read(struct file *file, char __user *buf, size_t count, loff_t *offset) { @@ -120,6 +124,11 @@ static const struct file_operations dump_fops = { void oprofile_create_files(struct super_block *sb, struct dentry *root) { + /* reinitialize default values */ + fs_buffer_size = FS_BUFFER_SIZE_DEFAULT; + fs_cpu_buffer_size = FS_CPU_BUFFER_SIZE_DEFAULT; + fs_buffer_watershed = FS_BUFFER_WATERSHED_DEFAULT; + oprofilefs_create_file(sb, root, "enable", &enable_fops); oprofilefs_create_file_perm(sb, root, "dump", &dump_fops, 0666); oprofilefs_create_file(sb, root, "buffer", &event_buffer_fops); -- cgit v1.2.3 From 7d468abee0f1a7e918b5e2f23120436a54ba9f33 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Thu, 27 Nov 2008 10:57:09 +0100 Subject: oprofile: adding cpu buffer r/w access functions This is in preparation for changes in the cpu buffer implementation. Signed-off-by: Robert Richter --- drivers/oprofile/buffer_sync.c | 20 +++++++++----------- drivers/oprofile/cpu_buffer.c | 2 +- drivers/oprofile/cpu_buffer.h | 12 ++++++++++++ 3 files changed, 22 insertions(+), 12 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c index 7d61ae8ee8cf..44f676c8a51d 100644 --- a/drivers/oprofile/buffer_sync.c +++ b/drivers/oprofile/buffer_sync.c @@ -331,10 +331,8 @@ static void add_trace_begin(void) #define IBS_FETCH_CODE_SIZE 2 #define IBS_OP_CODE_SIZE 5 -#define IBS_EIP(offset) \ - (((struct op_sample *)&cpu_buf->buffer[(offset)])->eip) -#define IBS_EVENT(offset) \ - (((struct op_sample *)&cpu_buf->buffer[(offset)])->event) +#define IBS_EIP(cpu_buf) ((cpu_buffer_read_entry(cpu_buf))->eip) +#define IBS_EVENT(cpu_buf) ((cpu_buffer_read_entry(cpu_buf))->event) /* * Add IBS fetch and op entries to event buffer @@ -349,10 +347,10 @@ static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code, increment_tail(cpu_buf); /* move to RIP entry */ - rip = IBS_EIP(cpu_buf->tail_pos); + rip = IBS_EIP(cpu_buf); #ifdef __LP64__ - rip += IBS_EVENT(cpu_buf->tail_pos) << 32; + rip += IBS_EVENT(cpu_buf) << 32; #endif if (mm) { @@ -376,8 +374,8 @@ static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code, add_event_entry(offset); /* Offset from Dcookie */ /* we send the Dcookie offset, but send the raw Linear Add also*/ - add_event_entry(IBS_EIP(cpu_buf->tail_pos)); - add_event_entry(IBS_EVENT(cpu_buf->tail_pos)); + add_event_entry(IBS_EIP(cpu_buf)); + add_event_entry(IBS_EVENT(cpu_buf)); if (code == IBS_FETCH_CODE) count = IBS_FETCH_CODE_SIZE; /*IBS FETCH is 2 int64s*/ @@ -386,8 +384,8 @@ static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code, for (i = 0; i < count; i++) { increment_tail(cpu_buf); - add_event_entry(IBS_EIP(cpu_buf->tail_pos)); - add_event_entry(IBS_EVENT(cpu_buf->tail_pos)); + add_event_entry(IBS_EIP(cpu_buf)); + add_event_entry(IBS_EVENT(cpu_buf)); } } @@ -584,7 +582,7 @@ void sync_buffer(int cpu) #else while (get_slots(cpu_buf)) { #endif - struct op_sample *s = &cpu_buf->buffer[cpu_buf->tail_pos]; + struct op_sample *s = cpu_buffer_read_entry(cpu_buf); if (is_code(s->eip)) { switch (s->event) { diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c index 2c4d54187b90..7e5e650e409f 100644 --- a/drivers/oprofile/cpu_buffer.c +++ b/drivers/oprofile/cpu_buffer.c @@ -168,7 +168,7 @@ static inline void add_sample(struct oprofile_cpu_buffer *cpu_buf, unsigned long pc, unsigned long event) { - struct op_sample *entry = &cpu_buf->buffer[cpu_buf->head_pos]; + struct op_sample *entry = cpu_buffer_write_entry(cpu_buf); entry->eip = pc; entry->event = event; increment_head(cpu_buf); diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h index d3cc26264db5..08706991fdd2 100644 --- a/drivers/oprofile/cpu_buffer.h +++ b/drivers/oprofile/cpu_buffer.h @@ -52,6 +52,18 @@ DECLARE_PER_CPU(struct oprofile_cpu_buffer, cpu_buffer); void cpu_buffer_reset(struct oprofile_cpu_buffer *cpu_buf); +static inline +struct op_sample *cpu_buffer_write_entry(struct oprofile_cpu_buffer *cpu_buf) +{ + return &cpu_buf->buffer[cpu_buf->head_pos]; +} + +static inline +struct op_sample *cpu_buffer_read_entry(struct oprofile_cpu_buffer *cpu_buf) +{ + return &cpu_buf->buffer[cpu_buf->tail_pos]; +} + /* transient events for the CPU buffer -> event buffer */ #define CPU_IS_KERNEL 1 #define CPU_TRACE_BEGIN 2 -- cgit v1.2.3 From 229234ae4a5ed9376b2e0524da04b0e5edadbf76 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Thu, 27 Nov 2008 18:36:08 +0100 Subject: oprofile: adding cpu_buffer_write_commit() This is in preparation for changes in the cpu buffer implementation. Signed-off-by: Robert Richter --- drivers/oprofile/cpu_buffer.c | 18 +----------------- drivers/oprofile/cpu_buffer.h | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 17 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c index 7e5e650e409f..d6f5de686363 100644 --- a/drivers/oprofile/cpu_buffer.c +++ b/drivers/oprofile/cpu_buffer.c @@ -148,22 +148,6 @@ static unsigned long nr_available_slots(struct oprofile_cpu_buffer const *b) return tail + (b->buffer_size - head) - 1; } -static void increment_head(struct oprofile_cpu_buffer *b) -{ - unsigned long new_head = b->head_pos + 1; - - /* - * Ensure anything written to the slot before we increment is - * visible - */ - wmb(); - - if (new_head < b->buffer_size) - b->head_pos = new_head; - else - b->head_pos = 0; -} - static inline void add_sample(struct oprofile_cpu_buffer *cpu_buf, unsigned long pc, unsigned long event) @@ -171,7 +155,7 @@ add_sample(struct oprofile_cpu_buffer *cpu_buf, struct op_sample *entry = cpu_buffer_write_entry(cpu_buf); entry->eip = pc; entry->event = event; - increment_head(cpu_buf); + cpu_buffer_write_commit(cpu_buf); } static inline void diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h index 08706991fdd2..e6089768ae66 100644 --- a/drivers/oprofile/cpu_buffer.h +++ b/drivers/oprofile/cpu_buffer.h @@ -58,6 +58,23 @@ struct op_sample *cpu_buffer_write_entry(struct oprofile_cpu_buffer *cpu_buf) return &cpu_buf->buffer[cpu_buf->head_pos]; } +static inline +void cpu_buffer_write_commit(struct oprofile_cpu_buffer *b) +{ + unsigned long new_head = b->head_pos + 1; + + /* + * Ensure anything written to the slot before we increment is + * visible + */ + wmb(); + + if (new_head < b->buffer_size) + b->head_pos = new_head; + else + b->head_pos = 0; +} + static inline struct op_sample *cpu_buffer_read_entry(struct oprofile_cpu_buffer *cpu_buf) { -- cgit v1.2.3 From bf589e32960181fa8cbca7bfdd92265e49dc2dfa Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Thu, 27 Nov 2008 22:33:37 +0100 Subject: oprofile: adding cpu_buffer_entries() This is in preparation for changes in the cpu buffer implementation. Signed-off-by: Robert Richter --- drivers/oprofile/buffer_sync.c | 31 ++----------------------------- drivers/oprofile/cpu_buffer.h | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 29 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c index 44f676c8a51d..aed286c3f16e 100644 --- a/drivers/oprofile/buffer_sync.c +++ b/drivers/oprofile/buffer_sync.c @@ -464,33 +464,6 @@ static inline int is_code(unsigned long val) } -/* "acquire" as many cpu buffer slots as we can */ -static unsigned long get_slots(struct oprofile_cpu_buffer *b) -{ - unsigned long head = b->head_pos; - unsigned long tail = b->tail_pos; - - /* - * Subtle. This resets the persistent last_task - * and in_kernel values used for switching notes. - * BUT, there is a small window between reading - * head_pos, and this call, that means samples - * can appear at the new head position, but not - * be prefixed with the notes for switching - * kernel mode or a task switch. This small hole - * can lead to mis-attribution or samples where - * we don't know if it's in the kernel or not, - * at the start of an event buffer. - */ - cpu_buffer_reset(b); - - if (head >= tail) - return head - tail; - - return head + (b->buffer_size - tail); -} - - /* Move tasks along towards death. Any tasks on dead_tasks * will definitely have no remaining references in any * CPU buffers at this point, because we use two lists, @@ -576,11 +549,11 @@ void sync_buffer(int cpu) /* Remember, only we can modify tail_pos */ #ifndef CONFIG_OPROFILE_IBS - available = get_slots(cpu_buf); + available = cpu_buffer_entries(cpu_buf); for (i = 0; i < available; ++i) { #else - while (get_slots(cpu_buf)) { + while (cpu_buffer_entries(cpu_buf)) { #endif struct op_sample *s = cpu_buffer_read_entry(cpu_buf); diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h index e6089768ae66..6055b5678394 100644 --- a/drivers/oprofile/cpu_buffer.h +++ b/drivers/oprofile/cpu_buffer.h @@ -81,6 +81,33 @@ struct op_sample *cpu_buffer_read_entry(struct oprofile_cpu_buffer *cpu_buf) return &cpu_buf->buffer[cpu_buf->tail_pos]; } +/* "acquire" as many cpu buffer slots as we can */ +static inline +unsigned long cpu_buffer_entries(struct oprofile_cpu_buffer *b) +{ + unsigned long head = b->head_pos; + unsigned long tail = b->tail_pos; + + /* + * Subtle. This resets the persistent last_task + * and in_kernel values used for switching notes. + * BUT, there is a small window between reading + * head_pos, and this call, that means samples + * can appear at the new head position, but not + * be prefixed with the notes for switching + * kernel mode or a task switch. This small hole + * can lead to mis-attribution or samples where + * we don't know if it's in the kernel or not, + * at the start of an event buffer. + */ + cpu_buffer_reset(b); + + if (head >= tail) + return head - tail; + + return head + (b->buffer_size - tail); +} + /* transient events for the CPU buffer -> event buffer */ #define CPU_IS_KERNEL 1 #define CPU_TRACE_BEGIN 2 -- cgit v1.2.3 From fbc9bf9f0ed4f0fbc47dcb5b1c26c28c93b60e33 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Thu, 4 Dec 2008 16:27:00 +0100 Subject: oprofile: moving cpu_buffer_reset() to cpu_buffer.h This is in preparation for changes in the cpu buffer implementation. Signed-off-by: Robert Richter --- drivers/oprofile/buffer_sync.c | 1 + drivers/oprofile/cpu_buffer.c | 12 ------------ drivers/oprofile/cpu_buffer.h | 28 +++++++++++++--------------- 3 files changed, 14 insertions(+), 27 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c index aed286c3f16e..944a5832d9e4 100644 --- a/drivers/oprofile/buffer_sync.c +++ b/drivers/oprofile/buffer_sync.c @@ -548,6 +548,7 @@ void sync_buffer(int cpu) /* Remember, only we can modify tail_pos */ + cpu_buffer_reset(cpu); #ifndef CONFIG_OPROFILE_IBS available = cpu_buffer_entries(cpu_buf); diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c index d6f5de686363..5cf7efe38e67 100644 --- a/drivers/oprofile/cpu_buffer.c +++ b/drivers/oprofile/cpu_buffer.c @@ -124,18 +124,6 @@ void end_cpu_work(void) flush_scheduled_work(); } -/* Resets the cpu buffer to a sane state. */ -void cpu_buffer_reset(struct oprofile_cpu_buffer *cpu_buf) -{ - /* - * reset these to invalid values; the next sample collected - * will populate the buffer with proper values to initialize - * the buffer - */ - cpu_buf->last_is_kernel = -1; - cpu_buf->last_task = NULL; -} - /* compute number of available slots in cpu_buffer queue */ static unsigned long nr_available_slots(struct oprofile_cpu_buffer const *b) { diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h index 6055b5678394..895763f065e9 100644 --- a/drivers/oprofile/cpu_buffer.h +++ b/drivers/oprofile/cpu_buffer.h @@ -50,7 +50,19 @@ struct oprofile_cpu_buffer { DECLARE_PER_CPU(struct oprofile_cpu_buffer, cpu_buffer); -void cpu_buffer_reset(struct oprofile_cpu_buffer *cpu_buf); +/* + * Resets the cpu buffer to a sane state. + * + * reset these to invalid values; the next sample collected will + * populate the buffer with proper values to initialize the buffer + */ +static inline void cpu_buffer_reset(int cpu) +{ + struct oprofile_cpu_buffer *cpu_buf = &per_cpu(cpu_buffer, cpu); + + cpu_buf->last_is_kernel = -1; + cpu_buf->last_task = NULL; +} static inline struct op_sample *cpu_buffer_write_entry(struct oprofile_cpu_buffer *cpu_buf) @@ -88,20 +100,6 @@ unsigned long cpu_buffer_entries(struct oprofile_cpu_buffer *b) unsigned long head = b->head_pos; unsigned long tail = b->tail_pos; - /* - * Subtle. This resets the persistent last_task - * and in_kernel values used for switching notes. - * BUT, there is a small window between reading - * head_pos, and this call, that means samples - * can appear at the new head position, but not - * be prefixed with the notes for switching - * kernel mode or a task switch. This small hole - * can lead to mis-attribution or samples where - * we don't know if it's in the kernel or not, - * at the start of an event buffer. - */ - cpu_buffer_reset(b); - if (head >= tail) return head - tail; -- cgit v1.2.3 From 6dad828b76c7224a22ddc9ce7aa495d994f03b31 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Tue, 9 Dec 2008 01:21:32 +0100 Subject: oprofile: port to the new ring_buffer This patch replaces the current oprofile cpu buffer implementation with the ring buffer provided by the tracing framework. The motivation here is to leave the pain of implementing ring buffers to others. Oh, no, there are more advantages. Main reason is the support of different sample sizes that could be stored in the buffer. Use cases for this are IBS and Cell spu profiling. Using the new ring buffer ensures valid and complete samples and allows copying the cpu buffer stateless without knowing its content. Second it will use generic kernel API and also reduce code size. And hopefully, there are less bugs. Since the new tracing ring buffer implementation uses spin locks to protect the buffer during read/write access, it is difficult to use the buffer in an NMI handler. In this case, writing to the buffer by the NMI handler (x86) could occur also during critical sections when reading the buffer. To avoid this, there are 2 buffers for independent read and write access. Read access is in process context only, write access only in the NMI handler. If the read buffer runs empty, both buffers are swapped atomically. There is potentially a small window during swapping where the buffers are disabled and samples could be lost. Using 2 buffers is a little bit overhead, but the solution is clear and does not require changes in the ring buffer implementation. It can be changed to a single buffer solution when the ring buffer access is implemented as non-locking atomic code. The new buffer requires more size to store the same amount of samples because each sample includes an u32 header. Also, there is more code to execute for buffer access. Nonetheless, the buffer implementation is proven in the ftrace environment and worth to use also in oprofile. Patches that changes the internal IBS buffer usage will follow. Cc: Steven Rostedt Signed-off-by: Robert Richter --- drivers/oprofile/buffer_sync.c | 65 +++++++++++++++----------------------- drivers/oprofile/cpu_buffer.c | 63 ++++++++++++++++++++++++++++--------- drivers/oprofile/cpu_buffer.h | 71 ++++++++++++++++++++++++------------------ 3 files changed, 114 insertions(+), 85 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c index 944a5832d9e4..737bd9484822 100644 --- a/drivers/oprofile/buffer_sync.c +++ b/drivers/oprofile/buffer_sync.c @@ -268,18 +268,6 @@ lookup_dcookie(struct mm_struct *mm, unsigned long addr, off_t *offset) return cookie; } -static void increment_tail(struct oprofile_cpu_buffer *b) -{ - unsigned long new_tail = b->tail_pos + 1; - - rmb(); /* be sure fifo pointers are synchronized */ - - if (new_tail < b->buffer_size) - b->tail_pos = new_tail; - else - b->tail_pos = 0; -} - static unsigned long last_cookie = INVALID_COOKIE; static void add_cpu_switch(int i) @@ -331,26 +319,25 @@ static void add_trace_begin(void) #define IBS_FETCH_CODE_SIZE 2 #define IBS_OP_CODE_SIZE 5 -#define IBS_EIP(cpu_buf) ((cpu_buffer_read_entry(cpu_buf))->eip) -#define IBS_EVENT(cpu_buf) ((cpu_buffer_read_entry(cpu_buf))->event) /* * Add IBS fetch and op entries to event buffer */ -static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code, - struct mm_struct *mm) +static void add_ibs_begin(int cpu, int code, struct mm_struct *mm) { unsigned long rip; int i, count; unsigned long ibs_cookie = 0; off_t offset; + struct op_sample *sample; - increment_tail(cpu_buf); /* move to RIP entry */ - - rip = IBS_EIP(cpu_buf); + sample = cpu_buffer_read_entry(cpu); + if (!sample) + goto Error; + rip = sample->eip; #ifdef __LP64__ - rip += IBS_EVENT(cpu_buf) << 32; + rip += sample->event << 32; #endif if (mm) { @@ -374,8 +361,8 @@ static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code, add_event_entry(offset); /* Offset from Dcookie */ /* we send the Dcookie offset, but send the raw Linear Add also*/ - add_event_entry(IBS_EIP(cpu_buf)); - add_event_entry(IBS_EVENT(cpu_buf)); + add_event_entry(sample->eip); + add_event_entry(sample->event); if (code == IBS_FETCH_CODE) count = IBS_FETCH_CODE_SIZE; /*IBS FETCH is 2 int64s*/ @@ -383,10 +370,17 @@ static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code, count = IBS_OP_CODE_SIZE; /*IBS OP is 5 int64s*/ for (i = 0; i < count; i++) { - increment_tail(cpu_buf); - add_event_entry(IBS_EIP(cpu_buf)); - add_event_entry(IBS_EVENT(cpu_buf)); + sample = cpu_buffer_read_entry(cpu); + if (!sample) + goto Error; + add_event_entry(sample->eip); + add_event_entry(sample->event); } + + return; + +Error: + return; } #endif @@ -530,33 +524,26 @@ typedef enum { */ void sync_buffer(int cpu) { - struct oprofile_cpu_buffer *cpu_buf = &per_cpu(cpu_buffer, cpu); struct mm_struct *mm = NULL; struct mm_struct *oldmm; struct task_struct *new; unsigned long cookie = 0; int in_kernel = 1; sync_buffer_state state = sb_buffer_start; -#ifndef CONFIG_OPROFILE_IBS unsigned int i; unsigned long available; -#endif mutex_lock(&buffer_mutex); add_cpu_switch(cpu); - /* Remember, only we can modify tail_pos */ - cpu_buffer_reset(cpu); -#ifndef CONFIG_OPROFILE_IBS - available = cpu_buffer_entries(cpu_buf); + available = cpu_buffer_entries(cpu); for (i = 0; i < available; ++i) { -#else - while (cpu_buffer_entries(cpu_buf)) { -#endif - struct op_sample *s = cpu_buffer_read_entry(cpu_buf); + struct op_sample *s = cpu_buffer_read_entry(cpu); + if (!s) + break; if (is_code(s->eip)) { switch (s->event) { @@ -575,11 +562,11 @@ void sync_buffer(int cpu) #ifdef CONFIG_OPROFILE_IBS case IBS_FETCH_BEGIN: state = sb_bt_start; - add_ibs_begin(cpu_buf, IBS_FETCH_CODE, mm); + add_ibs_begin(cpu, IBS_FETCH_CODE, mm); break; case IBS_OP_BEGIN: state = sb_bt_start; - add_ibs_begin(cpu_buf, IBS_OP_CODE, mm); + add_ibs_begin(cpu, IBS_OP_CODE, mm); break; #endif default: @@ -600,8 +587,6 @@ void sync_buffer(int cpu) atomic_inc(&oprofile_stats.bt_lost_no_mapping); } } - - increment_tail(cpu_buf); } release_mm(mm); diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c index 5cf7efe38e67..eb280ec96e24 100644 --- a/drivers/oprofile/cpu_buffer.c +++ b/drivers/oprofile/cpu_buffer.c @@ -28,6 +28,25 @@ #include "buffer_sync.h" #include "oprof.h" +#define OP_BUFFER_FLAGS 0 + +/* + * Read and write access is using spin locking. Thus, writing to the + * buffer by NMI handler (x86) could occur also during critical + * sections when reading the buffer. To avoid this, there are 2 + * buffers for independent read and write access. Read access is in + * process context only, write access only in the NMI handler. If the + * read buffer runs empty, both buffers are swapped atomically. There + * is potentially a small window during swapping where the buffers are + * disabled and samples could be lost. + * + * Using 2 buffers is a little bit overhead, but the solution is clear + * and does not require changes in the ring buffer implementation. It + * can be changed to a single buffer solution when the ring buffer + * access is implemented as non-locking atomic code. + */ +struct ring_buffer *op_ring_buffer_read; +struct ring_buffer *op_ring_buffer_write; DEFINE_PER_CPU(struct oprofile_cpu_buffer, cpu_buffer); static void wq_sync_buffer(struct work_struct *work); @@ -37,12 +56,12 @@ static int work_enabled; void free_cpu_buffers(void) { - int i; - - for_each_possible_cpu(i) { - vfree(per_cpu(cpu_buffer, i).buffer); - per_cpu(cpu_buffer, i).buffer = NULL; - } + if (op_ring_buffer_read) + ring_buffer_free(op_ring_buffer_read); + op_ring_buffer_read = NULL; + if (op_ring_buffer_write) + ring_buffer_free(op_ring_buffer_write); + op_ring_buffer_write = NULL; } unsigned long oprofile_get_cpu_buffer_size(void) @@ -64,14 +83,16 @@ int alloc_cpu_buffers(void) unsigned long buffer_size = fs_cpu_buffer_size; + op_ring_buffer_read = ring_buffer_alloc(buffer_size, OP_BUFFER_FLAGS); + if (!op_ring_buffer_read) + goto fail; + op_ring_buffer_write = ring_buffer_alloc(buffer_size, OP_BUFFER_FLAGS); + if (!op_ring_buffer_write) + goto fail; + for_each_possible_cpu(i) { struct oprofile_cpu_buffer *b = &per_cpu(cpu_buffer, i); - b->buffer = vmalloc_node(sizeof(struct op_sample) * buffer_size, - cpu_to_node(i)); - if (!b->buffer) - goto fail; - b->last_task = NULL; b->last_is_kernel = -1; b->tracing = 0; @@ -140,10 +161,22 @@ static inline void add_sample(struct oprofile_cpu_buffer *cpu_buf, unsigned long pc, unsigned long event) { - struct op_sample *entry = cpu_buffer_write_entry(cpu_buf); - entry->eip = pc; - entry->event = event; - cpu_buffer_write_commit(cpu_buf); + struct op_entry entry; + + if (cpu_buffer_write_entry(&entry)) + goto Error; + + entry.sample->eip = pc; + entry.sample->event = event; + + if (cpu_buffer_write_commit(&entry)) + goto Error; + + return; + +Error: + cpu_buf->sample_lost_overflow++; + return; } static inline void diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h index 895763f065e9..aacb0f0bc566 100644 --- a/drivers/oprofile/cpu_buffer.h +++ b/drivers/oprofile/cpu_buffer.h @@ -15,6 +15,7 @@ #include #include #include +#include struct task_struct; @@ -32,6 +33,12 @@ struct op_sample { unsigned long event; }; +struct op_entry { + struct ring_buffer_event *event; + struct op_sample *sample; + unsigned long irq_flags; +}; + struct oprofile_cpu_buffer { volatile unsigned long head_pos; volatile unsigned long tail_pos; @@ -39,7 +46,6 @@ struct oprofile_cpu_buffer { struct task_struct *last_task; int last_is_kernel; int tracing; - struct op_sample *buffer; unsigned long sample_received; unsigned long sample_lost_overflow; unsigned long backtrace_aborted; @@ -48,6 +54,8 @@ struct oprofile_cpu_buffer { struct delayed_work work; }; +extern struct ring_buffer *op_ring_buffer_read; +extern struct ring_buffer *op_ring_buffer_write; DECLARE_PER_CPU(struct oprofile_cpu_buffer, cpu_buffer); /* @@ -64,46 +72,49 @@ static inline void cpu_buffer_reset(int cpu) cpu_buf->last_task = NULL; } -static inline -struct op_sample *cpu_buffer_write_entry(struct oprofile_cpu_buffer *cpu_buf) +static inline int cpu_buffer_write_entry(struct op_entry *entry) { - return &cpu_buf->buffer[cpu_buf->head_pos]; -} + entry->event = ring_buffer_lock_reserve(op_ring_buffer_write, + sizeof(struct op_sample), + &entry->irq_flags); + if (entry->event) + entry->sample = ring_buffer_event_data(entry->event); + else + entry->sample = NULL; -static inline -void cpu_buffer_write_commit(struct oprofile_cpu_buffer *b) -{ - unsigned long new_head = b->head_pos + 1; + if (!entry->sample) + return -ENOMEM; - /* - * Ensure anything written to the slot before we increment is - * visible - */ - wmb(); + return 0; +} - if (new_head < b->buffer_size) - b->head_pos = new_head; - else - b->head_pos = 0; +static inline int cpu_buffer_write_commit(struct op_entry *entry) +{ + return ring_buffer_unlock_commit(op_ring_buffer_write, entry->event, + entry->irq_flags); } -static inline -struct op_sample *cpu_buffer_read_entry(struct oprofile_cpu_buffer *cpu_buf) +static inline struct op_sample *cpu_buffer_read_entry(int cpu) { - return &cpu_buf->buffer[cpu_buf->tail_pos]; + struct ring_buffer_event *e; + e = ring_buffer_consume(op_ring_buffer_read, cpu, NULL); + if (e) + return ring_buffer_event_data(e); + if (ring_buffer_swap_cpu(op_ring_buffer_read, + op_ring_buffer_write, + cpu)) + return NULL; + e = ring_buffer_consume(op_ring_buffer_read, cpu, NULL); + if (e) + return ring_buffer_event_data(e); + return NULL; } /* "acquire" as many cpu buffer slots as we can */ -static inline -unsigned long cpu_buffer_entries(struct oprofile_cpu_buffer *b) +static inline unsigned long cpu_buffer_entries(int cpu) { - unsigned long head = b->head_pos; - unsigned long tail = b->tail_pos; - - if (head >= tail) - return head - tail; - - return head + (b->buffer_size - tail); + return ring_buffer_entries_cpu(op_ring_buffer_read, cpu) + + ring_buffer_entries_cpu(op_ring_buffer_write, cpu); } /* transient events for the CPU buffer -> event buffer */ -- cgit v1.2.3 From 1d7503b5dccf2b95babca050e4960e10d2633f2b Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Mon, 8 Dec 2008 11:59:52 +0100 Subject: oprofile: remove nr_available_slots() This function is no longer available after the port to the new ring buffer. Its removal can lead to incomplete sampling sequences since IBS samples and backtraces are transfered in multiple samples. Due to a full buffer, samples could be lost any time. The userspace daemon has to live with such incomplete sampling sequences as long as the data within one sample is consistent. This will be fixed by changing the internal buffer data there all data of one IBS sample or a backtrace is packed in a single ring buffer entry. This is possible since the new ring buffer supports variable data size. Signed-off-by: Robert Richter --- drivers/oprofile/cpu_buffer.c | 34 ---------------------------------- 1 file changed, 34 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c index eb280ec96e24..7f7fc958297a 100644 --- a/drivers/oprofile/cpu_buffer.c +++ b/drivers/oprofile/cpu_buffer.c @@ -145,18 +145,6 @@ void end_cpu_work(void) flush_scheduled_work(); } -/* compute number of available slots in cpu_buffer queue */ -static unsigned long nr_available_slots(struct oprofile_cpu_buffer const *b) -{ - unsigned long head = b->head_pos; - unsigned long tail = b->tail_pos; - - if (tail > head) - return (tail - head) - 1; - - return tail + (b->buffer_size - head) - 1; -} - static inline void add_sample(struct oprofile_cpu_buffer *cpu_buf, unsigned long pc, unsigned long event) @@ -206,11 +194,6 @@ static int log_sample(struct oprofile_cpu_buffer *cpu_buf, unsigned long pc, return 0; } - if (nr_available_slots(cpu_buf) < 3) { - cpu_buf->sample_lost_overflow++; - return 0; - } - is_kernel = !!is_kernel; task = current; @@ -233,11 +216,6 @@ static int log_sample(struct oprofile_cpu_buffer *cpu_buf, unsigned long pc, static int oprofile_begin_trace(struct oprofile_cpu_buffer *cpu_buf) { - if (nr_available_slots(cpu_buf) < 4) { - cpu_buf->sample_lost_overflow++; - return 0; - } - add_code(cpu_buf, CPU_TRACE_BEGIN); cpu_buf->tracing = 1; return 1; @@ -291,12 +269,6 @@ void oprofile_add_ibs_sample(struct pt_regs * const regs, cpu_buf->sample_received++; - if (nr_available_slots(cpu_buf) < MAX_IBS_SAMPLE_SIZE) { - /* we can't backtrace since we lost the source of this event */ - cpu_buf->sample_lost_overflow++; - return; - } - /* notice a switch from user->kernel or vice versa */ if (cpu_buf->last_is_kernel != is_kernel) { cpu_buf->last_is_kernel = is_kernel; @@ -342,12 +314,6 @@ void oprofile_add_trace(unsigned long pc) if (!cpu_buf->tracing) return; - if (nr_available_slots(cpu_buf) < 1) { - cpu_buf->tracing = 0; - cpu_buf->sample_lost_overflow++; - return; - } - /* * broken frame can give an eip with the same value as an * escape code, abort the trace if we get it -- cgit v1.2.3 From 211117ff09b7d81d91b7857651587128ed8b13d9 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Tue, 9 Dec 2008 02:13:25 +0100 Subject: oprofile: fix lost sample counter The number of lost samples could be greater than the number of received samples. This patches fixes this. The implementation introduces return values for add_sample() and add_code(). Signed-off-by: Robert Richter --- drivers/oprofile/cpu_buffer.c | 83 +++++++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 30 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c index 7f7fc958297a..61090969158f 100644 --- a/drivers/oprofile/cpu_buffer.c +++ b/drivers/oprofile/cpu_buffer.c @@ -145,32 +145,31 @@ void end_cpu_work(void) flush_scheduled_work(); } -static inline void +static inline int add_sample(struct oprofile_cpu_buffer *cpu_buf, unsigned long pc, unsigned long event) { struct op_entry entry; + int ret; - if (cpu_buffer_write_entry(&entry)) - goto Error; + ret = cpu_buffer_write_entry(&entry); + if (ret) + return ret; entry.sample->eip = pc; entry.sample->event = event; - if (cpu_buffer_write_commit(&entry)) - goto Error; + ret = cpu_buffer_write_commit(&entry); + if (ret) + return ret; - return; - -Error: - cpu_buf->sample_lost_overflow++; - return; + return 0; } -static inline void +static inline int add_code(struct oprofile_cpu_buffer *buffer, unsigned long value) { - add_sample(buffer, ESCAPE_CODE, value); + return add_sample(buffer, ESCAPE_CODE, value); } /* This must be safe from any context. It's safe writing here @@ -201,17 +200,25 @@ static int log_sample(struct oprofile_cpu_buffer *cpu_buf, unsigned long pc, /* notice a switch from user->kernel or vice versa */ if (cpu_buf->last_is_kernel != is_kernel) { cpu_buf->last_is_kernel = is_kernel; - add_code(cpu_buf, is_kernel); + if (add_code(cpu_buf, is_kernel)) + goto fail; } /* notice a task switch */ if (cpu_buf->last_task != task) { cpu_buf->last_task = task; - add_code(cpu_buf, (unsigned long)task); + if (add_code(cpu_buf, (unsigned long)task)) + goto fail; } - add_sample(cpu_buf, pc, event); + if (add_sample(cpu_buf, pc, event)) + goto fail; + return 1; + +fail: + cpu_buf->sample_lost_overflow++; + return 0; } static int oprofile_begin_trace(struct oprofile_cpu_buffer *cpu_buf) @@ -266,37 +273,49 @@ void oprofile_add_ibs_sample(struct pt_regs * const regs, int is_kernel = !user_mode(regs); struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(cpu_buffer); struct task_struct *task; + int fail = 0; cpu_buf->sample_received++; /* notice a switch from user->kernel or vice versa */ if (cpu_buf->last_is_kernel != is_kernel) { + if (add_code(cpu_buf, is_kernel)) + goto fail; cpu_buf->last_is_kernel = is_kernel; - add_code(cpu_buf, is_kernel); } /* notice a task switch */ if (!is_kernel) { task = current; if (cpu_buf->last_task != task) { + if (add_code(cpu_buf, (unsigned long)task)) + goto fail; cpu_buf->last_task = task; - add_code(cpu_buf, (unsigned long)task); } } - add_code(cpu_buf, ibs_code); - add_sample(cpu_buf, ibs_sample[0], ibs_sample[1]); - add_sample(cpu_buf, ibs_sample[2], ibs_sample[3]); - add_sample(cpu_buf, ibs_sample[4], ibs_sample[5]); + fail = fail || add_code(cpu_buf, ibs_code); + fail = fail || add_sample(cpu_buf, ibs_sample[0], ibs_sample[1]); + fail = fail || add_sample(cpu_buf, ibs_sample[2], ibs_sample[3]); + fail = fail || add_sample(cpu_buf, ibs_sample[4], ibs_sample[5]); if (ibs_code == IBS_OP_BEGIN) { - add_sample(cpu_buf, ibs_sample[6], ibs_sample[7]); - add_sample(cpu_buf, ibs_sample[8], ibs_sample[9]); - add_sample(cpu_buf, ibs_sample[10], ibs_sample[11]); + fail = fail || add_sample(cpu_buf, ibs_sample[6], ibs_sample[7]); + fail = fail || add_sample(cpu_buf, ibs_sample[8], ibs_sample[9]); + fail = fail || add_sample(cpu_buf, ibs_sample[10], ibs_sample[11]); } + if (fail) + goto fail; + if (backtrace_depth) oprofile_ops.backtrace(regs, backtrace_depth); + + return; + +fail: + cpu_buf->sample_lost_overflow++; + return; } #endif @@ -318,13 +337,17 @@ void oprofile_add_trace(unsigned long pc) * broken frame can give an eip with the same value as an * escape code, abort the trace if we get it */ - if (pc == ESCAPE_CODE) { - cpu_buf->tracing = 0; - cpu_buf->backtrace_aborted++; - return; - } + if (pc == ESCAPE_CODE) + goto fail; + + if (add_sample(cpu_buf, pc, 0)) + goto fail; - add_sample(cpu_buf, pc, 0); + return; +fail: + cpu_buf->tracing = 0; + cpu_buf->backtrace_aborted++; + return; } /* -- cgit v1.2.3