[PATCH] Add callgraph support to Xenoprofile

[PATCH] Add callgraph support to Xenoprofile

[Amitabha Roy]

[-- Attachment #1: Type: text/plain, Size: 331 bytes --]

Hi

This patch implements backtrace support in Xenoprofile. It's been
reviewed by Renato. I've tested it out successfully on xen x86_32 and
x86_64. I've added stubs for the backtrace for ia64 to ensure that it
compiles ok but havent been able to test that out. However, I believe
there shouldn't be any problems.

Thanks
-Amitabha

[-- Attachment #2: xenoprof_backtrace_patch.txt --]
[-- Type: text/plain, Size: 17804 bytes --]

# HG changeset patch
# User root@localhost.localdomain
# Date 1181186537 -19800
# Node ID 1d47ef825eb4f53fdb6d686ddf4d2ae9e47a70a0
# Parent  bd3d6b4c52ec809f080c89c4ffcf61dc6e445978
Add backtrace support to xenoprof

Signed-off-by: Amitabha Roy <amitabha.roy@gmail.com>
Reviewed-by: Jose Renato G Santos <joserenato.santos@hp.com>

diff -r bd3d6b4c52ec -r 1d47ef825eb4 Config.mk
--- a/Config.mk	Fri Jun 01 14:50:52 2007 +0100
+++ b/Config.mk	Thu Jun 07 08:52:17 2007 +0530
@@ -12,13 +12,19 @@ XEN_TARGET_X86_PAE  ?= y
 XEN_TARGET_X86_PAE  ?= y
 endif
 
+ifneq ($(FRAME_POINTER),y)
+CONFIG_FRAME_POINTER ?= -fomit-frame-pointer
+else
+CONFIG_FRAME_POINTER ?= -DCONFIG_FRAME_POINTER -fno-omit-frame-pointer
+endif 
+
 CONFIG_$(XEN_OS) := y
 
 SHELL     ?= /bin/sh
 
 # Tools to run on system hosting the build
 HOSTCC     = gcc
-HOSTCFLAGS = -Wall -Werror -Wstrict-prototypes -O2 -fomit-frame-pointer
+HOSTCFLAGS = -Wall -Werror -Wstrict-prototypes -O2 $(CONFIG_FRAME_POINTER)
 
 DISTDIR     ?= $(XEN_ROOT)/dist
 DESTDIR     ?= /
diff -r bd3d6b4c52ec -r 1d47ef825eb4 config/StdGNU.mk
--- a/config/StdGNU.mk	Fri Jun 01 14:50:52 2007 +0100
+++ b/config/StdGNU.mk	Thu Jun 07 08:52:17 2007 +0530
@@ -26,8 +26,8 @@ SHLIB_CFLAGS = -shared
 
 ifneq ($(debug),y)
 # Optimisation flags are overridable
-CFLAGS ?= -O2 -fomit-frame-pointer
+CFLAGS ?= -O2 $(CONFIG_FRAME_POINTER)
 else
 # Less than -O1 produces bad code and large stack frames
-CFLAGS ?= -O1 -fno-omit-frame-pointer
+CFLAGS ?= -O1 $(CONFIG_FRAME_POINTER)
 endif
diff -r bd3d6b4c52ec -r 1d47ef825eb4 linux-2.6-xen-sparse/drivers/xen/xenoprof/xenoprofile.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenoprof/xenoprofile.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/xenoprof/xenoprofile.c	Thu Jun 07 08:52:17 2007 +0530
@@ -51,6 +51,8 @@ static int xenoprof_is_primary = 0;
 static int xenoprof_is_primary = 0;
 static int active_defined;
 
+extern unsigned long backtrace_depth;
+
 /* Number of buffers in shared area (one per VCPU) */
 int nbuf;
 /* Mappings of VIRQ_XENOPROF to irq number (per cpu) */
@@ -115,38 +117,57 @@ unsigned int pdomains;
 unsigned int pdomains;
 struct xenoprof_passive passive_domains[MAX_OPROF_DOMAINS];
 
+
+/* Check whether the given entry is an escape code */
+static int xenoprof_is_escape(xenoprof_buf_t * buf, int tail){
+    return (buf->event_log[tail].eip == XENOPROF_ESCAPE_CODE);
+}
+
+/* Get the event at the given entry  */
+static uint8_t xenoprof_get_event(xenoprof_buf_t * buf, int tail){
+    return (buf->event_log[tail].event);
+}
+
+
+
+
 static void xenoprof_add_pc(xenoprof_buf_t *buf, int is_passive)
 {
 	int head, tail, size;
+	int tracing = 0;
 
 	head = buf->event_head;
 	tail = buf->event_tail;
 	size = buf->event_size;
 
-	if (tail > head) {
-		while (tail < size) {
-			oprofile_add_pc(buf->event_log[tail].eip,
-					buf->event_log[tail].mode,
-					buf->event_log[tail].event);
+	while(tail != head) {
+		if(xenoprof_is_escape(buf, tail) && 
+		   xenoprof_get_event(buf, tail)==XENOPROF_TRACE_BEGIN){
+			tracing=1;
+			oprofile_add_pc(ESCAPE_CODE, buf->event_log[tail].mode, 
+					CPU_TRACE_BEGIN); 
 			if (!is_passive)
 				oprofile_samples++;
 			else
 				p_oprofile_samples++;
-			tail++;
-		}
-		tail = 0;
-	}
-	while (tail < head) {
-		oprofile_add_pc(buf->event_log[tail].eip,
-				buf->event_log[tail].mode,
-				buf->event_log[tail].event);
-		if (!is_passive)
-			oprofile_samples++;
-		else
-			p_oprofile_samples++;
+			
+		}
+		else{
+			oprofile_add_pc(buf->event_log[tail].eip,
+					buf->event_log[tail].mode,
+					buf->event_log[tail].event);
+			if(!tracing){
+				if (!is_passive)
+					oprofile_samples++;
+				else
+					p_oprofile_samples++;
+			}
+       
+		}
 		tail++;
-	}
-
+		if(tail==size)
+		    tail=0;
+	}
 	buf->event_tail = tail;
 }
 
@@ -294,9 +315,16 @@ static int xenoprof_setup(void)
 			active_defined = 1;
 		}
 
+		if(backtrace_depth > 0){
+		    ret = HYPERVISOR_xenoprof_op(XENOPROF_set_backtrace, 
+						 &backtrace_depth);
+		    if (ret)
+			backtrace_depth = 0;
+		}
 		ret = HYPERVISOR_xenoprof_op(XENOPROF_reserve_counters, NULL);
 		if (ret)
 			goto err;
+		
 		xenoprof_arch_counter();
 		ret = HYPERVISOR_xenoprof_op(XENOPROF_setup_events, NULL);
 
@@ -438,8 +466,21 @@ out:
 	for (j = 0; j < i; j++)
 		xenoprof_arch_unmap_shared_buffer(&p_shared_buffer[i]);
 
- 	return ret;
-}
+	return ret;
+}
+
+
+/* The dummy backtrace function to keep oprofile happy
+ * The real backtrace is done in xen
+ */
+static void xenoprof_dummy_backtrace(struct pt_regs * const regs, 
+				     unsigned int depth){
+	/* this should never be called */
+	BUG();
+	return;
+}
+
+
 
 struct oprofile_operations xenoprof_ops = {
 #ifdef HAVE_XENOPROF_CREATE_FILES
@@ -450,7 +491,8 @@ struct oprofile_operations xenoprof_ops 
 	.setup 		= xenoprof_setup,
 	.shutdown	= xenoprof_shutdown,
 	.start		= xenoprof_start,
-	.stop		= xenoprof_stop
+	.stop		= xenoprof_stop,
+	.backtrace	= xenoprof_dummy_backtrace
 };
 
 
diff -r bd3d6b4c52ec -r 1d47ef825eb4 xen/arch/ia64/xen/oprofile/perfmon.c
--- a/xen/arch/ia64/xen/oprofile/perfmon.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/arch/ia64/xen/oprofile/perfmon.c	Thu Jun 07 08:52:17 2007 +0530
@@ -37,7 +37,7 @@
 #include <asm/ptrace.h>
 
 // XXX move them to an appropriate header file
-extern void xenoprof_log_event(struct vcpu *vcpu,
+extern void xenoprof_log_event(struct vcpu *vcpu, struct pt_regs * regs,
                                unsigned long eip, int mode, int event);
 extern int is_active(struct domain *d);
 
@@ -55,7 +55,10 @@ xenoprof_handler(struct task_struct *tas
     if (!allow_virq || !allow_ints)
         return 0;
 
-    xenoprof_log_event(current, ip, xenoprofile_get_mode(task, regs), event);
+    // Note that log event actually expect cpu_user_regs, cast back 
+    // appropriately when doing the backtrace implementation in ia64
+    xenoprof_log_event(current, regs, ip, xenoprofile_get_mode(task, regs), 
+					   event);
     
     // send VIRQ_XENOPROF
     if (is_active(current->domain) && !ring_0(regs))
diff -r bd3d6b4c52ec -r 1d47ef825eb4 xen/arch/x86/oprofile/Makefile
--- a/xen/arch/x86/oprofile/Makefile	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/arch/x86/oprofile/Makefile	Thu Jun 07 08:52:17 2007 +0530
@@ -3,3 +3,4 @@ obj-y += op_model_p4.o
 obj-y += op_model_p4.o
 obj-y += op_model_ppro.o
 obj-y += op_model_athlon.o
+obj-y += backtrace.o
diff -r bd3d6b4c52ec -r 1d47ef825eb4 xen/arch/x86/oprofile/op_model_athlon.c
--- a/xen/arch/x86/oprofile/op_model_athlon.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/arch/x86/oprofile/op_model_athlon.c	Thu Jun 07 08:52:17 2007 +0530
@@ -43,8 +43,8 @@
 
 static unsigned long reset_value[NUM_COUNTERS];
 
-extern void xenoprof_log_event(struct vcpu *v, unsigned long eip,
-			       int mode, int event);
+extern void xenoprof_log_event(struct vcpu *v, struct cpu_user_regs * regs, 
+			       unsigned long eip, int mode, int event);
 extern int xenoprofile_get_mode(struct vcpu *v,
 				struct cpu_user_regs * const regs);
 
@@ -130,7 +130,7 @@ static int athlon_check_ctrs(unsigned in
 	for (i = 0 ; i < NUM_COUNTERS; ++i) {
 		CTR_READ(low, high, msrs, i);
 		if (CTR_OVERFLOWED(low)) {
-			xenoprof_log_event(current, eip, mode, i);
+			xenoprof_log_event(current, regs, eip, mode, i);
 			CTR_WRITE(reset_value[i], msrs, i);
 			ovf = 1;
 		}
diff -r bd3d6b4c52ec -r 1d47ef825eb4 xen/arch/x86/oprofile/op_model_p4.c
--- a/xen/arch/x86/oprofile/op_model_p4.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/arch/x86/oprofile/op_model_p4.c	Thu Jun 07 08:52:17 2007 +0530
@@ -620,8 +620,8 @@ static void p4_setup_ctrs(struct op_msrs
 	}
 }
 
-extern void xenoprof_log_event(struct vcpu *v, unsigned long eip,
-			       int mode, int event);
+extern void xenoprof_log_event(struct vcpu *v, struct cpu_user_regs * regs, 
+			       unsigned long eip, int mode, int event);
 extern int xenoprofile_get_mode(struct vcpu *v,
 				struct cpu_user_regs * const regs);
 
@@ -664,8 +664,8 @@ static int p4_check_ctrs(unsigned int co
 		CCCR_READ(low, high, real);
  		CTR_READ(ctr, high, real);
 		if (CCCR_OVF_P(low) || CTR_OVERFLOW_P(ctr)) {
-			xenoprof_log_event(current, eip, mode, i);
- 			CTR_WRITE(reset_value[i], real);
+			xenoprof_log_event(current, regs, eip, mode, i);
+			CTR_WRITE(reset_value[i], real);
 			CCCR_CLEAR_OVF(low);
 			CCCR_WRITE(low, high, real);
  			CTR_WRITE(reset_value[i], real);
diff -r bd3d6b4c52ec -r 1d47ef825eb4 xen/arch/x86/oprofile/op_model_ppro.c
--- a/xen/arch/x86/oprofile/op_model_ppro.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/arch/x86/oprofile/op_model_ppro.c	Thu Jun 07 08:52:17 2007 +0530
@@ -88,8 +88,8 @@ static void ppro_setup_ctrs(struct op_ms
 	}
 }
 
-extern void xenoprof_log_event(struct vcpu *v, unsigned long eip,
-			       int mode, int event);
+extern void xenoprof_log_event(struct vcpu *v, struct cpu_user_regs * regs, 
+			       unsigned long eip, int mode, int event);
 extern int xenoprofile_get_mode(struct vcpu *v,
 				struct cpu_user_regs * const regs);
  
@@ -106,7 +106,7 @@ static int ppro_check_ctrs(unsigned int 
 	for (i = 0 ; i < NUM_COUNTERS; ++i) {
 		CTR_READ(low, high, msrs, i);
 		if (CTR_OVERFLOWED(low)) {
-			xenoprof_log_event(current, eip, mode, i);
+			xenoprof_log_event(current, regs, eip, mode, i);
 			CTR_WRITE(reset_value[i], msrs, i);
 			ovf = 1;
 		}
diff -r bd3d6b4c52ec -r 1d47ef825eb4 xen/common/xenoprof.c
--- a/xen/common/xenoprof.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/common/xenoprof.c	Thu Jun 07 08:52:17 2007 +0530
@@ -31,6 +31,7 @@ unsigned int activated;
 unsigned int activated;
 struct domain *xenoprof_primary_profiler;
 int xenoprof_state = XENOPROF_IDLE;
+static unsigned long backtrace_depth=0;
 
 u64 total_samples;
 u64 invalid_buffer_samples;
@@ -414,55 +415,47 @@ static int add_passive_list(XEN_GUEST_HA
     return ret;
 }
 
-void xenoprof_log_event(
-    struct vcpu *vcpu, unsigned long eip, int mode, int event)
-{
-    struct domain *d = vcpu->domain;
-    struct xenoprof_vcpu *v;
-    xenoprof_buf_t *buf;
+
+/* Get space in the buffer */
+static int xenoprof_buf_space(struct domain *d, xenoprof_buf_t * buf, int size)
+{
+    int head;
+    int tail;
+
+    head = xenoprof_buf(d, buf, event_head);
+    tail = xenoprof_buf(d, buf, event_tail);
+
+    if(tail > head){
+        return ((tail - head) - 1);
+    }
+    else {
+        return (size - (head -tail) -1);
+    }
+}
+
+/* check for space and add a sample
+ * return 1 if successful, 0 otherwise
+ */
+
+inline int xenoprof_add_sample(struct domain *d, xenoprof_buf_t *buf, 
+                               unsigned long eip, int mode, int event)
+{
     int head;
     int tail;
     int size;
 
-
-    total_samples++;
-
-    /* ignore samples of un-monitored domains */
-    /* Count samples in idle separate from other unmonitored domains */
-    if ( !is_profiled(d) )
-    {
-        others_samples++;
-        return;
-    }
-
-    v = &d->xenoprof->vcpu[vcpu->vcpu_id];
-
-    /* Sanity check. Should never happen */ 
-    if ( v->buffer == NULL )
-    {
-        invalid_buffer_samples++;
-        return;
-    }
-
-    buf = v->buffer;
-
     head = xenoprof_buf(d, buf, event_head);
     tail = xenoprof_buf(d, buf, event_tail);
-    size = v->event_size;
-
+    size = xenoprof_buf(d, buf, event_size);
+    
     /* make sure indexes in shared buffer are sane */
     if ( (head < 0) || (head >= size) || (tail < 0) || (tail >= size) )
     {
         corrupted_buffer_samples++;
-        return;
-    }
-
-    if ( (head == tail - 1) || (head == size - 1 && tail == 0) )
-    {
-        xenoprof_buf(d, buf, lost_samples)++;
-        lost_samples++;
-    }
-    else
+        return 0;
+    }
+
+    if(xenoprof_buf_space(d, buf, size) > 0)
     {
         xenoprof_buf(d, buf, event_log[head].eip) = eip;
         xenoprof_buf(d, buf, event_log[head].mode) = mode;
@@ -470,7 +463,87 @@ void xenoprof_log_event(
         head++;
         if ( head >= size )
             head = 0;
+        
         xenoprof_buf(d, buf, event_head) = head;
+    }
+    else
+    {
+        xenoprof_buf(d, buf, lost_samples)++;
+        lost_samples++;
+        return 0;
+    }
+    return 1;
+}
+
+/* add a trace to the buffer */
+int xenoprof_add_trace(struct domain *d, struct vcpu *vcpu,
+                       unsigned long eip, int mode)
+{
+    xenoprof_buf_t * buf;
+    
+    /* Assume buffer is not null, it should have been checked before entering
+     * the arch specific backtrace code
+     */
+    
+    buf = (d->xenoprof->vcpu[vcpu->vcpu_id]).buffer;    
+
+    /* Ensure we do not accidentally write an escape code due to 
+     * a broken frame
+     */
+
+    if(eip==XENOPROF_ESCAPE_CODE)
+    {
+        invalid_buffer_samples++;
+        return 0;
+    }
+
+    return xenoprof_add_sample(d, buf, eip, mode, 0);
+}
+
+void xenoprof_log_event(
+    struct vcpu *vcpu, struct cpu_user_regs * regs, unsigned long eip, int mode,
+    int event)
+{
+    struct domain *d = vcpu->domain;
+    struct xenoprof_vcpu *v;
+    xenoprof_buf_t *buf;
+
+    total_samples++;
+
+    /* ignore samples of un-monitored domains */
+    /* Count samples in idle separate from other unmonitored domains */
+    if ( !is_profiled(d) )
+    {
+        others_samples++;
+        return;
+    }
+
+    v = &d->xenoprof->vcpu[vcpu->vcpu_id];
+    
+    if(v->buffer == NULL)
+    {
+        invalid_buffer_samples++;
+        return;
+    }
+    
+    buf = v->buffer;
+
+    /* Backtrace if asked for */
+    if(backtrace_depth > 0)
+    {
+        if(xenoprof_buf_space(d, buf, v->event_size) < 2){
+            xenoprof_buf(d, buf, lost_samples)++;
+            lost_samples++;
+            return;
+        }       
+        if(!xenoprof_add_sample(d, buf, XENOPROF_ESCAPE_CODE, mode, 
+		   XENOPROF_TRACE_BEGIN))
+        { /* could happen due to a corrupted buffer */
+            return;
+        }
+    }
+    if(xenoprof_add_sample(d, buf, eip, mode, event))
+    {
         if ( is_active(vcpu->domain) )
             active_samples++;
         else
@@ -481,8 +554,15 @@ void xenoprof_log_event(
             xenoprof_buf(d, buf, kernel_samples)++;
         else
             xenoprof_buf(d, buf, xen_samples)++;
-    }
-}
+    
+    }
+    if(backtrace_depth > 0)
+    {
+        backtrace(d, vcpu, regs, backtrace_depth, mode);
+    }
+}
+
+
 
 static int xenoprof_op_init(XEN_GUEST_HANDLE(void) arg)
 {
@@ -729,7 +809,18 @@ int do_xenoprof_op(int op, XEN_GUEST_HAN
             activated = 0;
             adomains=0;
             xenoprof_primary_profiler = NULL;
+            backtrace_depth=0;
             ret = 0;
+        }
+        break;
+                
+    case XENOPROF_set_backtrace:
+        ret = 0;
+        if(!arch_backtrace_supported()){
+            ret = -EINVAL;
+        }
+        else if(copy_from_guest(&backtrace_depth, arg, 1)){
+            ret=-EFAULT;
         }
         break;
 
diff -r bd3d6b4c52ec -r 1d47ef825eb4 xen/include/asm-ia64/xenoprof.h
--- a/xen/include/asm-ia64/xenoprof.h	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/include/asm-ia64/xenoprof.h	Thu Jun 07 08:52:17 2007 +0530
@@ -37,7 +37,16 @@ struct vcpu;
 struct vcpu;
 struct cpu_user_regs;
 int xenoprofile_get_mode(struct vcpu *v, struct cpu_user_regs * const regs);
-
+static inline int arch_backtrace_supported(void)
+{
+	return 0;
+}
+static inline void backtrace(struct domain *d, struct vcpu *vcpu, 
+                      struct pt_regs *const regs, unsigned long depth, int mode)
+{
+    /* To be implemented */
+    return;
+}
 #define xenoprof_shared_gmfn(d, gmaddr, maddr)  \
     assign_domain_page((d), (gmaddr), (maddr));
 
diff -r bd3d6b4c52ec -r 1d47ef825eb4 xen/include/asm-x86/xenoprof.h
--- a/xen/include/asm-x86/xenoprof.h	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/include/asm-x86/xenoprof.h	Thu Jun 07 08:52:17 2007 +0530
@@ -47,6 +47,12 @@ struct vcpu;
 struct vcpu;
 struct cpu_user_regs;
 int xenoprofile_get_mode(struct vcpu *v, struct cpu_user_regs * const regs);
+static inline int arch_backtrace_supported(void)
+{
+	return 1;
+}
+void backtrace(struct domain *d, struct vcpu *vcpu, 
+               struct cpu_user_regs *const regs, unsigned long depth, int mode);
 #define xenoprof_shared_gmfn(d, gmaddr, maddr)                      \
     do {                                                            \
         (void)(maddr);                                              \
diff -r bd3d6b4c52ec -r 1d47ef825eb4 xen/include/public/xenoprof.h
--- a/xen/include/public/xenoprof.h	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/include/public/xenoprof.h	Thu Jun 07 08:52:17 2007 +0530
@@ -49,7 +49,8 @@
 #define XENOPROF_release_counters   12
 #define XENOPROF_shutdown           13
 #define XENOPROF_get_buffer         14
-#define XENOPROF_last_op            14
+#define XENOPROF_set_backtrace      15
+#define XENOPROF_last_op            15
 
 #define MAX_OPROF_EVENTS    32
 #define MAX_OPROF_DOMAINS   25
@@ -61,6 +62,11 @@ struct event_log {
     uint8_t mode;
     uint8_t event;
 };
+
+/* PC value that indicates a special code */
+#define XENOPROF_ESCAPE_CODE ~0UL
+/* Transient events for the xenoprof->oprofile cpu buf */
+#define XENOPROF_TRACE_BEGIN 1
 
 /* Xenoprof buffer shared between Xen and domain - 1 per VCPU */
 struct xenoprof_buf {
diff -r bd3d6b4c52ec -r 1d47ef825eb4 xen/include/xen/xenoprof.h
--- a/xen/include/xen/xenoprof.h	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/include/xen/xenoprof.h	Thu Jun 07 08:52:17 2007 +0530
@@ -66,6 +66,8 @@ void free_xenoprof_pages(struct domain *
 void free_xenoprof_pages(struct domain *d);
 
 int do_xenoprof_op(int op, XEN_GUEST_HANDLE(void) arg);
+int xenoprof_add_trace(struct domain *d, struct vcpu *v, 
+                       unsigned long eip, int mode);
 
 extern struct domain *xenoprof_primary_profiler;
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] Add callgraph support to Xenoprofile

[Keir Fraser]

On 7/6/07 04:34, "Amitabha Roy" <amitabha.roy@gmail.com> wrote:

> This patch implements backtrace support in Xenoprofile. It's been
> reviewed by Renato. I've tested it out successfully on xen x86_32 and
> x86_64. I've added stubs for the backtrace for ia64 to ensure that it
> compiles ok but havent been able to test that out. However, I believe
> there shouldn't be any problems.

Coding style is all over the shop. Where does CONFIG_FRAME_POINTER get used?
I'm not sure that FRAME_POINTER needs to be a top-level option -- it's only
necessary for Xen build isn't it?

 -- Keir

Re: [PATCH] Add callgraph support to Xenoprofile

[Amitabha Roy]

Hi Keir

Coding style is likely because I've preserved the formatting of the
original files (esp. of backtrace.c from Linux for simple diffs).
There isnt a consistent style across the files that I could follow.

CONFIG_FRAME_POINTER gets used in config/StdGNU.mk.
I simply followed whatever was done for XEN_TARGET_X86_PAE. However
unlike the PAE case I haven't ensured that frame pointers are turned
on both in the kernel and Xen. If you want I can move all the frame
pointer related stuff into xen/Rules.mk.

-Amitabha


On 6/7/07, Keir Fraser <keir@xensource.com> wrote:
> On 7/6/07 04:34, "Amitabha Roy" <amitabha.roy@gmail.com> wrote:
>
> > This patch implements backtrace support in Xenoprofile. It's been
> > reviewed by Renato. I've tested it out successfully on xen x86_32 and
> > x86_64. I've added stubs for the backtrace for ia64 to ensure that it
> > compiles ok but havent been able to test that out. However, I believe
> > there shouldn't be any problems.
>
> Coding style is all over the shop. Where does CONFIG_FRAME_POINTER get used?
> I'm not sure that FRAME_POINTER needs to be a top-level option -- it's only
> necessary for Xen build isn't it?
>
>  -- Keir
>
>

Re: [PATCH] Add callgraph support to Xenoprofile

[Keir Fraser]

On 7/6/07 10:20, "Amitabha Roy" <amitabha.roy@gmail.com> wrote:

> Coding style is likely because I've preserved the formatting of the
> original files (esp. of backtrace.c from Linux for simple diffs).
> There isnt a consistent style across the files that I could follow.

Linux-derived files follow Linux style (this will apply to many of the files
under arch/x86/oprofile). Xen-specific files follow Xen style as in e.g.,
page_alloc.c, xmalloc.c, xenoprof.c).

If any of the files you edit don't follow this, please fix them first, and
send a separate patch as a prerequisite for your own.

> CONFIG_FRAME_POINTER gets used in config/StdGNU.mk.
> I simply followed whatever was done for XEN_TARGET_X86_PAE. However
> unlike the PAE case I haven't ensured that frame pointers are turned
> on both in the kernel and Xen. If you want I can move all the frame
> pointer related stuff into xen/Rules.mk.

Yes, please. Call the option frame_pointer, in line with other Xen-specific
options which are also lower case.

 -- Keir

Re: [PATCH] Add callgraph support to Xenoprofile

[Amitabha Roy]

[-- Attachment #1: Type: text/plain, Size: 1644 bytes --]

I've moved the frame pointer code into the xen specific makefile.
I've also fixed the per file inconsistencies. I noticed that some
files already have a few inconsistencies within themselves in places
other than my code.

Perhaps you could accept this patch and I can send a separate one
later fixing those ?
One of the suggestions I had from Renato was to minimize any diffs of
files based off linux versions so that pulling in updates will be
easier. So I can look into those on a case by case basis with him.

-Amitabha


On 6/7/07, Keir Fraser <keir@xensource.com> wrote:
> On 7/6/07 10:20, "Amitabha Roy" <amitabha.roy@gmail.com> wrote:
>
> > Coding style is likely because I've preserved the formatting of the
> > original files (esp. of backtrace.c from Linux for simple diffs).
> > There isnt a consistent style across the files that I could follow.
>
> Linux-derived files follow Linux style (this will apply to many of the files
> under arch/x86/oprofile). Xen-specific files follow Xen style as in e.g.,
> page_alloc.c, xmalloc.c, xenoprof.c).
>
> If any of the files you edit don't follow this, please fix them first, and
> send a separate patch as a prerequisite for your own.
>
> > CONFIG_FRAME_POINTER gets used in config/StdGNU.mk.
> > I simply followed whatever was done for XEN_TARGET_X86_PAE. However
> > unlike the PAE case I haven't ensured that frame pointers are turned
> > on both in the kernel and Xen. If you want I can move all the frame
> > pointer related stuff into xen/Rules.mk.
>
> Yes, please. Call the option frame_pointer, in line with other Xen-specific
> options which are also lower case.
>
>  -- Keir
>
>

[-- Attachment #2: xenoprof_backtrace_patch.txt --]
[-- Type: text/plain, Size: 21712 bytes --]

# HG changeset patch
# User root@localhost.localdomain
# Date 1181216195 -19800
# Node ID e6f484a96f435fd882de142797f6b309928a1ee9
# Parent  bd3d6b4c52ec809f080c89c4ffcf61dc6e445978
Add backtrace support to xenoprof

Signed-off-by: Amitabha Roy <amitabha.roy@gmail.com>
Reviewed-by: Jose Renato G Santos <joserenato.santos@hp.com>

diff -r bd3d6b4c52ec -r e6f484a96f43 linux-2.6-xen-sparse/drivers/xen/xenoprof/xenoprofile.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenoprof/xenoprofile.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/xenoprof/xenoprofile.c	Thu Jun 07 17:06:35 2007 +0530
@@ -51,6 +51,8 @@ static int xenoprof_is_primary = 0;
 static int xenoprof_is_primary = 0;
 static int active_defined;
 
+extern unsigned long backtrace_depth;
+
 /* Number of buffers in shared area (one per VCPU) */
 int nbuf;
 /* Mappings of VIRQ_XENOPROF to irq number (per cpu) */
@@ -115,38 +117,59 @@ unsigned int pdomains;
 unsigned int pdomains;
 struct xenoprof_passive passive_domains[MAX_OPROF_DOMAINS];
 
+
+/* Check whether the given entry is an escape code */
+static int xenoprof_is_escape(xenoprof_buf_t * buf, int tail)
+{
+	return (buf->event_log[tail].eip == XENOPROF_ESCAPE_CODE);
+}
+
+/* Get the event at the given entry  */
+static uint8_t xenoprof_get_event(xenoprof_buf_t * buf, int tail)
+{
+	return (buf->event_log[tail].event);
+}
+
+
+
+
 static void xenoprof_add_pc(xenoprof_buf_t *buf, int is_passive)
 {
 	int head, tail, size;
+	int tracing = 0;
 
 	head = buf->event_head;
 	tail = buf->event_tail;
 	size = buf->event_size;
 
-	if (tail > head) {
-		while (tail < size) {
-			oprofile_add_pc(buf->event_log[tail].eip,
-					buf->event_log[tail].mode,
-					buf->event_log[tail].event);
+	while(tail != head) {
+		if(xenoprof_is_escape(buf, tail) && 
+		   xenoprof_get_event(buf, tail)==XENOPROF_TRACE_BEGIN){
+			tracing=1;
+			oprofile_add_pc(ESCAPE_CODE, buf->event_log[tail].mode, 
+					CPU_TRACE_BEGIN); 
 			if (!is_passive)
 				oprofile_samples++;
 			else
 				p_oprofile_samples++;
-			tail++;
-		}
-		tail = 0;
-	}
-	while (tail < head) {
-		oprofile_add_pc(buf->event_log[tail].eip,
-				buf->event_log[tail].mode,
-				buf->event_log[tail].event);
-		if (!is_passive)
-			oprofile_samples++;
-		else
-			p_oprofile_samples++;
+			
+		}
+		else{
+			oprofile_add_pc(buf->event_log[tail].eip,
+					buf->event_log[tail].mode,
+					buf->event_log[tail].event);
+			if(!tracing){
+				if (!is_passive)
+					oprofile_samples++;
+				else
+					p_oprofile_samples++;
+			}
+       
+		}
 		tail++;
-	}
-
+		if(tail==size)
+		    tail=0;
+	}
 	buf->event_tail = tail;
 }
 
@@ -294,9 +317,16 @@ static int xenoprof_setup(void)
 			active_defined = 1;
 		}
 
+		if(backtrace_depth > 0){
+			ret = HYPERVISOR_xenoprof_op(XENOPROF_set_backtrace, 
+						 &backtrace_depth);
+			if (ret)
+			backtrace_depth = 0;
+		}
 		ret = HYPERVISOR_xenoprof_op(XENOPROF_reserve_counters, NULL);
 		if (ret)
 			goto err;
+		
 		xenoprof_arch_counter();
 		ret = HYPERVISOR_xenoprof_op(XENOPROF_setup_events, NULL);
 
@@ -438,8 +468,22 @@ out:
 	for (j = 0; j < i; j++)
 		xenoprof_arch_unmap_shared_buffer(&p_shared_buffer[i]);
 
- 	return ret;
-}
+	return ret;
+}
+
+
+/* The dummy backtrace function to keep oprofile happy
+ * The real backtrace is done in xen
+ */
+static void xenoprof_dummy_backtrace(struct pt_regs * const regs, 
+				     unsigned int depth)
+{
+	/* this should never be called */
+	BUG();
+	return;
+}
+
+
 
 struct oprofile_operations xenoprof_ops = {
 #ifdef HAVE_XENOPROF_CREATE_FILES
@@ -450,7 +494,8 @@ struct oprofile_operations xenoprof_ops 
 	.setup 		= xenoprof_setup,
 	.shutdown	= xenoprof_shutdown,
 	.start		= xenoprof_start,
-	.stop		= xenoprof_stop
+	.stop		= xenoprof_stop,
+	.backtrace	= xenoprof_dummy_backtrace
 };
 
 
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/Rules.mk
--- a/xen/Rules.mk	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/Rules.mk	Thu Jun 07 17:06:35 2007 +0530
@@ -3,10 +3,11 @@
 # If you change any of these configuration options then you must
 # 'make clean' before rebuilding.
 #
-verbose     ?= n
-perfc       ?= n
-perfc_arrays?= n
-crash_debug ?= n
+verbose      ?= n
+perfc        ?= n
+perfc_arrays ?= n
+crash_debug  ?= n
+frame_pointer?= n
 
 XEN_ROOT=$(BASEDIR)/..
 include $(XEN_ROOT)/Config.mk
@@ -19,6 +20,15 @@ ifeq ($(perfc_arrays),y)
 ifeq ($(perfc_arrays),y)
 perfc := y
 endif
+ifeq ($(frame_pointer),y)
+FRAME_ON := -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
+ifdef $(CFLAGS)
+CFLAGS   :=  $(shell echo $(CFLAGS) |\
+               sed -e 's/-fomit-frame-pointer/'"$(FRAME_ON)/")
+else
+CFLAGS   := $(FRAME_ON) 
+endif # CFLAGS defined ?
+endif # Need frame pointer
 
 # Set ARCH/SUBARCH appropriately.
 override COMPILE_SUBARCH := $(XEN_COMPILE_ARCH)
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/arch/ia64/xen/oprofile/perfmon.c
--- a/xen/arch/ia64/xen/oprofile/perfmon.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/arch/ia64/xen/oprofile/perfmon.c	Thu Jun 07 17:06:35 2007 +0530
@@ -37,7 +37,7 @@
 #include <asm/ptrace.h>
 
 // XXX move them to an appropriate header file
-extern void xenoprof_log_event(struct vcpu *vcpu,
+extern void xenoprof_log_event(struct vcpu *vcpu, struct pt_regs * regs,
                                unsigned long eip, int mode, int event);
 extern int is_active(struct domain *d);
 
@@ -55,7 +55,10 @@ xenoprof_handler(struct task_struct *tas
     if (!allow_virq || !allow_ints)
         return 0;
 
-    xenoprof_log_event(current, ip, xenoprofile_get_mode(task, regs), event);
+    // Note that log event actually expect cpu_user_regs, cast back 
+    // appropriately when doing the backtrace implementation in ia64
+    xenoprof_log_event(current, regs, ip, xenoprofile_get_mode(task, regs), 
+					   event);
     
     // send VIRQ_XENOPROF
     if (is_active(current->domain) && !ring_0(regs))
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/arch/x86/oprofile/Makefile
--- a/xen/arch/x86/oprofile/Makefile	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/arch/x86/oprofile/Makefile	Thu Jun 07 17:06:35 2007 +0530
@@ -3,3 +3,4 @@ obj-y += op_model_p4.o
 obj-y += op_model_p4.o
 obj-y += op_model_ppro.o
 obj-y += op_model_athlon.o
+obj-y += backtrace.o
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/arch/x86/oprofile/op_model_athlon.c
--- a/xen/arch/x86/oprofile/op_model_athlon.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/arch/x86/oprofile/op_model_athlon.c	Thu Jun 07 17:06:35 2007 +0530
@@ -43,8 +43,8 @@
 
 static unsigned long reset_value[NUM_COUNTERS];
 
-extern void xenoprof_log_event(struct vcpu *v, unsigned long eip,
-			       int mode, int event);
+extern void xenoprof_log_event(struct vcpu *v, struct cpu_user_regs * regs, 
+			       unsigned long eip, int mode, int event);
 extern int xenoprofile_get_mode(struct vcpu *v,
 				struct cpu_user_regs * const regs);
 
@@ -130,7 +130,7 @@ static int athlon_check_ctrs(unsigned in
 	for (i = 0 ; i < NUM_COUNTERS; ++i) {
 		CTR_READ(low, high, msrs, i);
 		if (CTR_OVERFLOWED(low)) {
-			xenoprof_log_event(current, eip, mode, i);
+			xenoprof_log_event(current, regs, eip, mode, i);
 			CTR_WRITE(reset_value[i], msrs, i);
 			ovf = 1;
 		}
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/arch/x86/oprofile/op_model_p4.c
--- a/xen/arch/x86/oprofile/op_model_p4.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/arch/x86/oprofile/op_model_p4.c	Thu Jun 07 17:06:35 2007 +0530
@@ -620,8 +620,8 @@ static void p4_setup_ctrs(struct op_msrs
 	}
 }
 
-extern void xenoprof_log_event(struct vcpu *v, unsigned long eip,
-			       int mode, int event);
+extern void xenoprof_log_event(struct vcpu *v, struct cpu_user_regs * regs, 
+			       unsigned long eip, int mode, int event);
 extern int xenoprofile_get_mode(struct vcpu *v,
 				struct cpu_user_regs * const regs);
 
@@ -664,8 +664,8 @@ static int p4_check_ctrs(unsigned int co
 		CCCR_READ(low, high, real);
  		CTR_READ(ctr, high, real);
 		if (CCCR_OVF_P(low) || CTR_OVERFLOW_P(ctr)) {
-			xenoprof_log_event(current, eip, mode, i);
- 			CTR_WRITE(reset_value[i], real);
+			xenoprof_log_event(current, regs, eip, mode, i);
+			CTR_WRITE(reset_value[i], real);
 			CCCR_CLEAR_OVF(low);
 			CCCR_WRITE(low, high, real);
  			CTR_WRITE(reset_value[i], real);
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/arch/x86/oprofile/op_model_ppro.c
--- a/xen/arch/x86/oprofile/op_model_ppro.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/arch/x86/oprofile/op_model_ppro.c	Thu Jun 07 17:06:35 2007 +0530
@@ -88,8 +88,8 @@ static void ppro_setup_ctrs(struct op_ms
 	}
 }
 
-extern void xenoprof_log_event(struct vcpu *v, unsigned long eip,
-			       int mode, int event);
+extern void xenoprof_log_event(struct vcpu *v, struct cpu_user_regs * regs, 
+			       unsigned long eip, int mode, int event);
 extern int xenoprofile_get_mode(struct vcpu *v,
 				struct cpu_user_regs * const regs);
  
@@ -106,7 +106,7 @@ static int ppro_check_ctrs(unsigned int 
 	for (i = 0 ; i < NUM_COUNTERS; ++i) {
 		CTR_READ(low, high, msrs, i);
 		if (CTR_OVERFLOWED(low)) {
-			xenoprof_log_event(current, eip, mode, i);
+			xenoprof_log_event(current, regs, eip, mode, i);
 			CTR_WRITE(reset_value[i], msrs, i);
 			ovf = 1;
 		}
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/common/xenoprof.c
--- a/xen/common/xenoprof.c	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/common/xenoprof.c	Thu Jun 07 17:06:35 2007 +0530
@@ -31,6 +31,7 @@ unsigned int activated;
 unsigned int activated;
 struct domain *xenoprof_primary_profiler;
 int xenoprof_state = XENOPROF_IDLE;
+static unsigned long backtrace_depth=0;
 
 u64 total_samples;
 u64 invalid_buffer_samples;
@@ -414,55 +415,47 @@ static int add_passive_list(XEN_GUEST_HA
     return ret;
 }
 
-void xenoprof_log_event(
-    struct vcpu *vcpu, unsigned long eip, int mode, int event)
-{
-    struct domain *d = vcpu->domain;
-    struct xenoprof_vcpu *v;
-    xenoprof_buf_t *buf;
+
+/* Get space in the buffer */
+static int xenoprof_buf_space(struct domain *d, xenoprof_buf_t * buf, int size)
+{
+    int head;
+    int tail;
+
+    head = xenoprof_buf(d, buf, event_head);
+    tail = xenoprof_buf(d, buf, event_tail);
+
+    if(tail > head){
+        return ((tail - head) - 1);
+    }
+    else {
+        return (size - (head -tail) -1);
+    }
+}
+
+/* check for space and add a sample
+ * return 1 if successful, 0 otherwise
+ */
+
+static inline int xenoprof_add_sample(struct domain *d, xenoprof_buf_t *buf,
+                                      unsigned long eip, int mode, int event)
+{
     int head;
     int tail;
     int size;
 
-
-    total_samples++;
-
-    /* ignore samples of un-monitored domains */
-    /* Count samples in idle separate from other unmonitored domains */
-    if ( !is_profiled(d) )
-    {
-        others_samples++;
-        return;
-    }
-
-    v = &d->xenoprof->vcpu[vcpu->vcpu_id];
-
-    /* Sanity check. Should never happen */ 
-    if ( v->buffer == NULL )
-    {
-        invalid_buffer_samples++;
-        return;
-    }
-
-    buf = v->buffer;
-
     head = xenoprof_buf(d, buf, event_head);
     tail = xenoprof_buf(d, buf, event_tail);
-    size = v->event_size;
-
+    size = xenoprof_buf(d, buf, event_size);
+    
     /* make sure indexes in shared buffer are sane */
     if ( (head < 0) || (head >= size) || (tail < 0) || (tail >= size) )
     {
         corrupted_buffer_samples++;
-        return;
-    }
-
-    if ( (head == tail - 1) || (head == size - 1 && tail == 0) )
-    {
-        xenoprof_buf(d, buf, lost_samples)++;
-        lost_samples++;
-    }
-    else
+        return 0;
+    }
+
+    if(xenoprof_buf_space(d, buf, size) > 0)
     {
         xenoprof_buf(d, buf, event_log[head].eip) = eip;
         xenoprof_buf(d, buf, event_log[head].mode) = mode;
@@ -470,7 +463,87 @@ void xenoprof_log_event(
         head++;
         if ( head >= size )
             head = 0;
+        
         xenoprof_buf(d, buf, event_head) = head;
+    }
+    else
+    {
+        xenoprof_buf(d, buf, lost_samples)++;
+        lost_samples++;
+        return 0;
+    }
+    return 1;
+}
+
+/* add a trace to the buffer */
+int xenoprof_add_trace(struct domain *d, struct vcpu *vcpu,
+                       unsigned long eip, int mode)
+{
+    xenoprof_buf_t * buf;
+    
+    /* Assume buffer is not null, it should have been checked before entering
+     * the arch specific backtrace code
+     */
+    
+    buf = (d->xenoprof->vcpu[vcpu->vcpu_id]).buffer;    
+
+    /* Ensure we do not accidentally write an escape code due to 
+     * a broken frame
+     */
+
+    if(eip==XENOPROF_ESCAPE_CODE)
+    {
+        invalid_buffer_samples++;
+        return 0;
+    }
+
+    return xenoprof_add_sample(d, buf, eip, mode, 0);
+}
+
+void xenoprof_log_event(struct vcpu *vcpu, 
+                        struct cpu_user_regs * regs, unsigned long eip, 
+                        int mode, int event)
+{
+    struct domain *d = vcpu->domain;
+    struct xenoprof_vcpu *v;
+    xenoprof_buf_t *buf;
+
+    total_samples++;
+
+    /* ignore samples of un-monitored domains */
+    /* Count samples in idle separate from other unmonitored domains */
+    if ( !is_profiled(d) )
+    {
+        others_samples++;
+        return;
+    }
+
+    v = &d->xenoprof->vcpu[vcpu->vcpu_id];
+    
+    if(v->buffer == NULL)
+    {
+        invalid_buffer_samples++;
+        return;
+    }
+    
+    buf = v->buffer;
+
+    /* Backtrace if asked for */
+    if(backtrace_depth > 0)
+    {
+        if(xenoprof_buf_space(d, buf, v->event_size) < 2){
+            xenoprof_buf(d, buf, lost_samples)++;
+            lost_samples++;
+            return;
+        }       
+        if(!xenoprof_add_sample(d, buf, XENOPROF_ESCAPE_CODE, mode, 
+		   XENOPROF_TRACE_BEGIN))
+        { /* could happen due to a corrupted buffer */
+            return;
+        }
+    }
+    if(xenoprof_add_sample(d, buf, eip, mode, event))
+    {
         if ( is_active(vcpu->domain) )
             active_samples++;
         else
@@ -481,8 +554,15 @@ void xenoprof_log_event(
             xenoprof_buf(d, buf, kernel_samples)++;
         else
             xenoprof_buf(d, buf, xen_samples)++;
-    }
-}
+    
+    }
+    if(backtrace_depth > 0)
+    {
+        backtrace(d, vcpu, regs, backtrace_depth, mode);
+    }
+}
+
+
 
 static int xenoprof_op_init(XEN_GUEST_HANDLE(void) arg)
 {
@@ -729,7 +809,18 @@ int do_xenoprof_op(int op, XEN_GUEST_HAN
             activated = 0;
             adomains=0;
             xenoprof_primary_profiler = NULL;
+            backtrace_depth=0;
             ret = 0;
+        }
+        break;
+                
+    case XENOPROF_set_backtrace:
+        ret = 0;
+        if(!arch_backtrace_supported()){
+            ret = -EINVAL;
+        }
+        else if(copy_from_guest(&backtrace_depth, arg, 1)){
+            ret=-EFAULT;
         }
         break;
 
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/include/asm-ia64/xenoprof.h
--- a/xen/include/asm-ia64/xenoprof.h	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/include/asm-ia64/xenoprof.h	Thu Jun 07 17:06:35 2007 +0530
@@ -37,7 +37,16 @@ struct vcpu;
 struct vcpu;
 struct cpu_user_regs;
 int xenoprofile_get_mode(struct vcpu *v, struct cpu_user_regs * const regs);
-
+static inline int arch_backtrace_supported(void)
+{
+    return 0;
+}
+static inline void backtrace(struct domain *d, struct vcpu *vcpu, 
+                      struct pt_regs *const regs, unsigned long depth, int mode)
+{
+    /* To be implemented */
+    return;
+}
 #define xenoprof_shared_gmfn(d, gmaddr, maddr)  \
     assign_domain_page((d), (gmaddr), (maddr));
 
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/include/asm-x86/xenoprof.h
--- a/xen/include/asm-x86/xenoprof.h	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/include/asm-x86/xenoprof.h	Thu Jun 07 17:06:35 2007 +0530
@@ -47,6 +47,12 @@ struct vcpu;
 struct vcpu;
 struct cpu_user_regs;
 int xenoprofile_get_mode(struct vcpu *v, struct cpu_user_regs * const regs);
+static inline int arch_backtrace_supported(void)
+{
+    return 1;
+}
+void backtrace(struct domain *d, struct vcpu *vcpu, 
+               struct cpu_user_regs *const regs, unsigned long depth, int mode);
 #define xenoprof_shared_gmfn(d, gmaddr, maddr)                      \
     do {                                                            \
         (void)(maddr);                                              \
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/include/public/xenoprof.h
--- a/xen/include/public/xenoprof.h	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/include/public/xenoprof.h	Thu Jun 07 17:06:35 2007 +0530
@@ -49,7 +49,8 @@
 #define XENOPROF_release_counters   12
 #define XENOPROF_shutdown           13
 #define XENOPROF_get_buffer         14
-#define XENOPROF_last_op            14
+#define XENOPROF_set_backtrace      15
+#define XENOPROF_last_op            15
 
 #define MAX_OPROF_EVENTS    32
 #define MAX_OPROF_DOMAINS   25
@@ -61,6 +62,11 @@ struct event_log {
     uint8_t mode;
     uint8_t event;
 };
+
+/* PC value that indicates a special code */
+#define XENOPROF_ESCAPE_CODE ~0UL
+/* Transient events for the xenoprof->oprofile cpu buf */
+#define XENOPROF_TRACE_BEGIN 1
 
 /* Xenoprof buffer shared between Xen and domain - 1 per VCPU */
 struct xenoprof_buf {
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/include/xen/xenoprof.h
--- a/xen/include/xen/xenoprof.h	Fri Jun 01 14:50:52 2007 +0100
+++ b/xen/include/xen/xenoprof.h	Thu Jun 07 17:06:35 2007 +0530
@@ -66,6 +66,8 @@ void free_xenoprof_pages(struct domain *
 void free_xenoprof_pages(struct domain *d);
 
 int do_xenoprof_op(int op, XEN_GUEST_HANDLE(void) arg);
+int xenoprof_add_trace(struct domain *d, struct vcpu *v, 
+                       unsigned long eip, int mode);
 
 extern struct domain *xenoprof_primary_profiler;
 
diff -r bd3d6b4c52ec -r e6f484a96f43 xen/arch/x86/oprofile/backtrace.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/arch/x86/oprofile/backtrace.c	Thu Jun 07 17:06:35 2007 +0530
@@ -0,0 +1,138 @@
+/**
+ * @file backtrace.c
+ *
+ * @remark Copyright 2002 OProfile authors
+ * @remark Read the file COPYING
+ *
+ * @author John Levon
+ * @author David Smith
+ * Modified for Xen by Amitabha Roy
+ *
+ */
+
+#include<xen/types.h>
+#include<asm/page.h>
+#include<xen/xenoprof.h>
+#include<asm/guest_access.h>
+
+struct frame_head {
+	struct frame_head * ebp;
+	unsigned long ret;
+} __attribute__((packed));
+
+static struct frame_head *
+dump_hypervisor_backtrace(struct domain *d, struct vcpu *vcpu, 
+			  struct frame_head * head, int mode)
+{
+	if(!xenoprof_add_trace(d, vcpu, head->ret, mode))
+		return 0;
+
+	/* frame pointers should strictly progress back up the stack
+	 * (towards higher addresses) */
+	if (head >= head->ebp)
+		return NULL;
+
+	return head->ebp;
+}
+
+static struct frame_head *
+dump_guest_backtrace(struct domain *d, struct vcpu *vcpu, 
+		     struct frame_head * head, int mode)
+{
+	struct frame_head bufhead[2];
+	XEN_GUEST_HANDLE(char) guest_head = guest_handle_from_ptr(head, char);
+	
+	/* Also check accessibility of one struct frame_head beyond */
+	if (!guest_handle_okay(guest_head, sizeof(bufhead)))
+		return 0;
+	if (__copy_from_guest_offset((char *)bufhead, guest_head, 0, 
+	    sizeof(bufhead)))
+		return 0;
+
+	if(!xenoprof_add_trace(d, vcpu, bufhead[0].ret, mode))
+	    return 0;
+
+	/* frame pointers should strictly progress back up the stack
+	 * (towards higher addresses) */
+	if (head >= bufhead[0].ebp)
+		return NULL;
+
+	return bufhead[0].ebp;
+}
+
+/*
+ * |             | /\ Higher addresses
+ * |             |
+ * --------------- stack base (address of current_thread_info)
+ * | thread info |
+ * .             .
+ * |    stack    |
+ * --------------- saved regs->ebp value if valid (frame_head address)
+ * .             .
+ * --------------- saved regs->rsp value if x86_64
+ * |             |
+ * --------------- struct pt_regs * stored on stack if 32-bit
+ * |             |
+ * .             .
+ * |             |
+ * --------------- %esp
+ * |             |
+ * |             | \/ Lower addresses
+ *
+ * Thus, regs (or regs->rsp for x86_64) <-> stack base restricts the
+ * valid(ish) ebp values. Note: (1) for x86_64, NMI and several other
+ * exceptions use special stacks, maintained by the interrupt stack table
+ * (IST). These stacks are set up in trap_init() in
+ * arch/x86_64/kernel/traps.c. Thus, for x86_64, regs now does not point
+ * to the kernel stack; instead, it points to some location on the NMI
+ * stack. On the other hand, regs->rsp is the stack pointer saved when the
+ * NMI occurred. (2) For 32-bit, regs->esp is not valid because the
+ * processor does not save %esp on the kernel stack when interrupts occur
+ * in the kernel mode.
+ */
+#ifdef CONFIG_FRAME_POINTER
+static int valid_hypervisor_stack(struct frame_head * head, 
+				  struct cpu_user_regs * regs)
+{
+	unsigned long headaddr = (unsigned long)head;
+#ifdef CONFIG_X86_64
+	unsigned long stack = (unsigned long)regs->rsp;
+#else
+	unsigned long stack = (unsigned long)regs;
+#endif
+	unsigned long stack_base = (stack & ~(STACK_SIZE - 1)) + STACK_SIZE;
+
+	return headaddr > stack && headaddr < stack_base;
+}
+#else
+/* without fp, it's just junk */
+static int valid_hypervisor_stack(struct frame_head * head, 
+				  struct cpu_user_regs * regs)
+{
+	return 0;
+}
+#endif
+
+
+void
+backtrace(struct domain *d, struct vcpu *vcpu, 
+	  struct cpu_user_regs * const regs, unsigned long depth, 
+	  int mode)
+{
+	struct frame_head *head;
+
+#ifdef CONFIG_X86_64
+	head = (struct frame_head *)regs->rbp;
+#else
+	head = (struct frame_head *)regs->ebp;
+#endif
+
+	if (mode > 1) {
+		while (depth-- && valid_hypervisor_stack(head, regs))
+		    head = dump_hypervisor_backtrace(d, vcpu, head, mode);
+		return;
+	}
+
+	while (depth-- && head)
+	    head = dump_guest_backtrace(d, vcpu, head, mode);
+}

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] Add callgraph support to Xenoprofile

[Amitabha Roy]

Forgot to mention, the CONFIG_FRAME_POINTER is needed by the
backtrace.c file I pulled from linux.

On 6/7/07, Amitabha Roy <amitabha.roy@gmail.com> wrote:
> I've moved the frame pointer code into the xen specific makefile.
> I've also fixed the per file inconsistencies. I noticed that some
> files already have a few inconsistencies within themselves in places
> other than my code.
>
> Perhaps you could accept this patch and I can send a separate one
> later fixing those ?
> One of the suggestions I had from Renato was to minimize any diffs of
> files based off linux versions so that pulling in updates will be
> easier. So I can look into those on a case by case basis with him.
>
> -Amitabha
>
>
> On 6/7/07, Keir Fraser <keir@xensource.com> wrote:
> > On 7/6/07 10:20, "Amitabha Roy" <amitabha.roy@gmail.com> wrote:
> >
> > > Coding style is likely because I've preserved the formatting of the
> > > original files (esp. of backtrace.c from Linux for simple diffs).
> > > There isnt a consistent style across the files that I could follow.
> >
> > Linux-derived files follow Linux style (this will apply to many of the files
> > under arch/x86/oprofile). Xen-specific files follow Xen style as in e.g.,
> > page_alloc.c, xmalloc.c, xenoprof.c).
> >
> > If any of the files you edit don't follow this, please fix them first, and
> > send a separate patch as a prerequisite for your own.
> >
> > > CONFIG_FRAME_POINTER gets used in config/StdGNU.mk.
> > > I simply followed whatever was done for XEN_TARGET_X86_PAE. However
> > > unlike the PAE case I haven't ensured that frame pointers are turned
> > > on both in the kernel and Xen. If you want I can move all the frame
> > > pointer related stuff into xen/Rules.mk.
> >
> > Yes, please. Call the option frame_pointer, in line with other Xen-specific
> > options which are also lower case.
> >
> >  -- Keir
> >
> >
>
>