perf & kvm: Enhance perf to collect KVM guest os statistics from host side

PATCH In review Zhang, Yanmin 4 participants 16 messages Apr 19, 2010 – Apr 26, 2010

Subsystems (4)

x86 KVM perf events KVM x86

lore

Share & tools

lore

Zhang, Yanmin

author

2010-04-19 05:33 UTC #0 lore raw

PATCH v5 1/3 perf & kvm: Enhance perf to collect KVM guest os statistics from host side

Below patch introduces perf_guest_info_callbacks and related register/unregister functions. Add more PERF_RECORD_MISC_XXX bits meaning guest kernel and guest user space.

Signed-off-by Zhang Yanmin <[email protected]>

---

arch/x86/include/asm/perf_event.h +4 ~~-11~~

diff -Nraup --exclude-from=exclude.diff linux-2.6_tip0417/arch/x86/include/asm/perf_event.h linux-2.6_tip0417_perfkvm/arch/x86/include/asm/perf_event.h
@@ -135,17 +135,10 @@ extern void perf_events_lapic_init(void)
  */
 #define PERF_EFLAGS_EXACT	(1UL << 3)
 
-#define perf_misc_flags(regs)				\
-({	int misc = 0;					\
-	if (user_mode(regs))				\
-		misc |= PERF_RECORD_MISC_USER;		\
-	else						\
-		misc |= PERF_RECORD_MISC_KERNEL;	\
-	if (regs->flags & PERF_EFLAGS_EXACT)		\
-		misc |= PERF_RECORD_MISC_EXACT;		\
-	misc; })
-
-#define perf_instruction_pointer(regs)	((regs)->ip)
+struct pt_regs;
+extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
+extern unsigned long perf_misc_flags(struct pt_regs *regs);
+#define perf_misc_flags(regs)	perf_misc_flags(regs)
 
 #else
 static inline void init_hw_perf_events(void)		{ }

arch/x86/kernel/cpu/perf_event.c +32 -0

diff -Nraup --exclude-from=exclude.diff linux-2.6_tip0417/arch/x86/kernel/cpu/perf_event.c linux-2.6_tip0417_perfkvm/arch/x86/kernel/cpu/perf_event.c
@@ -1720,6 +1720,11 @@ struct perf_callchain_entry *perf_callch
 {
 	struct perf_callchain_entry *entry;
 
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+		/* TODO: We don't support guest os callchain now */
+		return NULL;
+	}
+
 	if (in_nmi())
 		entry = &__get_cpu_var(pmc_nmi_entry);
 	else

@@ -1743,3 +1748,30 @@ void perf_arch_fetch_caller_regs(struct 
 	regs->cs = __KERNEL_CS;
 	local_save_flags(regs->flags);
 }
+
+unsigned long perf_instruction_pointer(struct pt_regs *regs)
+{
+	unsigned long ip;
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
+		ip = perf_guest_cbs->get_guest_ip();
+	else
+		ip = instruction_pointer(regs);
+	return ip;
+}
+
+unsigned long perf_misc_flags(struct pt_regs *regs)
+{
+	int misc = 0;
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+		misc |= perf_guest_cbs->is_user_mode() ?
+			PERF_RECORD_MISC_GUEST_USER :
+			PERF_RECORD_MISC_GUEST_KERNEL;
+	} else
+		misc |= user_mode(regs) ? PERF_RECORD_MISC_USER :
+			PERF_RECORD_MISC_KERNEL;
+	if (regs->flags & PERF_EFLAGS_EXACT)
+		misc |= PERF_RECORD_MISC_EXACT;
+
+	return misc;
+}
+

include/linux/perf_event.h +20 -1

diff -Nraup --exclude-from=exclude.diff linux-2.6_tip0417/include/linux/perf_event.h linux-2.6_tip0417_perfkvm/include/linux/perf_event.h
@@ -288,11 +288,13 @@ struct perf_event_mmap_page {
 	__u64	data_tail;		/* user-space written tail */
 };
 
-#define PERF_RECORD_MISC_CPUMODE_MASK		(3 << 0)
+#define PERF_RECORD_MISC_CPUMODE_MASK		(7 << 0)
 #define PERF_RECORD_MISC_CPUMODE_UNKNOWN	(0 << 0)
 #define PERF_RECORD_MISC_KERNEL			(1 << 0)
 #define PERF_RECORD_MISC_USER			(2 << 0)
 #define PERF_RECORD_MISC_HYPERVISOR		(3 << 0)
+#define PERF_RECORD_MISC_GUEST_KERNEL		(4 << 0)
+#define PERF_RECORD_MISC_GUEST_USER		(5 << 0)
 
 #define PERF_RECORD_MISC_EXACT			(1 << 14)
 /*

@@ -446,6 +448,12 @@ enum perf_callchain_context {
 # include <asm/perf_event.h>
 #endif
 
+struct perf_guest_info_callbacks {
+	int (*is_in_guest) (void);
+	int (*is_user_mode) (void);
+	unsigned long (*get_guest_ip) (void);
+};
+
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
 #include <asm/hw_breakpoint.h>
 #endif

@@ -932,6 +940,12 @@ static inline void perf_event_mmap(struc
 		__perf_event_mmap(vma);
 }
 
+extern struct perf_guest_info_callbacks *perf_guest_cbs;
+extern int perf_register_guest_info_callbacks(
+		struct perf_guest_info_callbacks *);
+extern int perf_unregister_guest_info_callbacks(
+		struct perf_guest_info_callbacks *);
+
 extern void perf_event_comm(struct task_struct *tsk);
 extern void perf_event_fork(struct task_struct *tsk);

@@ -1001,6 +1015,11 @@ perf_sw_event(u32 event_id, u64 nr, int 
 static inline void
 perf_bp_event(struct perf_event *event, void *data)			{ }
 
+static inline int perf_register_guest_info_callbacks
+(struct perf_guest_info_callbacks *) {return 0; }
+static inline int perf_unregister_guest_info_callbacks
+(struct perf_guest_info_callbacks *) {return 0; }
+
 static inline void perf_event_mmap(struct vm_area_struct *vma)		{ }
 static inline void perf_event_comm(struct task_struct *tsk)		{ }
 static inline void perf_event_fork(struct task_struct *tsk)		{ }

kernel/perf_event.c +22 -1

diff -Nraup --exclude-from=exclude.diff linux-2.6_tip0417/kernel/perf_event.c linux-2.6_tip0417_perfkvm/kernel/perf_event.c
@@ -2798,6 +2798,27 @@ void perf_arch_fetch_caller_regs(struct 
 
 
 /*
+ * We assume there is only KVM supporting the callbacks.
+ * Later on, we might change it to a list if there is
+ * another virtualization implementation supporting the callbacks.
+ */
+struct perf_guest_info_callbacks *perf_guest_cbs;
+
+int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
+{
+	perf_guest_cbs = cbs;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks);
+
+int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
+{
+	perf_guest_cbs = NULL;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
+
+/*
  * Output
  */
 static bool perf_output_space(struct perf_mmap_data *data, unsigned long tail,

@@ -3749,7 +3770,7 @@ void __perf_event_mmap(struct vm_area_st
 		.event_id  = {
 			.header = {
 				.type = PERF_RECORD_MMAP,
-				.misc = 0,
+				.misc = PERF_RECORD_MISC_USER,
 				/* .size */
 			},
 			/* .pid */

Avi Kivity

2010-04-19 08:38 UTC #1 lore raw

Quoted 4 lines · Yanmin

Below patch introduces perf_guest_info_callbacks and related register/unregister functions. Add more PERF_RECORD_MISC_XXX bits meaning guest kernel and guest user space.

This doesn't apply against upstream. What branch was this generated against?

Signature · 1 line

error compiling committee.c: too many arguments to function

Zhang, Yanmin

author

2010-04-19 08:55 UTC #2 lore raw

Quoted 7 lines · Avi Kivity

Quoted 4 lines · Yanmin
Below patch introduces perf_guest_info_callbacks and related register/unregister functions. Add more PERF_RECORD_MISC_XXX bits meaning guest kernel and guest user space.
This doesn't apply against upstream.

What does upstream mean here? The vanilla 2.6.34-rc4?

Quoted 3 lines

What branch was this generated
against?

It's against the latest tip/master. I checked out to 19b26586090 as the latest tip/master has some updates on perf.

Avi Kivity

2010-04-19 09:00 UTC #3 lore raw

Quoted 4 lines · Yanmin

Quoted 2 lines · Avi Kivity
On 04/19/2010 08:32 AM, Zhang, Yanmin wrote:

Quoted 9 lines

Quoted 7 lines
Quoted 5 lines
Below patch introduces perf_guest_info_callbacks and related register/unregister functions. Add more PERF_RECORD_MISC_XXX bits meaning guest kernel and guest user space.
This doesn't apply against upstream.
What does upstream mean here? The vanilla 2.6.34-rc4?

Yes, sorry for being unclear.

Quoted 7 lines

Quoted 4 lines
What branch was this generated
against?
It's against the latest tip/master. I checked out to 19b26586090 as the latest tip/master has some updates on perf.

I don't want to merge tip/master... does tip/perf/core contain the needed updates?

Signature · 1 line

error compiling committee.c: too many arguments to function

Avi Kivity

2010-04-19 09:09 UTC #4 lore raw

Quoted 9 lines · Avi Kivity

Quoted 6 lines
Quoted 4 lines
What branch was this generated
against?
It's against the latest tip/master. I checked out to 19b26586090 as the latest tip/master has some updates on perf.
I don't want to merge tip/master... does tip/perf/core contain the needed updates?

Note, given that there won't be changes to NMI handling in kvm, we can go the simpler route of merging all the patches in tip/perf/core.

Signature · 1 line

error compiling committee.c: too many arguments to function

Zhang, Yanmin

author

2010-04-19 09:22 UTC #5 lore raw

In reply to Avi Kivity

Quoted 21 lines · Avi Kivity

Quoted 4 lines · Yanmin
Quoted 2 lines · Avi Kivity
On 04/19/2010 08:32 AM, Zhang, Yanmin wrote:
Quoted 9 lines
Quoted 7 lines
Quoted 5 lines
Below patch introduces perf_guest_info_callbacks and related register/unregister functions. Add more PERF_RECORD_MISC_XXX bits meaning guest kernel and guest user space.
This doesn't apply against upstream.
What does upstream mean here? The vanilla 2.6.34-rc4?
Yes, sorry for being unclear.
Quoted 4 lines
Quoted 4 lines
What branch was this generated
against?

Quoted 6 lines

Quoted 3 lines
It's against the latest tip/master. I checked out to 19b26586090 as the latest tip/master has some updates on perf.
I don't want to merge tip/master... does tip/perf/core contain the needed updates?

I think so. A moment ago, I checked out to b5a80b7e9 of tip/perf/core. All 3 patches could be applied cleanly and compilation is ok. A quick testing shows tip/perf/core kernel does work.

Avi Kivity

2010-04-19 10:00 UTC #6 lore raw

Quoted 9 lines · Yanmin

Quoted 4 lines
I don't want to merge tip/master... does tip/perf/core contain the needed updates?
I think so. A moment ago, I checked out to b5a80b7e9 of tip/perf/core. All 3 patches could be applied cleanly and compilation is ok. A quick testing shows tip/perf/core kernel does work.

Thanks. I applied all three patches to the 'perf' branch in kvm.git and merged it to master.

Ingo, please pull

git://git.kernel.org/pub/scm/virt/kvm/kvm.git perf

into tip's perf/core to receive those three patches.

Signature · 1 line

error compiling committee.c: too many arguments to function

Ingo Molnar

2010-04-19 18:11 UTC #7 lore raw

In reply to Zhang, Yanmin

FYI, i found a few problems that need fixing:

arch/x86/kernel/cpu/perf_event.c void perf_arch_fetch_caller_regs(struct · line 1755

Quoted 2 lines

arch/x86/kernel/cpu/perf_event.c +2 -0

+	unsigned long ip;
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest())

missing newline.

arch/x86/kernel/cpu/perf_event.c void perf_arch_fetch_caller_regs(struct · line 1765

Quoted 2 lines

arch/x86/kernel/cpu/perf_event.c +2 -0

+	int misc = 0;
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {

ditto.

arch/x86/kernel/cpu/perf_event.c void perf_arch_fetch_caller_regs(struct · line 1771

Quoted 4 lines

arch/x86/kernel/cpu/perf_event.c +4 -0

+			PERF_RECORD_MISC_GUEST_KERNEL;
+	} else
+		misc |= user_mode(regs) ? PERF_RECORD_MISC_USER :
+			PERF_RECORD_MISC_KERNEL;

- unbalanced curly braces
- missing curly brace for multi-line statement.
- unnecessary line-break due to col80 warning from checkpatch

include/linux/perf_event.h static inline void perf_event_mmap(struc · line 947

Quoted 5 lines

include/linux/perf_event.h +5 -0

+extern struct perf_guest_info_callbacks *perf_guest_cbs;
+extern int perf_register_guest_info_callbacks(
+		struct perf_guest_info_callbacks *);
+extern int perf_unregister_guest_info_callbacks(
+		struct perf_guest_info_callbacks *);

- unnecessary line-break due to col80 warning from checkpatch

include/linux/perf_event.h perf_sw_event(u32 event_id, u64 nr, int · line 1021

Quoted 4 lines

include/linux/perf_event.h +4 -0

+static inline int perf_register_guest_info_callbacks
+(struct perf_guest_info_callbacks *) {return 0; }
+static inline int perf_unregister_guest_info_callbacks
+(struct perf_guest_info_callbacks *) {return 0; }

- invalid C: function parameter needs name even if unused
- missing space after opening curly brace

Please provide delta fixes.

Ingo

Zhang, Yanmin

author

2010-04-20 02:14 UTC #8 lore raw

Quoted 37 lines · Ingo Molnar

FYI, i found a few problems that need fixing:
Review
Quoted 2 lines
(quoted diff) +2 -0
+	unsigned long ip;
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
missing newline.
Review
Quoted 2 lines
(quoted diff) +2 -0
+	int misc = 0;
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
ditto.
Review
Quoted 4 lines
(quoted diff) +4 -0
+			PERF_RECORD_MISC_GUEST_KERNEL;
+	} else
+		misc |= user_mode(regs) ? PERF_RECORD_MISC_USER :
+			PERF_RECORD_MISC_KERNEL;
- unbalanced curly braces
- missing curly brace for multi-line statement.
- unnecessary line-break due to col80 warning from checkpatch
Review
Quoted 5 lines
(quoted diff) +5 -0
+extern struct perf_guest_info_callbacks *perf_guest_cbs;
+extern int perf_register_guest_info_callbacks(
+		struct perf_guest_info_callbacks *);
+extern int perf_unregister_guest_info_callbacks(
+		struct perf_guest_info_callbacks *);
- unnecessary line-break due to col80 warning from checkpatch
Review
Quoted 4 lines
(quoted diff) +4 -0
+static inline int perf_register_guest_info_callbacks
+(struct perf_guest_info_callbacks *) {return 0; }
+static inline int perf_unregister_guest_info_callbacks
+(struct perf_guest_info_callbacks *) {return 0; }
- invalid C: function parameter needs name even if unused
- missing space after opening curly brace

Quoted 1 line

Please provide delta fixes.

Here is the fix on the top of the prior 3 patches of V5.

From: Zhang, Yanmin <[email protected]>

Fix some programming style issues on the top of perf kvm enhancement V5.

Signed-off-by Zhang Yanmin <[email protected]>

---

arch/x86/kernel/cpu/perf_event.c +12 -6

diff -Nraup linux-2.6_tip0417_perfkvm/arch/x86/kernel/cpu/perf_event.c linux-2.6_tip0417_perfkvmstyle/arch/x86/kernel/cpu/perf_event.c
@@ -1752,23 +1752,29 @@ void perf_arch_fetch_caller_regs(struct 
 unsigned long perf_instruction_pointer(struct pt_regs *regs)
 {
 	unsigned long ip;
+
 	if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
 		ip = perf_guest_cbs->get_guest_ip();
 	else
 		ip = instruction_pointer(regs);
+
 	return ip;
 }
 
 unsigned long perf_misc_flags(struct pt_regs *regs)
 {
 	int misc = 0;
+
 	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
-		misc |= perf_guest_cbs->is_user_mode() ?
-			PERF_RECORD_MISC_GUEST_USER :
-			PERF_RECORD_MISC_GUEST_KERNEL;
-	} else
-		misc |= user_mode(regs) ? PERF_RECORD_MISC_USER :
-			PERF_RECORD_MISC_KERNEL;
+		if (perf_guest_cbs->is_user_mode())
+			misc |= PERF_RECORD_MISC_GUEST_USER;
+		else
+			misc |= PERF_RECORD_MISC_GUEST_KERNEL;
+	} else if (user_mode(regs))
+		misc |= PERF_RECORD_MISC_USER;
+	else
+		misc |= PERF_RECORD_MISC_KERNEL;
+
 	if (regs->flags & PERF_EFLAGS_EXACT)
 		misc |= PERF_RECORD_MISC_EXACT;

arch/x86/kvm/x86.c +4 -0

diff -Nraup linux-2.6_tip0417_perfkvm/arch/x86/kvm/x86.c linux-2.6_tip0417_perfkvmstyle/arch/x86/kvm/x86.c
@@ -3776,16 +3776,20 @@ static int kvm_is_in_guest(void)
 static int kvm_is_user_mode(void)
 {
 	int user_mode = 3;
+
 	if (percpu_read(current_vcpu))
 		user_mode = kvm_x86_ops->get_cpl(percpu_read(current_vcpu));
+
 	return user_mode != 0;
 }
 
 static unsigned long kvm_get_guest_ip(void)
 {
 	unsigned long ip = 0;
+
 	if (percpu_read(current_vcpu))
 		ip = kvm_rip_read(percpu_read(current_vcpu));
+
 	return ip;
 }

include/linux/perf_event.h +4 -6

diff -Nraup linux-2.6_tip0417_perfkvm/include/linux/perf_event.h linux-2.6_tip0417_perfkvmstyle/include/linux/perf_event.h
@@ -941,10 +941,8 @@ static inline void perf_event_mmap(struc
 }
 
 extern struct perf_guest_info_callbacks *perf_guest_cbs;
-extern int perf_register_guest_info_callbacks(
-		struct perf_guest_info_callbacks *);
-extern int perf_unregister_guest_info_callbacks(
-		struct perf_guest_info_callbacks *);
+extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
+extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
 
 extern void perf_event_comm(struct task_struct *tsk);
 extern void perf_event_fork(struct task_struct *tsk);

@@ -1016,9 +1014,9 @@ static inline void
 perf_bp_event(struct perf_event *event, void *data)			{ }
 
 static inline int perf_register_guest_info_callbacks
-(struct perf_guest_info_callbacks *) {return 0; }
+(struct perf_guest_info_callbacks *callbacks) { return 0; }
 static inline int perf_unregister_guest_info_callbacks
-(struct perf_guest_info_callbacks *) {return 0; }
+(struct perf_guest_info_callbacks *callbacks) { return 0; }
 
 static inline void perf_event_mmap(struct vm_area_struct *vma)		{ }
 static inline void perf_event_comm(struct task_struct *tsk)		{ }

Ingo Molnar

2010-04-20 06:06 UTC #9 lore raw

Quoted 4 lines · Zhang, Yanmin

unsigned long perf_misc_flags(struct pt_regs *regs) {
arch/x86/kernel/cpu/perf_event.c +1 -0
 	int misc = 0;
+

arch/x86/kernel/cpu/perf_event.c void perf_arch_fetch_caller_regs(struct · line 1777

Quoted 10 lines

arch/x86/kernel/cpu/perf_event.c +9 -0

 	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+		if (perf_guest_cbs->is_user_mode())
+			misc |= PERF_RECORD_MISC_GUEST_USER;
+		else
+			misc |= PERF_RECORD_MISC_GUEST_KERNEL;
+	} else if (user_mode(regs))
+		misc |= PERF_RECORD_MISC_USER;
+	else
+		misc |= PERF_RECORD_MISC_KERNEL;
+

We try to use balanced curly braces. I.e.:

	if (x) {
		boo();
	} else {
		if (y)
			foo();
		else	
			bar();
	}

And avoid unbalanced ones:

	if (x) {
		boo();
	} else
		if (y)
			foo();
		else	
			bar();

	Ingo

Ingo Molnar

2010-04-20 06:09 UTC #10 lore raw

Quoted 31 lines · Ingo Molnar

Quoted 4 lines · Zhang, Yanmin
unsigned long perf_misc_flags(struct pt_regs *regs) {
arch/x86/kernel/cpu/perf_event.c +1 -0
 	int misc = 0;
+
arch/x86/kernel/cpu/perf_event.c void perf_arch_fetch_caller_regs(struct · line 1777
Quoted 10 lines
arch/x86/kernel/cpu/perf_event.c +9 -0
 	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+		if (perf_guest_cbs->is_user_mode())
+			misc |= PERF_RECORD_MISC_GUEST_USER;
+		else
+			misc |= PERF_RECORD_MISC_GUEST_KERNEL;
+	} else if (user_mode(regs))
+		misc |= PERF_RECORD_MISC_USER;
+	else
+		misc |= PERF_RECORD_MISC_KERNEL;
+
We try to use balanced curly braces. I.e.:
	if (x) {
		boo();
	} else {
		if (y)
			foo();
		else	
			bar();
	}	
And avoid unbalanced ones:

Quoted 7 lines

	if (x) {
		boo();
	} else
		if (y)
			foo();
		else	
			bar();

Note, i fixed this in the patch and applied it to perf/core. (the invalid-C problem was causing build failures)

Thanks,

Ingo

Zhang, Yanmin

author

2010-04-20 06:21 UTC #11 lore raw

Quoted 41 lines · Ingo Molnar

Quoted 31 lines · Ingo Molnar
Quoted 4 lines · Zhang, Yanmin
unsigned long perf_misc_flags(struct pt_regs *regs) {
arch/x86/kernel/cpu/perf_event.c +1 -0
 	int misc = 0;
+
arch/x86/kernel/cpu/perf_event.c void perf_arch_fetch_caller_regs(struct · line 1777
Quoted 10 lines
arch/x86/kernel/cpu/perf_event.c +9 -0
 	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+		if (perf_guest_cbs->is_user_mode())
+			misc |= PERF_RECORD_MISC_GUEST_USER;
+		else
+			misc |= PERF_RECORD_MISC_GUEST_KERNEL;
+	} else if (user_mode(regs))
+		misc |= PERF_RECORD_MISC_USER;
+	else
+		misc |= PERF_RECORD_MISC_KERNEL;
+
We try to use balanced curly braces. I.e.:
	if (x) {
		boo();
	} else {
		if (y)
			foo();
		else	
			bar();
	}	
And avoid unbalanced ones:
Quoted 7 lines
	if (x) {
		boo();
	} else
		if (y)
			foo();
		else	
			bar();

Quoted 2 lines

Note, i fixed this in the patch and applied it to perf/core. (the invalid-C problem was causing build failures)

Thanks for your teaching. Originally, I used {} in the 2nd half, but deleted it.

Yanmin

tip-bot for Zhang, Yanmin

author

2010-04-20 17:55 UTC #12 lore raw

[tip:perf/core] perf & kvm: Clean up some of the guest profiling callback API details

In reply to Zhang, Yanmin

Commit-ID:  dcf46b9443ad48a227a61713adea001228925adf
Gitweb:     http://git.kernel.org/tip/dcf46b9443ad48a227a61713adea001228925adf
Author:     Zhang, Yanmin <[email protected]>
AuthorDate: Tue, 20 Apr 2010 10:13:58 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 20 Apr 2010 08:08:28 +0200

perf & kvm: Clean up some of the guest profiling callback API details

Fix some build bug and programming style issues:

- use valid C
- fix up various style details

Signed-off-by Zhang Yanmin <[email protected]>

Cc Avi Kivity <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Sheng Yang <[email protected]>

Cc Marcelo Tosatti <[email protected]>

Cc oerg Roedel <[email protected]>

Cc Jes Sorensen <[email protected]>

Cc Gleb Natapov <[email protected]>

Cc Zachary Amsden <[email protected]>

Cc [email protected]

Cc Arnaldo Carvalho de Melo <[email protected]>

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/cpu/perf_event.c |   20 ++++++++++++++------
 arch/x86/kvm/x86.c               |    4 ++++
 include/linux/perf_event.h       |   10 ++++------
 3 files changed, 22 insertions(+), 12 deletions(-)

arch/x86/kernel/cpu/perf_event.c +14 -6

index 2ea78ab..7de7061 100644
@@ -1752,23 +1752,31 @@ void perf_arch_fetch_caller_regs(struct pt_regs *regs, unsigned long ip, int ski
 unsigned long perf_instruction_pointer(struct pt_regs *regs)
 {
 	unsigned long ip;
+
 	if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
 		ip = perf_guest_cbs->get_guest_ip();
 	else
 		ip = instruction_pointer(regs);
+
 	return ip;
 }
 
 unsigned long perf_misc_flags(struct pt_regs *regs)
 {
 	int misc = 0;
+
 	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
-		misc |= perf_guest_cbs->is_user_mode() ?
-			PERF_RECORD_MISC_GUEST_USER :
-			PERF_RECORD_MISC_GUEST_KERNEL;
-	} else
-		misc |= user_mode(regs) ? PERF_RECORD_MISC_USER :
-			PERF_RECORD_MISC_KERNEL;
+		if (perf_guest_cbs->is_user_mode())
+			misc |= PERF_RECORD_MISC_GUEST_USER;
+		else
+			misc |= PERF_RECORD_MISC_GUEST_KERNEL;
+	} else {
+		if (user_mode(regs))
+			misc |= PERF_RECORD_MISC_USER;
+		else
+			misc |= PERF_RECORD_MISC_KERNEL;
+	}
+
 	if (regs->flags & PERF_EFLAGS_EXACT)
 		misc |= PERF_RECORD_MISC_EXACT;

arch/x86/kvm/x86.c +4 -0

index c3a33b2..21b9b6a 100644
@@ -3776,16 +3776,20 @@ static int kvm_is_in_guest(void)
 static int kvm_is_user_mode(void)
 {
 	int user_mode = 3;
+
 	if (percpu_read(current_vcpu))
 		user_mode = kvm_x86_ops->get_cpl(percpu_read(current_vcpu));
+
 	return user_mode != 0;
 }
 
 static unsigned long kvm_get_guest_ip(void)
 {
 	unsigned long ip = 0;
+
 	if (percpu_read(current_vcpu))
 		ip = kvm_rip_read(percpu_read(current_vcpu));
+
 	return ip;
 }

include/linux/perf_event.h +4 -6

index 24de5f1..ace31fb 100644
@@ -941,10 +941,8 @@ static inline void perf_event_mmap(struct vm_area_struct *vma)
 }
 
 extern struct perf_guest_info_callbacks *perf_guest_cbs;
-extern int perf_register_guest_info_callbacks(
-		struct perf_guest_info_callbacks *);
-extern int perf_unregister_guest_info_callbacks(
-		struct perf_guest_info_callbacks *);
+extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
+extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
 
 extern void perf_event_comm(struct task_struct *tsk);
 extern void perf_event_fork(struct task_struct *tsk);

@@ -1016,9 +1014,9 @@ static inline void
 perf_bp_event(struct perf_event *event, void *data)			{ }
 
 static inline int perf_register_guest_info_callbacks
-(struct perf_guest_info_callbacks *) {return 0; }
+(struct perf_guest_info_callbacks *callbacks) { return 0; }
 static inline int perf_unregister_guest_info_callbacks
-(struct perf_guest_info_callbacks *) {return 0; }
+(struct perf_guest_info_callbacks *callbacks) { return 0; }
 
 static inline void perf_event_mmap(struct vm_area_struct *vma)		{ }
 static inline void perf_event_comm(struct task_struct *tsk)		{ }

Liu Yu-B13201

2010-04-22 10:27 UTC #13 lore raw

In reply to Zhang, Yanmin

Quoted 59 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Zhang, Yanmin
Sent: Monday, April 19, 2010 1:33 PM
To: Avi Kivity
Cc: Ingo Molnar; Peter Zijlstra; Avi Kivity; Sheng Yang; [email protected]; [email protected]; Marcelo Tosatti; oerg Roedel; Jes Sorensen; Gleb Natapov; Zachary Amsden; [email protected]; [email protected]; Arnaldo Carvalho de Melo
Subject: [PATCH V5 1/3] perf & kvm: Enhance perf to collect KVM guest os statistics from host side
Below patch introduces perf_guest_info_callbacks and related register/unregister
functions. Add more PERF_RECORD_MISC_XXX bits meaning guest kernel and guest user
space.
Signed-off-by Zhang Yanmin <[email protected]>
---
diff -Nraup --exclude-from=exclude.diff linux-2.6_tip0417/include/linux/perf_event.h linux-2.6_tip0417_perfkvm/include/linux/perf_event.h --- linux-2.6_tip0417/include/linux/perf_event.h
2010-04-19 09:51:59.544791000 +0800
+++ linux-2.6_tip0417_perfkvm/include/linux/perf_event.h	
2010-04-19 09:53:59.691378953 +0800
include/linux/perf_event.h +11 -0
@@ -932,6 +940,12 @@ static inline void perf_event_mmap(struc
 		__perf_event_mmap(vma);
 }
 
+extern struct perf_guest_info_callbacks *perf_guest_cbs;
+extern int perf_register_guest_info_callbacks(
+		struct perf_guest_info_callbacks *);
+extern int perf_unregister_guest_info_callbacks(
+		struct perf_guest_info_callbacks *);
+
 extern void perf_event_comm(struct task_struct *tsk);
 extern void perf_event_fork(struct task_struct *tsk);
 
@@ -1001,6 +1015,11 @@ perf_sw_event(u32 event_id, u64 nr, int 
 static inline void
 perf_bp_event(struct perf_event *event, void *data)		
 	{ }
 
+static inline int perf_register_guest_info_callbacks
+(struct perf_guest_info_callbacks *) {return 0; }
+static inline int perf_unregister_guest_info_callbacks
+(struct perf_guest_info_callbacks *) {return 0; }
+
 static inline void perf_event_mmap(struct vm_area_struct 
 *vma)		{ }
static inline void perf_event_comm(struct task_struct *tsk)

Quoted 3 lines

	{ }
 static inline void perf_event_fork(struct task_struct *tsk)	
	{ }

Hi,

I met this error when built kernel. Anything wrong?

CC init/main.o
In file included from include/linux/ftrace_event.h:8,

                 from include/trace/syscall.h:6,
                 from include/linux/syscalls.h:75,
                 from init/main.c:16:

include/linux/perf_event.h: In function 'perf_register_guest_info_callbacks':
include/linux/perf_event.h:1019: error: parameter name omitted include/linux/perf_event.h: In function 'perf_unregister_guest_info_callbacks':
include/linux/perf_event.h:1021: error: parameter name omitted make[1]: *** [init/main.o] Error 1
make: *** [init] Error 2

Avi Kivity

2010-04-23 10:50 UTC #14 lore raw

Quoted 8 lines · On 04/22/2010 01:27 PM, Liu Yu-B13201 wrote:

I met this error when built kernel. Anything wrong?
CC init/main.o
In file included from include/linux/ftrace_event.h:8,
                  from include/trace/syscall.h:6,
                  from include/linux/syscalls.h:75,
                  from init/main.c:16:

Quoted 7 lines

include/linux/perf_event.h: In function 'perf_register_guest_info_callbacks':
include/linux/perf_event.h:1019: error: parameter name omitted include/linux/perf_event.h: In function 'perf_unregister_guest_info_callbacks':
include/linux/perf_event.h:1021: error: parameter name omitted make[1]: *** [init/main.o] Error 1
make: *** [init] Error 2

I merged tip/perf/code which may fix this. Find it in kvm.git next branch.

Signature · 1 line

Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Zhang, Yanmin

author

2010-04-26 06:11 UTC #15 lore raw

Quoted 17 lines · Avi Kivity

Quoted 8 lines · On 04/22/2010 01:27 PM, Liu Yu-B13201 wrote:
I met this error when built kernel. Anything wrong?
CC init/main.o
In file included from include/linux/ftrace_event.h:8,
                  from include/trace/syscall.h:6,
                  from include/linux/syscalls.h:75,
                  from init/main.c:16:
Quoted 7 lines
include/linux/perf_event.h: In function 'perf_register_guest_info_callbacks':
include/linux/perf_event.h:1019: error: parameter name omitted include/linux/perf_event.h: In function 'perf_unregister_guest_info_callbacks':
include/linux/perf_event.h:1021: error: parameter name omitted make[1]: *** [init/main.o] Error 1
make: *** [init] Error 2

Quoted 1 line

I merged tip/perf/code which may fix this. Find it in kvm.git next branch.

I downloaded the latest kvm.git tree and compilation is ok, including both enabling FTRACE and disabling FTRACE.

Yanmin