Reworked Cell OProfile: SPU mutex lock fix

This is a reworked patch to fix the SPU data storage. Currently, the SPU escape sequences and program counter data is being added directly into the kernel buffer without holding the buffer_mutex lock. This patch changes how the data is stored. A new function, oprofile_add_value, is added into the oprofile driver to allow adding
generic data to the per cpu buffers. This enables a series of calls
to the oprofile_add_value to enter the needed SPU escape sequences and SPU program data into the kernel buffer via the per cpu buffers
without any additional processing. The oprofile_add_value function is
generic so it could be used by other architecures as well provided
the needed postprocessing was added to opreport.

Finally, this patch backs out the changes previously added to the oprofile generic code for handling the architecture specific ops.sync_start and ops.sync_stop that allowed the architecture to skip the per CPU buffer creation.

Signed-off-by Carl Love <[email protected]>

Index: Cell_kernel_4_15_2008/arch/powerpc/oprofile/cell/pr_util.h
===================================================================

arch/powerpc/oprofile/cell/pr_util.h +1 -6

@@ -20,11 +20,6 @@
 #include <asm/cell-regs.h>
 #include <asm/spu.h>
 
-/* Defines used for sync_start */
-#define SKIP_GENERIC_SYNC 0
-#define SYNC_START_ERROR -1
-#define DO_GENERIC_SYNC 1
-
 struct spu_overlay_info {	/* map of sections within an SPU overlay */
 	unsigned int vma;	/* SPU virtual memory address from elf */
 	unsigned int size;	/* size of section from elf */

@@ -85,7 +80,7 @@ void stop_spu_profiling(void);
 int spu_sync_start(void);
 
 /* remove the hooks */
-int spu_sync_stop(void);
+void spu_sync_stop(void);
 
 /* Record SPU program counter samples to the oprofile event buffer. */
 void spu_sync_buffer(int spu_num, unsigned int *samples,

Index: Cell_kernel_4_15_2008/arch/powerpc/oprofile/cell/spu_task_sync.c
===================================================================

arch/powerpc/oprofile/cell/spu_task_sync.c +1 -0

 int spu_prof_num_nodes;
 int last_guard_val[MAX_NUMNODES * 8];
+static int spu_ctx_sw_seen[MAX_NUMNODES * 8];
 

int spu_sync_start(void)
{

arch/powerpc/oprofile/cell/spu_task_sync.c +2 -2

 	int k;
-	int ret = SKIP_GENERIC_SYNC;
+	int ret = 0;
 	int register_ret;
-	unsigned long flags = 0;
+	int cpu;

arch/powerpc/oprofile/cell/spu_task_sync.c +10 -0

+
+	/* The SPU_PROFILING_CODE escape sequence must proceed
+	 * the SPU context switch info.
+	 */
+	for_each_online_cpu(cpu) {
+		oprofile_add_value(ESCAPE_CODE, cpu);
+		oprofile_add_value(SPU_PROFILING_CODE, cpu);
+		oprofile_add_value((unsigned long int)
+					    num_spu_nodes, cpu);
+	}

arch/powerpc/oprofile/cell/spu_task_sync.c +6 -2

+		ret = -1;
 		goto out;
 	}
 
-	for (k = 0; k < (MAX_NUMNODES * 8); k++)
+	for (k = 0; k < (MAX_NUMNODES * 8); k++) {
 		last_guard_val[k] = 0;
+		spu_ctx_sw_seen[k] = 0;
+	}
 	pr_debug("spu_sync_start -- running.\n");
 out:
 	return ret;
+
 }
 
 /* Record SPU program counter samples to the oprofile event buffer. */

@@ -432,7 +460,7 @@ void spu_sync_buffer(int spu_num, unsign
 
 	map = c_info->map;
 	the_spu = c_info->the_spu;
-	spin_lock(&buffer_lock);
+	spin_lock(&add_value_lock);
 	for (i = 0; i < num_samples; i++) {
 		unsigned int sample = *(samples+i);
 		int grd_val = 0;

arch/powerpc/oprofile/cell/spu_task_sync.c +2 -2

 	}
-	spin_unlock(&buffer_lock);
+	spin_unlock(&add_value_lock);
 out:
 	spin_unlock_irqrestore(&cache_lock, flags);
 }
 
 
-int spu_sync_stop(void)
+void spu_sync_stop(void)
 {

arch/powerpc/oprofile/cell/spu_task_sync.c +1 -1

 	pr_debug("spu_sync_stop -- done.\n");
-	return ret;
+	return;
 }
 

Index: Cell_kernel_4_15_2008/arch/powerpc/oprofile/op_model_cell.c
===================================================================

arch/powerpc/oprofile/op_model_cell.c +5 -5

@@ -1191,15 +1191,15 @@ static int cell_sync_start(void)
 	if (spu_cycle_reset)
 		return spu_sync_start();
 	else
-		return DO_GENERIC_SYNC;
+		return 0;
 }
 
-static int cell_sync_stop(void)
+static void cell_sync_stop(void)
 {
 	if (spu_cycle_reset)
-		return spu_sync_stop();
-	else
-		return 1;
+		spu_sync_stop();
+
+	return;
 }
 
 struct op_powerpc_model op_model_cell = {

Index: Cell_kernel_4_15_2008/drivers/oprofile/buffer_sync.c
===================================================================

drivers/oprofile/buffer_sync.c +1 -0

@@ -521,6 +521,20 @@ void sync_buffer(int cpu)
 			} else if (s->event == CPU_TRACE_BEGIN) {
 				state = sb_bt_start;
 				add_trace_begin();
+			} else if (s->event == VALUE_HEADER_ID) {

drivers/oprofile/buffer_sync.c +4 -0

+					add_event_entry(s->event);
+				else
+					printk("KERN_ERR Oprofile per CPU" \
+					       "buffer sequence error.\n");
 			} else {
 				struct mm_struct * oldmm = mm;

Index: Cell_kernel_4_15_2008/drivers/oprofile/cpu_buffer.c
===================================================================

drivers/oprofile/cpu_buffer.c +0 -0

@@ -224,6 +224,35 @@ static void oprofile_end_trace(struct op
 	cpu_buf->tracing = 0;
 }

include/linux/oprofile.h +13 -4

@@ -56,12 +56,10 @@ struct oprofile_operations {
 	/* Stop delivering interrupts. */
 	void (*stop)(void);
 	/* Arch-specific buffer sync functions.
-	 * Return value = 0:  Success
-	 * Return value = -1: Failure
-	 * Return value = 1:  Run generic sync function
+	 * Sync start: Return 0 for Success,  -1 for Failure
 	 */
 	int (*sync_start)(void);
-	int (*sync_stop)(void);
+	void (*sync_stop)(void);
 
 	/* Initiate a stack backtrace. Optional. */
 	void (*backtrace)(struct pt_regs * const regs, unsigned int depth);

@@ -106,6 +104,17 @@ void oprofile_add_sample(struct pt_regs 
 void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
 				unsigned long event, int is_kernel);
 
+/*
+ * Add a value to the per CPU buffer.  The value is passed from the per CPU
+ * buffer to the kernel buffer with no additional processing.  The assumption
+ * is any processing of the value will be done in the postprocessor.  This
+ * function should only be used for special architecture specific data.
+ * Currently only used by the CELL processor.
+ *
+ * This function does not perform a backtrace.
+ */
+void oprofile_add_value(unsigned long value, int cpu);
+
 /* Use this instead when the PC value is not from the regs. Doesn't
  * backtrace. */
 void oprofile_add_pc(unsigned long pc, int is_kernel, unsigned long event);

Index: Cell_kernel_4_15_2008/include/asm-powerpc/oprofile_impl.h
===================================================================

include/asm-powerpc/oprofile_impl.h +1 -2

@@ -48,7 +48,7 @@ struct op_powerpc_model {
 	void (*stop) (void);
 	void (*global_stop) (void);
 	int (*sync_start)(void);
-	int (*sync_stop)(void);
+	void (*sync_stop)(void);
 	void (*handle_interrupt) (struct pt_regs *,
 				  struct op_counter_config *);
 	int num_counters;
 
 
 
-------------------------------------------------------------------------

This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
oprofile-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/oprofile-list