Subject: [0/18] oprofile: fixes and cleanup patches

This patch series contains some fixes and cleanups, mostly
trivial. The patches are in preparation of the implementation of
support for variable data size samples.



Subject: [PATCH 05/18] oprofile: reordering some code in cpu_buffer.c

Reordering code to keep alloc/free functions together.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/cpu_buffer.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index cd67d4d..b353b19 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -54,16 +54,6 @@ static void wq_sync_buffer(struct work_struct *work);
#define DEFAULT_TIMER_EXPIRE (HZ / 10)
static int work_enabled;

-void free_cpu_buffers(void)
-{
- 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)
{
return oprofile_cpu_buffer_size;
@@ -77,6 +67,16 @@ void oprofile_cpu_buffer_inc_smpl_lost(void)
cpu_buf->sample_lost_overflow++;
}

+void free_cpu_buffers(void)
+{
+ 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;
+}
+
int alloc_cpu_buffers(void)
{
int i;
--
1.6.0.1

Subject: [PATCH 01/18] oprofile: rename kernel-wide identifiers

This patch renames kernel-wide identifiers to something more oprofile
specific names.

Cc: Andrew Morton <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/cpu_buffer.c | 12 ++++++------
drivers/oprofile/event_buffer.c | 4 ++--
drivers/oprofile/oprof.c | 4 ++--
drivers/oprofile/oprof.h | 8 ++++----
drivers/oprofile/oprofile_files.c | 27 ++++++++++++++-------------
5 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index 6109096..fcf96f6 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -66,7 +66,7 @@ void free_cpu_buffers(void)

unsigned long oprofile_get_cpu_buffer_size(void)
{
- return fs_cpu_buffer_size;
+ return oprofile_cpu_buffer_size;
}

void oprofile_cpu_buffer_inc_smpl_lost(void)
@@ -81,7 +81,7 @@ int alloc_cpu_buffers(void)
{
int i;

- unsigned long buffer_size = fs_cpu_buffer_size;
+ unsigned long buffer_size = oprofile_cpu_buffer_size;

op_ring_buffer_read = ring_buffer_alloc(buffer_size, OP_BUFFER_FLAGS);
if (!op_ring_buffer_read)
@@ -238,7 +238,7 @@ void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
{
struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(cpu_buffer);

- if (!backtrace_depth) {
+ if (!oprofile_backtrace_depth) {
log_sample(cpu_buf, pc, is_kernel, event);
return;
}
@@ -251,7 +251,7 @@ void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
* source of this event
*/
if (log_sample(cpu_buf, pc, is_kernel, event))
- oprofile_ops.backtrace(regs, backtrace_depth);
+ oprofile_ops.backtrace(regs, oprofile_backtrace_depth);
oprofile_end_trace(cpu_buf);
}

@@ -308,8 +308,8 @@ void oprofile_add_ibs_sample(struct pt_regs * const regs,
if (fail)
goto fail;

- if (backtrace_depth)
- oprofile_ops.backtrace(regs, backtrace_depth);
+ if (oprofile_backtrace_depth)
+ oprofile_ops.backtrace(regs, oprofile_backtrace_depth);

return;

diff --git a/drivers/oprofile/event_buffer.c b/drivers/oprofile/event_buffer.c
index 191a320..2b7ae36 100644
--- a/drivers/oprofile/event_buffer.c
+++ b/drivers/oprofile/event_buffer.c
@@ -73,8 +73,8 @@ int alloc_event_buffer(void)
unsigned long flags;

spin_lock_irqsave(&oprofilefs_lock, flags);
- buffer_size = fs_buffer_size;
- buffer_watershed = fs_buffer_watershed;
+ buffer_size = oprofile_buffer_size;
+ buffer_watershed = oprofile_buffer_watershed;
spin_unlock_irqrestore(&oprofilefs_lock, flags);

if (buffer_watershed >= buffer_size)
diff --git a/drivers/oprofile/oprof.c b/drivers/oprofile/oprof.c
index cd37590..3cffce9 100644
--- a/drivers/oprofile/oprof.c
+++ b/drivers/oprofile/oprof.c
@@ -23,7 +23,7 @@
struct oprofile_operations oprofile_ops;

unsigned long oprofile_started;
-unsigned long backtrace_depth;
+unsigned long oprofile_backtrace_depth;
static unsigned long is_setup;
static DEFINE_MUTEX(start_mutex);

@@ -172,7 +172,7 @@ int oprofile_set_backtrace(unsigned long val)
goto out;
}

- backtrace_depth = val;
+ oprofile_backtrace_depth = val;

out:
mutex_unlock(&start_mutex);
diff --git a/drivers/oprofile/oprof.h b/drivers/oprofile/oprof.h
index 5df0c21..c288d3c 100644
--- a/drivers/oprofile/oprof.h
+++ b/drivers/oprofile/oprof.h
@@ -21,12 +21,12 @@ void oprofile_stop(void);

struct oprofile_operations;

-extern unsigned long fs_buffer_size;
-extern unsigned long fs_cpu_buffer_size;
-extern unsigned long fs_buffer_watershed;
+extern unsigned long oprofile_buffer_size;
+extern unsigned long oprofile_cpu_buffer_size;
+extern unsigned long oprofile_buffer_watershed;
extern struct oprofile_operations oprofile_ops;
extern unsigned long oprofile_started;
-extern unsigned long backtrace_depth;
+extern unsigned long oprofile_backtrace_depth;

struct super_block;
struct dentry;
diff --git a/drivers/oprofile/oprofile_files.c b/drivers/oprofile/oprofile_files.c
index d820199..5d36ffc 100644
--- a/drivers/oprofile/oprofile_files.c
+++ b/drivers/oprofile/oprofile_files.c
@@ -14,17 +14,18 @@
#include "oprofile_stats.h"
#include "oprof.h"

-#define FS_BUFFER_SIZE_DEFAULT 131072
-#define FS_CPU_BUFFER_SIZE_DEFAULT 8192
-#define FS_BUFFER_WATERSHED_DEFAULT 32768 /* FIXME: tune */
+#define BUFFER_SIZE_DEFAULT 131072
+#define CPU_BUFFER_SIZE_DEFAULT 8192
+#define BUFFER_WATERSHED_DEFAULT 32768 /* FIXME: tune */

-unsigned long fs_buffer_size;
-unsigned long fs_cpu_buffer_size;
-unsigned long fs_buffer_watershed;
+unsigned long oprofile_buffer_size;
+unsigned long oprofile_cpu_buffer_size;
+unsigned long oprofile_buffer_watershed;

static ssize_t depth_read(struct file *file, char __user *buf, size_t count, loff_t *offset)
{
- return oprofilefs_ulong_to_user(backtrace_depth, buf, count, offset);
+ return oprofilefs_ulong_to_user(oprofile_backtrace_depth, buf, count,
+ offset);
}


@@ -125,16 +126,16 @@ 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;
+ oprofile_buffer_size = BUFFER_SIZE_DEFAULT;
+ oprofile_cpu_buffer_size = CPU_BUFFER_SIZE_DEFAULT;
+ oprofile_buffer_watershed = 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);
- oprofilefs_create_ulong(sb, root, "buffer_size", &fs_buffer_size);
- oprofilefs_create_ulong(sb, root, "buffer_watershed", &fs_buffer_watershed);
- oprofilefs_create_ulong(sb, root, "cpu_buffer_size", &fs_cpu_buffer_size);
+ oprofilefs_create_ulong(sb, root, "buffer_size", &oprofile_buffer_size);
+ oprofilefs_create_ulong(sb, root, "buffer_watershed", &oprofile_buffer_watershed);
+ oprofilefs_create_ulong(sb, root, "cpu_buffer_size", &oprofile_cpu_buffer_size);
oprofilefs_create_file(sb, root, "cpu_type", &cpu_type_fops);
oprofilefs_create_file(sb, root, "backtrace_depth", &depth_fops);
oprofilefs_create_file(sb, root, "pointer_size", &pointer_size_fops);
--
1.6.0.1

Subject: [PATCH 04/18] x86/oprofile: fix pci_dev use count for AMD northbridge devices

This patch fixes the PCI device use count for AMD northbridge
devices. In case of an IBS LVT initialization failure, the PCI device
is released now by calling pci_dev_put().

If there are no initialization errors, the devices are released in
pci_get_device() while iterating.

Signed-off-by: Robert Richter <[email protected]>
---
arch/x86/oprofile/op_model_amd.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
index 98658f2..c5b5c7f 100644
--- a/arch/x86/oprofile/op_model_amd.c
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -409,6 +409,7 @@ static int init_ibs_nmi(void)
| IBSCTL_LVTOFFSETVAL);
pci_read_config_dword(cpu_cfg, IBSCTL, &value);
if (value != (ibs_eilvt_off | IBSCTL_LVTOFFSETVAL)) {
+ pci_dev_put(cpu_cfg);
printk(KERN_DEBUG "Failed to setup IBS LVT offset, "
"IBSCTL = 0x%08x", value);
return 1;
--
1.6.0.1

Subject: [PATCH 02/18] oprofile: rename cpu buffer functions

This patch renames cpu buffer functions to something more oprofile
specific names. Functions will be moved to the global name space.

Cc: Andrew Morton <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/buffer_sync.c | 10 +++++-----
drivers/oprofile/cpu_buffer.c | 4 ++--
drivers/oprofile/cpu_buffer.h | 10 +++++-----
3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index 737bd94..d295d92 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -331,7 +331,7 @@ static void add_ibs_begin(int cpu, int code, struct mm_struct *mm)
off_t offset;
struct op_sample *sample;

- sample = cpu_buffer_read_entry(cpu);
+ sample = op_cpu_buffer_read_entry(cpu);
if (!sample)
goto Error;
rip = sample->eip;
@@ -370,7 +370,7 @@ static void add_ibs_begin(int cpu, int code, struct mm_struct *mm)
count = IBS_OP_CODE_SIZE; /*IBS OP is 5 int64s*/

for (i = 0; i < count; i++) {
- sample = cpu_buffer_read_entry(cpu);
+ sample = op_cpu_buffer_read_entry(cpu);
if (!sample)
goto Error;
add_event_entry(sample->eip);
@@ -537,11 +537,11 @@ void sync_buffer(int cpu)

add_cpu_switch(cpu);

- cpu_buffer_reset(cpu);
- available = cpu_buffer_entries(cpu);
+ op_cpu_buffer_reset(cpu);
+ available = op_cpu_buffer_entries(cpu);

for (i = 0; i < available; ++i) {
- struct op_sample *s = cpu_buffer_read_entry(cpu);
+ struct op_sample *s = op_cpu_buffer_read_entry(cpu);
if (!s)
break;

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index fcf96f6..e52c085 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -152,14 +152,14 @@ add_sample(struct oprofile_cpu_buffer *cpu_buf,
struct op_entry entry;
int ret;

- ret = cpu_buffer_write_entry(&entry);
+ ret = op_cpu_buffer_write_entry(&entry);
if (ret)
return ret;

entry.sample->eip = pc;
entry.sample->event = event;

- ret = cpu_buffer_write_commit(&entry);
+ ret = op_cpu_buffer_write_commit(&entry);
if (ret)
return ret;

diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h
index aacb0f0..83d491e 100644
--- a/drivers/oprofile/cpu_buffer.h
+++ b/drivers/oprofile/cpu_buffer.h
@@ -64,7 +64,7 @@ DECLARE_PER_CPU(struct oprofile_cpu_buffer, cpu_buffer);
* 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)
+static inline void op_cpu_buffer_reset(int cpu)
{
struct oprofile_cpu_buffer *cpu_buf = &per_cpu(cpu_buffer, cpu);

@@ -72,7 +72,7 @@ static inline void cpu_buffer_reset(int cpu)
cpu_buf->last_task = NULL;
}

-static inline int cpu_buffer_write_entry(struct op_entry *entry)
+static inline int op_cpu_buffer_write_entry(struct op_entry *entry)
{
entry->event = ring_buffer_lock_reserve(op_ring_buffer_write,
sizeof(struct op_sample),
@@ -88,13 +88,13 @@ static inline int cpu_buffer_write_entry(struct op_entry *entry)
return 0;
}

-static inline int cpu_buffer_write_commit(struct op_entry *entry)
+static inline int op_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(int cpu)
+static inline struct op_sample *op_cpu_buffer_read_entry(int cpu)
{
struct ring_buffer_event *e;
e = ring_buffer_consume(op_ring_buffer_read, cpu, NULL);
@@ -111,7 +111,7 @@ static inline struct op_sample *cpu_buffer_read_entry(int cpu)
}

/* "acquire" as many cpu buffer slots as we can */
-static inline unsigned long cpu_buffer_entries(int cpu)
+static inline unsigned long op_cpu_buffer_entries(int cpu)
{
return ring_buffer_entries_cpu(op_ring_buffer_read, cpu)
+ ring_buffer_entries_cpu(op_ring_buffer_write, cpu);
--
1.6.0.1

Subject: [PATCH 10/18] oprofile: simplify add_sample() in cpu_buffer.c

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/cpu_buffer.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index 435bd6e..d92f002 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -203,11 +203,7 @@ add_sample(struct oprofile_cpu_buffer *cpu_buf,
entry.sample->eip = pc;
entry.sample->event = event;

- ret = op_cpu_buffer_write_commit(&entry);
- if (ret)
- return ret;
-
- return 0;
+ return op_cpu_buffer_write_commit(&entry);
}

static inline int
--
1.6.0.1

Subject: [PATCH 03/18] oprofile: remove ring buffer inline functions in cpu_buffer.h

This patch moves ring buffer inline functions to cpu_buffer.c.

Cc: Andrew Morton <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/cpu_buffer.c | 48 +++++++++++++++++++++++++++++++++++++-
drivers/oprofile/cpu_buffer.h | 50 +++-------------------------------------
2 files changed, 50 insertions(+), 48 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index e52c085..cd67d4d 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -45,8 +45,8 @@
* 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;
+static struct ring_buffer *op_ring_buffer_read;
+static 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);
@@ -145,6 +145,50 @@ void end_cpu_work(void)
flush_scheduled_work();
}

+int op_cpu_buffer_write_entry(struct op_entry *entry)
+{
+ 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;
+
+ if (!entry->sample)
+ return -ENOMEM;
+
+ return 0;
+}
+
+int op_cpu_buffer_write_commit(struct op_entry *entry)
+{
+ return ring_buffer_unlock_commit(op_ring_buffer_write, entry->event,
+ entry->irq_flags);
+}
+
+struct op_sample *op_cpu_buffer_read_entry(int cpu)
+{
+ 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;
+}
+
+unsigned long op_cpu_buffer_entries(int cpu)
+{
+ return ring_buffer_entries_cpu(op_ring_buffer_read, cpu)
+ + ring_buffer_entries_cpu(op_ring_buffer_write, cpu);
+}
+
static inline int
add_sample(struct oprofile_cpu_buffer *cpu_buf,
unsigned long pc, unsigned long event)
diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h
index 83d491e..cd28abc 100644
--- a/drivers/oprofile/cpu_buffer.h
+++ b/drivers/oprofile/cpu_buffer.h
@@ -54,8 +54,6 @@ 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);

/*
@@ -72,50 +70,10 @@ static inline void op_cpu_buffer_reset(int cpu)
cpu_buf->last_task = NULL;
}

-static inline int op_cpu_buffer_write_entry(struct op_entry *entry)
-{
- 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;
-
- if (!entry->sample)
- return -ENOMEM;
-
- return 0;
-}
-
-static inline int op_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 *op_cpu_buffer_read_entry(int cpu)
-{
- 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 op_cpu_buffer_entries(int cpu)
-{
- return ring_buffer_entries_cpu(op_ring_buffer_read, cpu)
- + ring_buffer_entries_cpu(op_ring_buffer_write, cpu);
-}
+int op_cpu_buffer_write_entry(struct op_entry *entry);
+int op_cpu_buffer_write_commit(struct op_entry *entry);
+struct op_sample *op_cpu_buffer_read_entry(int cpu);
+unsigned long op_cpu_buffer_entries(int cpu);

/* transient events for the CPU buffer -> event buffer */
#define CPU_IS_KERNEL 1
--
1.6.0.1

Subject: [PATCH 07/18] oprofile: simplify add_sample()

This patch removes add_us_sample() and simplifies add_sample(). Code
is much more readable now.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/buffer_sync.c | 39 +++++++++++++++++++--------------------
1 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index d295d92..0abe29e 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -392,11 +392,29 @@ static void add_sample_entry(unsigned long offset, unsigned long event)
}


-static int add_us_sample(struct mm_struct *mm, struct op_sample *s)
+/*
+ * Add a sample to the global event buffer. If possible the
+ * sample is converted into a persistent dentry/offset pair
+ * for later lookup from userspace. Return 0 on failure.
+ */
+static int
+add_sample(struct mm_struct *mm, struct op_sample *s, int in_kernel)
{
unsigned long cookie;
off_t offset;

+ if (in_kernel) {
+ add_sample_entry(s->eip, s->event);
+ return 1;
+ }
+
+ /* add userspace sample */
+
+ if (!mm) {
+ atomic_inc(&oprofile_stats.sample_lost_no_mm);
+ return 0;
+ }
+
cookie = lookup_dcookie(mm, s->eip, &offset);

if (cookie == INVALID_COOKIE) {
@@ -415,25 +433,6 @@ static int add_us_sample(struct mm_struct *mm, struct op_sample *s)
}


-/* Add a sample to the global event buffer. If possible the
- * sample is converted into a persistent dentry/offset pair
- * for later lookup from userspace.
- */
-static int
-add_sample(struct mm_struct *mm, struct op_sample *s, int in_kernel)
-{
- if (in_kernel) {
- add_sample_entry(s->eip, s->event);
- return 1;
- } else if (mm) {
- return add_us_sample(mm, s);
- } else {
- atomic_inc(&oprofile_stats.sample_lost_no_mm);
- }
- return 0;
-}
-
-
static void release_mm(struct mm_struct *mm)
{
if (!mm)
--
1.6.0.1

Subject: [PATCH 12/18] oprofile: remove unused components in struct oprofile_cpu_buffer

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/cpu_buffer.c | 2 --
drivers/oprofile/cpu_buffer.h | 2 --
2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index d92f002..b426ae8 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -97,8 +97,6 @@ int alloc_cpu_buffers(void)
b->last_is_kernel = -1;
b->tracing = 0;
b->buffer_size = buffer_size;
- b->tail_pos = 0;
- b->head_pos = 0;
b->sample_received = 0;
b->sample_lost_overflow = 0;
b->backtrace_aborted = 0;
diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h
index cd28abc..65b763a 100644
--- a/drivers/oprofile/cpu_buffer.h
+++ b/drivers/oprofile/cpu_buffer.h
@@ -40,8 +40,6 @@ struct op_entry {
};

struct oprofile_cpu_buffer {
- volatile unsigned long head_pos;
- volatile unsigned long tail_pos;
unsigned long buffer_size;
struct task_struct *last_task;
int last_is_kernel;
--
1.6.0.1

Subject: [PATCH 08/18] oprofile: simplify sync_buffer()

Make code more readable. No functional changes.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/buffer_sync.c | 20 ++++++++++++++------
1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index 0abe29e..22cdb51 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -579,12 +579,20 @@ void sync_buffer(int cpu)
add_user_ctx_switch(new, cookie);
break;
}
- } else if (state >= sb_bt_start &&
- !add_sample(mm, s, in_kernel)) {
- if (state == sb_bt_start) {
- state = sb_bt_ignore;
- atomic_inc(&oprofile_stats.bt_lost_no_mapping);
- }
+ continue;
+ }
+
+ if (state < sb_bt_start)
+ /* ignore sample */
+ continue;
+
+ if (add_sample(mm, s, in_kernel))
+ continue;
+
+ /* ignore backtraces if failed to add a sample */
+ if (state == sb_bt_start) {
+ state = sb_bt_ignore;
+ atomic_inc(&oprofile_stats.bt_lost_no_mapping);
}
}
release_mm(mm);
--
1.6.0.1

Subject: [PATCH 06/18] oprofile: add inline function __oprofile_add_ext_sample()

This patch adds the inline function __oprofile_add_ext_sample() to
cpu_buffer.c and thus reduces overhead when calling
oprofile_add_sample().

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/cpu_buffer.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index b353b19..9e66c38 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -277,8 +277,9 @@ static void oprofile_end_trace(struct oprofile_cpu_buffer *cpu_buf)
cpu_buf->tracing = 0;
}

-void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
- unsigned long event, int is_kernel)
+static inline void
+__oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
+ unsigned long event, int is_kernel)
{
struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(cpu_buffer);

@@ -299,12 +300,18 @@ void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
oprofile_end_trace(cpu_buf);
}

+void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
+ unsigned long event, int is_kernel)
+{
+ __oprofile_add_ext_sample(pc, regs, event, is_kernel);
+}
+
void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
{
int is_kernel = !user_mode(regs);
unsigned long pc = profile_pc(regs);

- oprofile_add_ext_sample(pc, regs, event, is_kernel);
+ __oprofile_add_ext_sample(pc, regs, event, is_kernel);
}

#ifdef CONFIG_OPROFILE_IBS
--
1.6.0.1

Subject: [PATCH 15/18] oprofile: making add_sample_entry() inline

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/buffer_sync.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index e61e25f..bf8fcc7 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -382,7 +382,7 @@ static void add_ibs_begin(int cpu, int code, struct mm_struct *mm)

#endif

-static void add_sample_entry(unsigned long offset, unsigned long event)
+static inline void add_sample_entry(unsigned long offset, unsigned long event)
{
add_event_entry(offset);
add_event_entry(event);
--
1.6.0.1

Subject: [PATCH 13/18] oprofile: remove unused ibs macro

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/cpu_buffer.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index b426ae8..8ae37c9 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -309,8 +309,6 @@ void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)

#ifdef CONFIG_OPROFILE_IBS

-#define MAX_IBS_SAMPLE_SIZE 14
-
void oprofile_add_ibs_sample(struct pt_regs * const regs,
unsigned int * const ibs_sample, int ibs_code)
{
--
1.6.0.1

Subject: [PATCH 18/18] oprofile: rename variables in add_ibs_begin()

This unifies usage of variable names within oprofile.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/buffer_sync.c | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index bf8fcc7..21fd249 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -325,36 +325,36 @@ static void add_trace_begin(void)
*/
static void add_ibs_begin(int cpu, int code, struct mm_struct *mm)
{
- unsigned long rip;
+ unsigned long pc;
int i, count;
- unsigned long ibs_cookie = 0;
+ unsigned long cookie = 0;
off_t offset;
struct op_sample *sample;

sample = op_cpu_buffer_read_entry(cpu);
if (!sample)
return;
- rip = sample->eip;
+ pc = sample->eip;

#ifdef __LP64__
- rip += sample->event << 32;
+ pc += sample->event << 32;
#endif

if (mm) {
- ibs_cookie = lookup_dcookie(mm, rip, &offset);
+ cookie = lookup_dcookie(mm, pc, &offset);

- if (ibs_cookie == NO_COOKIE)
- offset = rip;
- if (ibs_cookie == INVALID_COOKIE) {
+ if (cookie == NO_COOKIE)
+ offset = pc;
+ if (cookie == INVALID_COOKIE) {
atomic_inc(&oprofile_stats.sample_lost_no_mapping);
- offset = rip;
+ offset = pc;
}
- if (ibs_cookie != last_cookie) {
- add_cookie_switch(ibs_cookie);
- last_cookie = ibs_cookie;
+ if (cookie != last_cookie) {
+ add_cookie_switch(cookie);
+ last_cookie = cookie;
}
} else
- offset = rip;
+ offset = pc;

add_event_entry(ESCAPE_CODE);
add_event_entry(code);
--
1.6.0.1

Subject: [PATCH 14/18] oprofile: remove backtrace code for ibs

This code is broken since a TRACE_BEGIN_CODE is never sent to the
daemon. The data becomes corrupt since the backtrace is interpreted as
ibs sample.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/buffer_sync.c | 2 --
drivers/oprofile/cpu_buffer.c | 10 ++--------
2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index 7415d2e..e61e25f 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -557,11 +557,9 @@ void sync_buffer(int cpu)
break;
#ifdef CONFIG_OPROFILE_IBS
case IBS_FETCH_BEGIN:
- state = sb_bt_start;
add_ibs_begin(cpu, IBS_FETCH_CODE, mm);
break;
case IBS_OP_BEGIN:
- state = sb_bt_start;
add_ibs_begin(cpu, IBS_OP_CODE, mm);
break;
#endif
diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index 8ae37c9..92bf8c0 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -347,17 +347,11 @@ void oprofile_add_ibs_sample(struct pt_regs * const regs,
fail = fail || add_sample(cpu_buf, ibs_sample[10], ibs_sample[11]);
}

- if (fail)
- goto fail;
-
- if (oprofile_backtrace_depth)
- oprofile_ops.backtrace(regs, oprofile_backtrace_depth);
-
- return;
+ if (!fail)
+ return;

fail:
cpu_buf->sample_lost_overflow++;
- return;
}

#endif
--
1.6.0.1

Subject: [PATCH 17/18] oprofile: rename add_sample() in cpu_buffer.c

Rename the fucntion to op_add_sample() since there is a collision with
another one with the same name in buffer_sync.c.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/cpu_buffer.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index 92bf8c0..ac79f66 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -188,8 +188,8 @@ unsigned long op_cpu_buffer_entries(int cpu)
}

static inline int
-add_sample(struct oprofile_cpu_buffer *cpu_buf,
- unsigned long pc, unsigned long event)
+op_add_sample(struct oprofile_cpu_buffer *cpu_buf,
+ unsigned long pc, unsigned long event)
{
struct op_entry entry;
int ret;
@@ -207,7 +207,7 @@ add_sample(struct oprofile_cpu_buffer *cpu_buf,
static inline int
add_code(struct oprofile_cpu_buffer *buffer, unsigned long value)
{
- return add_sample(buffer, ESCAPE_CODE, value);
+ return op_add_sample(buffer, ESCAPE_CODE, value);
}

/* This must be safe from any context. It's safe writing here
@@ -249,7 +249,7 @@ static int log_sample(struct oprofile_cpu_buffer *cpu_buf, unsigned long pc,
goto fail;
}

- if (add_sample(cpu_buf, pc, event))
+ if (op_add_sample(cpu_buf, pc, event))
goto fail;

return 1;
@@ -337,14 +337,14 @@ void oprofile_add_ibs_sample(struct pt_regs * const regs,
}

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]);
+ fail = fail || op_add_sample(cpu_buf, ibs_sample[0], ibs_sample[1]);
+ fail = fail || op_add_sample(cpu_buf, ibs_sample[2], ibs_sample[3]);
+ fail = fail || op_add_sample(cpu_buf, ibs_sample[4], ibs_sample[5]);

if (ibs_code == IBS_OP_BEGIN) {
- 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]);
+ fail = fail || op_add_sample(cpu_buf, ibs_sample[6], ibs_sample[7]);
+ fail = fail || op_add_sample(cpu_buf, ibs_sample[8], ibs_sample[9]);
+ fail = fail || op_add_sample(cpu_buf, ibs_sample[10], ibs_sample[11]);
}

if (!fail)
@@ -376,7 +376,7 @@ void oprofile_add_trace(unsigned long pc)
if (pc == ESCAPE_CODE)
goto fail;

- if (add_sample(cpu_buf, pc, 0))
+ if (op_add_sample(cpu_buf, pc, 0))
goto fail;

return;
--
1.6.0.1

Subject: [PATCH 16/18] oprofile: rename variable ibs_allowed to has_ibs in op_model_amd.c

This patch renames ibs_allowed to has_ibs. Varible name fits better
now.

Signed-off-by: Robert Richter <[email protected]>
---
arch/x86/oprofile/op_model_amd.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
index c5b5c7f..423a954 100644
--- a/arch/x86/oprofile/op_model_amd.c
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -106,7 +106,7 @@ struct ibs_op_sample {
unsigned int ibs_dc_phys_high;
};

-static int ibs_allowed; /* AMD Family10h and later */
+static int has_ibs; /* AMD Family10h and later */

struct op_ibs_config {
unsigned long op_enabled;
@@ -201,7 +201,7 @@ op_amd_handle_ibs(struct pt_regs * const regs,
struct ibs_fetch_sample ibs_fetch;
struct ibs_op_sample ibs_op;

- if (!ibs_allowed)
+ if (!has_ibs)
return 1;

if (ibs_config.fetch_enabled) {
@@ -305,14 +305,14 @@ static void op_amd_start(struct op_msrs const * const msrs)
}

#ifdef CONFIG_OPROFILE_IBS
- if (ibs_allowed && ibs_config.fetch_enabled) {
+ if (has_ibs && ibs_config.fetch_enabled) {
low = (ibs_config.max_cnt_fetch >> 4) & 0xFFFF;
high = ((ibs_config.rand_en & 0x1) << 25) /* bit 57 */
+ IBS_FETCH_HIGH_ENABLE;
wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
}

- if (ibs_allowed && ibs_config.op_enabled) {
+ if (has_ibs && ibs_config.op_enabled) {
low = ((ibs_config.max_cnt_op >> 4) & 0xFFFF)
+ ((ibs_config.dispatched_ops & 0x1) << 19) /* bit 19 */
+ IBS_OP_LOW_ENABLE;
@@ -341,14 +341,14 @@ static void op_amd_stop(struct op_msrs const * const msrs)
}

#ifdef CONFIG_OPROFILE_IBS
- if (ibs_allowed && ibs_config.fetch_enabled) {
+ if (has_ibs && ibs_config.fetch_enabled) {
/* clear max count and enable */
low = 0;
high = 0;
wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
}

- if (ibs_allowed && ibs_config.op_enabled) {
+ if (has_ibs && ibs_config.op_enabled) {
/* clear max count and enable */
low = 0;
high = 0;
@@ -437,20 +437,20 @@ static int init_ibs_nmi(void)
/* uninitialize the APIC for the IBS interrupts if needed */
static void clear_ibs_nmi(void)
{
- if (ibs_allowed)
+ if (has_ibs)
on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1);
}

/* initialize the APIC for the IBS interrupts if available */
static void ibs_init(void)
{
- ibs_allowed = boot_cpu_has(X86_FEATURE_IBS);
+ has_ibs = boot_cpu_has(X86_FEATURE_IBS);

- if (!ibs_allowed)
+ if (!has_ibs)
return;

if (init_ibs_nmi()) {
- ibs_allowed = 0;
+ has_ibs = 0;
return;
}

@@ -459,7 +459,7 @@ static void ibs_init(void)

static void ibs_exit(void)
{
- if (!ibs_allowed)
+ if (!has_ibs)
return;

clear_ibs_nmi();
@@ -479,7 +479,7 @@ static int setup_ibs_files(struct super_block *sb, struct dentry *root)
if (ret)
return ret;

- if (!ibs_allowed)
+ if (!has_ibs)
return ret;

/* model specific files */
--
1.6.0.1

Subject: [PATCH 11/18] oprofile: simplify add_ibs_begin()

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/buffer_sync.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index 22cdb51..7415d2e 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -333,7 +333,7 @@ static void add_ibs_begin(int cpu, int code, struct mm_struct *mm)

sample = op_cpu_buffer_read_entry(cpu);
if (!sample)
- goto Error;
+ return;
rip = sample->eip;

#ifdef __LP64__
@@ -372,15 +372,12 @@ static void add_ibs_begin(int cpu, int code, struct mm_struct *mm)
for (i = 0; i < count; i++) {
sample = op_cpu_buffer_read_entry(cpu);
if (!sample)
- goto Error;
+ return;
add_event_entry(sample->eip);
add_event_entry(sample->event);
}

return;
-
-Error:
- return;
}

#endif
--
1.6.0.1

Subject: [PATCH 09/18] oprofile: simplify oprofile_begin_trace()

This patch removes the unused return parameter in
oprofile_begin_trace(). Also, oprofile_begin_trace() and
oprofile_end_trace() are inline now.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/oprofile/cpu_buffer.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index 9e66c38..435bd6e 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -265,14 +265,13 @@ fail:
return 0;
}

-static int oprofile_begin_trace(struct oprofile_cpu_buffer *cpu_buf)
+static inline void oprofile_begin_trace(struct oprofile_cpu_buffer *cpu_buf)
{
add_code(cpu_buf, CPU_TRACE_BEGIN);
cpu_buf->tracing = 1;
- return 1;
}

-static void oprofile_end_trace(struct oprofile_cpu_buffer *cpu_buf)
+static inline void oprofile_end_trace(struct oprofile_cpu_buffer *cpu_buf)
{
cpu_buf->tracing = 0;
}
@@ -288,8 +287,7 @@ __oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
return;
}

- if (!oprofile_begin_trace(cpu_buf))
- return;
+ oprofile_begin_trace(cpu_buf);

/*
* if log_sample() fail we can't backtrace since we lost the
@@ -297,6 +295,7 @@ __oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
*/
if (log_sample(cpu_buf, pc, is_kernel, event))
oprofile_ops.backtrace(regs, oprofile_backtrace_depth);
+
oprofile_end_trace(cpu_buf);
}

--
1.6.0.1

2009-01-08 12:10:28

by Ingo Molnar

[permalink] [raw]
Subject: Re: [0/18] oprofile: fixes and cleanup patches


* Robert Richter <[email protected]> wrote:

> This patch series contains some fixes and cleanups, mostly trivial. The
> patches are in preparation of the implementation of support for variable
> data size samples.

pulled, thanks Robert!

Ingo

2009-01-13 17:53:33

by Maynard Johnson

[permalink] [raw]
Subject: Re: [PATCH 01/18] oprofile: rename kernel-wide identifiers

Robert Richter wrote:
> This patch renames kernel-wide identifiers to something more oprofile
> specific names.
Are these oprofile-specific names going to be exported outside the oprofile kernel driver at some point? If not, I don't really see the need for these changes. Maybe I'm missing something . . .

-Maynard

>
> Cc: Andrew Morton <[email protected]>
> Signed-off-by: Robert Richter <[email protected]>
> ---
> drivers/oprofile/cpu_buffer.c | 12 ++++++------
> drivers/oprofile/event_buffer.c | 4 ++--
> drivers/oprofile/oprof.c | 4 ++--
> drivers/oprofile/oprof.h | 8 ++++----
> drivers/oprofile/oprofile_files.c | 27 ++++++++++++++-------------
> 5 files changed, 28 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
> index 6109096..fcf96f6 100644
> --- a/drivers/oprofile/cpu_buffer.c
> +++ b/drivers/oprofile/cpu_buffer.c
> @@ -66,7 +66,7 @@ void free_cpu_buffers(void)
>
> unsigned long oprofile_get_cpu_buffer_size(void)
> {
> - return fs_cpu_buffer_size;
> + return oprofile_cpu_buffer_size;
> }
>
> void oprofile_cpu_buffer_inc_smpl_lost(void)
> @@ -81,7 +81,7 @@ int alloc_cpu_buffers(void)
> {
> int i;
>
> - unsigned long buffer_size = fs_cpu_buffer_size;
> + unsigned long buffer_size = oprofile_cpu_buffer_size;
>
> op_ring_buffer_read = ring_buffer_alloc(buffer_size, OP_BUFFER_FLAGS);
> if (!op_ring_buffer_read)
> @@ -238,7 +238,7 @@ void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
> {
> struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(cpu_buffer);
>
> - if (!backtrace_depth) {
> + if (!oprofile_backtrace_depth) {
> log_sample(cpu_buf, pc, is_kernel, event);
> return;
> }
> @@ -251,7 +251,7 @@ void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
> * source of this event
> */
> if (log_sample(cpu_buf, pc, is_kernel, event))
> - oprofile_ops.backtrace(regs, backtrace_depth);
> + oprofile_ops.backtrace(regs, oprofile_backtrace_depth);
> oprofile_end_trace(cpu_buf);
> }
>
> @@ -308,8 +308,8 @@ void oprofile_add_ibs_sample(struct pt_regs * const regs,
> if (fail)
> goto fail;
>
> - if (backtrace_depth)
> - oprofile_ops.backtrace(regs, backtrace_depth);
> + if (oprofile_backtrace_depth)
> + oprofile_ops.backtrace(regs, oprofile_backtrace_depth);
>
> return;
>
> diff --git a/drivers/oprofile/event_buffer.c b/drivers/oprofile/event_buffer.c
> index 191a320..2b7ae36 100644
> --- a/drivers/oprofile/event_buffer.c
> +++ b/drivers/oprofile/event_buffer.c
> @@ -73,8 +73,8 @@ int alloc_event_buffer(void)
> unsigned long flags;
>
> spin_lock_irqsave(&oprofilefs_lock, flags);
> - buffer_size = fs_buffer_size;
> - buffer_watershed = fs_buffer_watershed;
> + buffer_size = oprofile_buffer_size;
> + buffer_watershed = oprofile_buffer_watershed;
> spin_unlock_irqrestore(&oprofilefs_lock, flags);
>
> if (buffer_watershed >= buffer_size)
> diff --git a/drivers/oprofile/oprof.c b/drivers/oprofile/oprof.c
> index cd37590..3cffce9 100644
> --- a/drivers/oprofile/oprof.c
> +++ b/drivers/oprofile/oprof.c
> @@ -23,7 +23,7 @@
> struct oprofile_operations oprofile_ops;
>
> unsigned long oprofile_started;
> -unsigned long backtrace_depth;
> +unsigned long oprofile_backtrace_depth;
> static unsigned long is_setup;
> static DEFINE_MUTEX(start_mutex);
>
> @@ -172,7 +172,7 @@ int oprofile_set_backtrace(unsigned long val)
> goto out;
> }
>
> - backtrace_depth = val;
> + oprofile_backtrace_depth = val;
>
> out:
> mutex_unlock(&start_mutex);
> diff --git a/drivers/oprofile/oprof.h b/drivers/oprofile/oprof.h
> index 5df0c21..c288d3c 100644
> --- a/drivers/oprofile/oprof.h
> +++ b/drivers/oprofile/oprof.h
> @@ -21,12 +21,12 @@ void oprofile_stop(void);
>
> struct oprofile_operations;
>
> -extern unsigned long fs_buffer_size;
> -extern unsigned long fs_cpu_buffer_size;
> -extern unsigned long fs_buffer_watershed;
> +extern unsigned long oprofile_buffer_size;
> +extern unsigned long oprofile_cpu_buffer_size;
> +extern unsigned long oprofile_buffer_watershed;
> extern struct oprofile_operations oprofile_ops;
> extern unsigned long oprofile_started;
> -extern unsigned long backtrace_depth;
> +extern unsigned long oprofile_backtrace_depth;
>
> struct super_block;
> struct dentry;
> diff --git a/drivers/oprofile/oprofile_files.c b/drivers/oprofile/oprofile_files.c
> index d820199..5d36ffc 100644
> --- a/drivers/oprofile/oprofile_files.c
> +++ b/drivers/oprofile/oprofile_files.c
> @@ -14,17 +14,18 @@
> #include "oprofile_stats.h"
> #include "oprof.h"
>
> -#define FS_BUFFER_SIZE_DEFAULT 131072
> -#define FS_CPU_BUFFER_SIZE_DEFAULT 8192
> -#define FS_BUFFER_WATERSHED_DEFAULT 32768 /* FIXME: tune */
> +#define BUFFER_SIZE_DEFAULT 131072
> +#define CPU_BUFFER_SIZE_DEFAULT 8192
> +#define BUFFER_WATERSHED_DEFAULT 32768 /* FIXME: tune */
>
> -unsigned long fs_buffer_size;
> -unsigned long fs_cpu_buffer_size;
> -unsigned long fs_buffer_watershed;
> +unsigned long oprofile_buffer_size;
> +unsigned long oprofile_cpu_buffer_size;
> +unsigned long oprofile_buffer_watershed;
>
> static ssize_t depth_read(struct file *file, char __user *buf, size_t count, loff_t *offset)
> {
> - return oprofilefs_ulong_to_user(backtrace_depth, buf, count, offset);
> + return oprofilefs_ulong_to_user(oprofile_backtrace_depth, buf, count,
> + offset);
> }
>
>
> @@ -125,16 +126,16 @@ 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;
> + oprofile_buffer_size = BUFFER_SIZE_DEFAULT;
> + oprofile_cpu_buffer_size = CPU_BUFFER_SIZE_DEFAULT;
> + oprofile_buffer_watershed = 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);
> - oprofilefs_create_ulong(sb, root, "buffer_size", &fs_buffer_size);
> - oprofilefs_create_ulong(sb, root, "buffer_watershed", &fs_buffer_watershed);
> - oprofilefs_create_ulong(sb, root, "cpu_buffer_size", &fs_cpu_buffer_size);
> + oprofilefs_create_ulong(sb, root, "buffer_size", &oprofile_buffer_size);
> + oprofilefs_create_ulong(sb, root, "buffer_watershed", &oprofile_buffer_watershed);
> + oprofilefs_create_ulong(sb, root, "cpu_buffer_size", &oprofile_cpu_buffer_size);
> oprofilefs_create_file(sb, root, "cpu_type", &cpu_type_fops);
> oprofilefs_create_file(sb, root, "backtrace_depth", &depth_fops);
> oprofilefs_create_file(sb, root, "pointer_size", &pointer_size_fops);

2009-01-13 18:01:57

by Maynard Johnson

[permalink] [raw]
Subject: Re: [PATCH 02/18] oprofile: rename cpu buffer functions

Robert Richter wrote:
> This patch renames cpu buffer functions to something more oprofile
> specific names. Functions will be moved to the global name space.
I'm not seeing where these functions are moved to the global name space, either in this set of 18 patches or the next set of 9 patches. Is that in some patch set that is yet to be posted?
>
> Cc: Andrew Morton <[email protected]>
> Signed-off-by: Robert Richter <[email protected]>
> ---
> drivers/oprofile/buffer_sync.c | 10 +++++-----
> drivers/oprofile/cpu_buffer.c | 4 ++--
> drivers/oprofile/cpu_buffer.h | 10 +++++-----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
> index 737bd94..d295d92 100644
> --- a/drivers/oprofile/buffer_sync.c
> +++ b/drivers/oprofile/buffer_sync.c
> @@ -331,7 +331,7 @@ static void add_ibs_begin(int cpu, int code, struct mm_struct *mm)
> off_t offset;
> struct op_sample *sample;
>
> - sample = cpu_buffer_read_entry(cpu);
> + sample = op_cpu_buffer_read_entry(cpu);
> if (!sample)
> goto Error;
> rip = sample->eip;
> @@ -370,7 +370,7 @@ static void add_ibs_begin(int cpu, int code, struct mm_struct *mm)
> count = IBS_OP_CODE_SIZE; /*IBS OP is 5 int64s*/
>
> for (i = 0; i < count; i++) {
> - sample = cpu_buffer_read_entry(cpu);
> + sample = op_cpu_buffer_read_entry(cpu);
> if (!sample)
> goto Error;
> add_event_entry(sample->eip);
> @@ -537,11 +537,11 @@ void sync_buffer(int cpu)
>
> add_cpu_switch(cpu);
>
> - cpu_buffer_reset(cpu);
> - available = cpu_buffer_entries(cpu);
> + op_cpu_buffer_reset(cpu);
> + available = op_cpu_buffer_entries(cpu);
>
> for (i = 0; i < available; ++i) {
> - struct op_sample *s = cpu_buffer_read_entry(cpu);
> + struct op_sample *s = op_cpu_buffer_read_entry(cpu);
> if (!s)
> break;
>
> diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
> index fcf96f6..e52c085 100644
> --- a/drivers/oprofile/cpu_buffer.c
> +++ b/drivers/oprofile/cpu_buffer.c
> @@ -152,14 +152,14 @@ add_sample(struct oprofile_cpu_buffer *cpu_buf,
> struct op_entry entry;
> int ret;
>
> - ret = cpu_buffer_write_entry(&entry);
> + ret = op_cpu_buffer_write_entry(&entry);
> if (ret)
> return ret;
>
> entry.sample->eip = pc;
> entry.sample->event = event;
>
> - ret = cpu_buffer_write_commit(&entry);
> + ret = op_cpu_buffer_write_commit(&entry);
> if (ret)
> return ret;
>
> diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h
> index aacb0f0..83d491e 100644
> --- a/drivers/oprofile/cpu_buffer.h
> +++ b/drivers/oprofile/cpu_buffer.h
> @@ -64,7 +64,7 @@ DECLARE_PER_CPU(struct oprofile_cpu_buffer, cpu_buffer);
> * 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)
> +static inline void op_cpu_buffer_reset(int cpu)
> {
> struct oprofile_cpu_buffer *cpu_buf = &per_cpu(cpu_buffer, cpu);
>
> @@ -72,7 +72,7 @@ static inline void cpu_buffer_reset(int cpu)
> cpu_buf->last_task = NULL;
> }
>
> -static inline int cpu_buffer_write_entry(struct op_entry *entry)
> +static inline int op_cpu_buffer_write_entry(struct op_entry *entry)
> {
> entry->event = ring_buffer_lock_reserve(op_ring_buffer_write,
> sizeof(struct op_sample),
> @@ -88,13 +88,13 @@ static inline int cpu_buffer_write_entry(struct op_entry *entry)
> return 0;
> }
>
> -static inline int cpu_buffer_write_commit(struct op_entry *entry)
> +static inline int op_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(int cpu)
> +static inline struct op_sample *op_cpu_buffer_read_entry(int cpu)
> {
> struct ring_buffer_event *e;
> e = ring_buffer_consume(op_ring_buffer_read, cpu, NULL);
> @@ -111,7 +111,7 @@ static inline struct op_sample *cpu_buffer_read_entry(int cpu)
> }
>
> /* "acquire" as many cpu buffer slots as we can */
> -static inline unsigned long cpu_buffer_entries(int cpu)
> +static inline unsigned long op_cpu_buffer_entries(int cpu)
> {
> return ring_buffer_entries_cpu(op_ring_buffer_read, cpu)
> + ring_buffer_entries_cpu(op_ring_buffer_write, cpu);

2009-01-13 19:51:11

by Maynard Johnson

[permalink] [raw]
Subject: Re: [0/18] oprofile: fixes and cleanup patches

Robert Richter wrote:
> This patch series contains some fixes and cleanups, mostly
> trivial. The patches are in preparation of the implementation of
> support for variable data size samples.
Other than the comments I made to patches 1 and 2, the rest of the patch set looks fine to me.

-Maynard
>
>
>
>
> ------------------------------------------------------------------------------
> Check out the new SourceForge.net Marketplace.
> It is the best place to buy or sell services for
> just about anything Open Source.
> http://p.sf.net/sfu/Xq1LFB
> _______________________________________________
> oprofile-list mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/oprofile-list

Subject: Re: [PATCH 01/18] oprofile: rename kernel-wide identifiers

On 13.01.09 11:53:59, Maynard Johnson wrote:
> Robert Richter wrote:
> > This patch renames kernel-wide identifiers to something more oprofile
> > specific names.
> Are these oprofile-specific names going to be exported outside the oprofile kernel driver at some point? If not, I don't really see the need for these changes. Maybe I'm missing something . . .

These names are visible as symbols when compiling into the
kernel. There would be a collision with any other non-static variable
or function in any other file.

$ nm .build/oprofile-x86_64-standard.obj/vmlinux | grep oprofile_cpu_buffer_size
ffffffff808bf428 B oprofile_cpu_buffer_size

-Robert

--
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]