KVM: fix tracing kvm_try_async_get_page

PATCH Superseded Xiao Guangrong 2 participants 23 messages Oct 27, 2010 – Oct 28, 2010

Subsystems (4)

x86 KVM Tracing KVM x86

lore

Share & tools

lore

Xiao Guangrong

author

2010-10-27 08:57 UTC #0 lore raw

PATCH 1/8 KVM: fix tracing kvm_try_async_get_page

Tracing 'async' and *pfn is useless, since 'async' is always true, and '*pfn' is always "fault_pfn'

We can trace 'gva' and 'gfn' instead, it can help us to see the life-cycle of an async_pf

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/kvm/mmu.c         |    2 +-
 include/trace/events/kvm.h |   12 +++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

arch/x86/kvm/mmu.c +1 -1

index 7ab734d..7449803 100644
@@ -2618,7 +2618,7 @@ static bool try_async_pf(struct kvm_vcpu *vcpu, bool no_apf, gfn_t gfn,
 	put_page(pfn_to_page(*pfn));
 
 	if (!no_apf && can_do_async_pf(vcpu)) {
-		trace_kvm_try_async_get_page(async, *pfn);
+		trace_kvm_try_async_get_page(gva, gfn);
 		if (kvm_find_async_pf_gfn(vcpu, gfn)) {
 			trace_kvm_async_pf_doublefault(gva, gfn);
 			kvm_make_request(KVM_REQ_APF_HALT, vcpu);

include/trace/events/kvm.h +7 -5

index 9c2cc6a..30063c6 100644
@@ -188,18 +188,20 @@ TRACE_EVENT(kvm_age_page,
 #ifdef CONFIG_KVM_ASYNC_PF
 TRACE_EVENT(
 	kvm_try_async_get_page,
-	TP_PROTO(bool async, u64 pfn),
-	TP_ARGS(async, pfn),
+	TP_PROTO(u64 gva, u64 gfn),
+	TP_ARGS(gva, gfn),
 
 	TP_STRUCT__entry(
-		__field(__u64, pfn)
+		__field(u64, gva)
+		__field(u64, gfn)
 		),
 
 	TP_fast_assign(
-		__entry->pfn = (!async) ? pfn : (u64)-1;
+		__entry->gva = gva;
+		__entry->gfn = gfn;
 		),
 
-	TP_printk("pfn %#llx", __entry->pfn)
+	TP_printk("gva = %#llx, gfn = %#llx", __entry->gva, __entry->gfn)
 );
 
 TRACE_EVENT(

Signature · 1 line

1.7.0.4

Xiao Guangrong

author

2010-10-27 08:58 UTC #1 lore raw

PATCH 2/8 KVM: cleanup aysnc_pf tracepoints

Use 'DECLARE_EVENT_CLASS' to cleanup async_pf tracepoints

Signed-off-by Xiao Guangrong <[email protected]>

---
 include/trace/events/kvm.h |   76 ++++++++++++++++++++-----------------------
 1 files changed, 35 insertions(+), 41 deletions(-)

include/trace/events/kvm.h +35 ~~-41~~

index 30063c6..7bec396 100644
@@ -186,59 +186,71 @@ TRACE_EVENT(kvm_age_page,
 );
 
 #ifdef CONFIG_KVM_ASYNC_PF
-TRACE_EVENT(
-	kvm_try_async_get_page,
+DECLARE_EVENT_CLASS(kvm_async_get_page_class,
+
 	TP_PROTO(u64 gva, u64 gfn),
+
 	TP_ARGS(gva, gfn),
 
 	TP_STRUCT__entry(
-		__field(u64, gva)
+		__field(__u64, gva)
 		__field(u64, gfn)
-		),
+	),
 
 	TP_fast_assign(
 		__entry->gva = gva;
 		__entry->gfn = gfn;
-		),
+	),
 
 	TP_printk("gva = %#llx, gfn = %#llx", __entry->gva, __entry->gfn)
 );
 
-TRACE_EVENT(
-	kvm_async_pf_not_present,
+DEFINE_EVENT(kvm_async_get_page_class, kvm_try_async_get_page,
+
+	TP_PROTO(u64 gva, u64 gfn),
+
+	TP_ARGS(gva, gfn)
+);
+
+DEFINE_EVENT(kvm_async_get_page_class, kvm_async_pf_doublefault,
+
+	TP_PROTO(u64 gva, u64 gfn),
+
+	TP_ARGS(gva, gfn)
+);
+
+DECLARE_EVENT_CLASS(kvm_async_pf_nopresent_ready,
+
 	TP_PROTO(u64 token, u64 gva),
+
 	TP_ARGS(token, gva),
 
 	TP_STRUCT__entry(
 		__field(__u64, token)
 		__field(__u64, gva)
-		),
+	),
 
 	TP_fast_assign(
 		__entry->token = token;
 		__entry->gva = gva;
-		),
+	),
+
+	TP_printk("token %#llx gva %#llx", __entry->token, __entry->gva)
 
-	TP_printk("token %#llx gva %#llx not present", __entry->token,
-		  __entry->gva)
 );
 
-TRACE_EVENT(
-	kvm_async_pf_ready,
+DEFINE_EVENT(kvm_async_pf_nopresent_ready, kvm_async_pf_not_present,
+
 	TP_PROTO(u64 token, u64 gva),
-	TP_ARGS(token, gva),
 
-	TP_STRUCT__entry(
-		__field(__u64, token)
-		__field(__u64, gva)
-		),
+	TP_ARGS(token, gva)
+);
 
-	TP_fast_assign(
-		__entry->token = token;
-		__entry->gva = gva;
-		),
+DEFINE_EVENT(kvm_async_pf_nopresent_ready, kvm_async_pf_ready,
+
+	TP_PROTO(u64 token, u64 gva),
 
-	TP_printk("token %#llx gva %#llx ready", __entry->token, __entry->gva)
+	TP_ARGS(token, gva)
 );
 
 TRACE_EVENT(

@@ -262,24 +274,6 @@ TRACE_EVENT(
 		  __entry->address, __entry->pfn)
 );
 
-TRACE_EVENT(
-	kvm_async_pf_doublefault,
-	TP_PROTO(u64 gva, u64 gfn),
-	TP_ARGS(gva, gfn),
-
-	TP_STRUCT__entry(
-		__field(u64, gva)
-		__field(u64, gfn)
-		),
-
-	TP_fast_assign(
-		__entry->gva = gva;
-		__entry->gfn = gfn;
-		),
-
-	TP_printk("gva = %#llx, gfn = %#llx", __entry->gva, __entry->gfn)
-);
-
 #endif
 
 #endif /* _TRACE_KVM_MAIN_H */

Signature · 1 line

1.7.0.4

Xiao Guangrong

author

2010-10-27 08:59 UTC #3 lore raw

PATCH 3/8 KVM: fix searching async gfn in kvm_async_pf_gfn_slot

In reply to Xiao Guangrong

The 'gfn' is not recorded if the next slot is empty

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/kvm/x86.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

arch/x86/kvm/x86.c +2 -2

index 7876ec8..16f42ff 100644
@@ -6207,8 +6207,8 @@ static u32 kvm_async_pf_gfn_slot(struct kvm_vcpu *vcpu, gfn_t gfn)
 	u32 key = kvm_async_pf_hash_fn(gfn);
 
 	for (i = 0; i < roundup_pow_of_two(ASYNC_PF_PER_VCPU) &&
-		     (vcpu->arch.apf.gfns[key] != gfn ||
-		      vcpu->arch.apf.gfns[key] == ~0); i++)
+		     (vcpu->arch.apf.gfns[key] != gfn &&
+		      vcpu->arch.apf.gfns[key] != ~0); i++)
 		key = kvm_async_pf_next_probe(key);
 
 	return key;

Signature · 1 line

1.7.0.4

Xiao Guangrong

author

2010-10-27 09:00 UTC #5 lore raw

PATCH 4/8 KVM: avoid unnecessary wait for a async pf

In reply to Xiao Guangrong

In current code, it checks async pf completion out of the wait context, like this:

if (vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE &&

		    !vcpu->arch.apf.halted)
			r = vcpu_enter_guest(vcpu);
		else {
			......
			kvm_vcpu_block(vcpu)
			 ^- waiting until 'async_pf.done' is not empty
}

kvm_check_async_pf_completion(vcpu)
^- delete list from async_pf.done

So, if we check aysnc pf completion first, it can be blocked at kvm_vcpu_block

Fixed by move the check into wait context

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/kvm/x86.c       |    3 ---
 include/linux/kvm_host.h |    1 -
 virt/kvm/async_pf.c      |    6 ++++--
 virt/kvm/async_pf.h      |    6 ++++++
 virt/kvm/kvm_main.c      |    3 ++-
 5 files changed, 12 insertions(+), 7 deletions(-)

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

index 16f42ff..0b2c420 100644
@@ -5321,8 +5321,6 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
 			vcpu->run->exit_reason = KVM_EXIT_INTR;
 			++vcpu->stat.request_irq_exits;
 		}
-		
-		kvm_check_async_pf_completion(vcpu);
 
 		if (signal_pending(current)) {
 			r = -EINTR;

@@ -6108,7 +6106,6 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
 {
 	return (vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE &&
 		!vcpu->arch.apf.halted)
-		|| !list_empty_careful(&vcpu->async_pf.done)
 		|| vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED
 		|| vcpu->arch.nmi_pending ||
 		(kvm_arch_interrupt_allowed(vcpu) &&

include/linux/kvm_host.h +0 -1

index ee4314e..0c1b7c5 100644
@@ -90,7 +90,6 @@ struct kvm_async_pf {
 };
 
 void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu);
-void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu);
 int kvm_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn,
 		       struct kvm_arch_async_pf *arch);
 int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);

virt/kvm/async_pf.c +4 -2

index 60df9e0..e213ca4 100644
@@ -120,13 +120,13 @@ void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu)
 	vcpu->async_pf.queued = 0;
 }
 
-void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
+bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
 {
 	struct kvm_async_pf *work;
 
 	if (list_empty_careful(&vcpu->async_pf.done) ||
 	    !kvm_arch_can_inject_async_page_present(vcpu))
-		return;
+		return false;
 
 	spin_lock(&vcpu->async_pf.lock);
 	work = list_first_entry(&vcpu->async_pf.done, typeof(*work), link);

@@ -142,6 +142,8 @@ void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
 	if (work->page)
 		put_page(work->page);
 	kmem_cache_free(async_pf_cache, work);
+
+	return true;
 }
 
 int kvm_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn,

virt/kvm/async_pf.h +6 -0

index e7ef644..e21533c 100644
@@ -27,10 +27,16 @@
 int kvm_async_pf_init(void);
 void kvm_async_pf_deinit(void);
 void kvm_async_pf_vcpu_init(struct kvm_vcpu *vcpu);
+bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu);
+
 #else
 #define kvm_async_pf_init() (0)
 #define kvm_async_pf_deinit() do{}while(0)
 #define kvm_async_pf_vcpu_init(C) do{}while(0)
+static inline bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
+{
+	return false;
+}
 #endif
 
 #endif

virt/kvm/kvm_main.c +2 -1

index 88d869e..d9aed28 100644
@@ -1347,7 +1347,8 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
 	for (;;) {
 		prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
 
-		if (kvm_arch_vcpu_runnable(vcpu)) {
+		if (kvm_arch_vcpu_runnable(vcpu) ||
+		     kvm_check_async_pf_completion(vcpu)) {
 			kvm_make_request(KVM_REQ_UNHALT, vcpu);
 			break;
 		}

Signature · 1 line

1.7.0.4

Gleb Natapov

2010-10-27 10:42 UTC #6 lore raw

Re: [PATCH 4/8] KVM: avoid unnecessary wait for a async pf

Quoted 12 lines · Xiao Guangrong

In current code, it checks async pf completion out of the wait context, like this:
if (vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE &&
		    !vcpu->arch.apf.halted)
			r = vcpu_enter_guest(vcpu);
		else {
			......
			kvm_vcpu_block(vcpu)
			 ^- waiting until 'async_pf.done' is not empty
}

Quoted 6 lines

kvm_check_async_pf_completion(vcpu)
^- delete list from async_pf.done
So, if we check aysnc pf completion first, it can be blocked at kvm_vcpu_block

Correct, but it can be fixed by adding vcpu->arch.apf.halted = false; to kvm_arch_async_page_present(), no?
Adding kvm_check_async_pf_completion() to arch independent kvm_vcpu_block() constrains how other archs may implement async pf support IMO.

Quoted 115 lines

Fixed by move the check into wait context

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/kvm/x86.c       |    3 ---
 include/linux/kvm_host.h |    1 -
 virt/kvm/async_pf.c      |    6 ++++--
 virt/kvm/async_pf.h      |    6 ++++++
 virt/kvm/kvm_main.c      |    3 ++-
 5 files changed, 12 insertions(+), 7 deletions(-)

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

index 16f42ff..0b2c420 100644
@@ -5321,8 +5321,6 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
 			vcpu->run->exit_reason = KVM_EXIT_INTR;
 			++vcpu->stat.request_irq_exits;
 		}
-		
-		kvm_check_async_pf_completion(vcpu);
 
 		if (signal_pending(current)) {
 			r = -EINTR;

@@ -6108,7 +6106,6 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
 {
 	return (vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE &&
 		!vcpu->arch.apf.halted)
-		|| !list_empty_careful(&vcpu->async_pf.done)
 		|| vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED
 		|| vcpu->arch.nmi_pending ||
 		(kvm_arch_interrupt_allowed(vcpu) &&

include/linux/kvm_host.h +0 -1

index ee4314e..0c1b7c5 100644
@@ -90,7 +90,6 @@ struct kvm_async_pf {
 };
 
 void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu);
-void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu);
 int kvm_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn,
 		       struct kvm_arch_async_pf *arch);
 int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);

virt/kvm/async_pf.c +4 -2

index 60df9e0..e213ca4 100644
@@ -120,13 +120,13 @@ void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu)
 	vcpu->async_pf.queued = 0;
 }
 
-void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
+bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
 {
 	struct kvm_async_pf *work;
 
 	if (list_empty_careful(&vcpu->async_pf.done) ||
 	    !kvm_arch_can_inject_async_page_present(vcpu))
-		return;
+		return false;
 
 	spin_lock(&vcpu->async_pf.lock);
 	work = list_first_entry(&vcpu->async_pf.done, typeof(*work), link);

@@ -142,6 +142,8 @@ void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
 	if (work->page)
 		put_page(work->page);
 	kmem_cache_free(async_pf_cache, work);
+
+	return true;
 }
 
 int kvm_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn,

virt/kvm/async_pf.h +6 -0

index e7ef644..e21533c 100644
@@ -27,10 +27,16 @@
 int kvm_async_pf_init(void);
 void kvm_async_pf_deinit(void);
 void kvm_async_pf_vcpu_init(struct kvm_vcpu *vcpu);
+bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu);
+
 #else
 #define kvm_async_pf_init() (0)
 #define kvm_async_pf_deinit() do{}while(0)
 #define kvm_async_pf_vcpu_init(C) do{}while(0)
+static inline bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
+{
+	return false;
+}
 #endif
 
 #endif

virt/kvm/kvm_main.c +2 -1

index 88d869e..d9aed28 100644
@@ -1347,7 +1347,8 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
 	for (;;) {
 		prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
 
-		if (kvm_arch_vcpu_runnable(vcpu)) {
+		if (kvm_arch_vcpu_runnable(vcpu) ||
+		     kvm_check_async_pf_completion(vcpu)) {
 			kvm_make_request(KVM_REQ_UNHALT, vcpu);
 			break;
 		}

Signature · 6 lines

1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Gleb.

Xiao Guangrong

author

2010-10-28 07:02 UTC #7 lore raw

Re: [PATCH 4/8] KVM: avoid unnecessary wait for a async pf

Quoted 16 lines · Gleb Natapov

Quoted 15 lines · Xiao Guangrong
In current code, it checks async pf completion out of the wait context, like this:
if (vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE &&
		    !vcpu->arch.apf.halted)
			r = vcpu_enter_guest(vcpu);
		else {
			......
			kvm_vcpu_block(vcpu)
			 ^- waiting until 'async_pf.done' is not empty
}
kvm_check_async_pf_completion(vcpu)
^- delete list from async_pf.done

Quoted 8 lines

Quoted 3 lines
So, if we check aysnc pf completion first, it can be blocked at kvm_vcpu_block
Correct, but it can be fixed by adding vcpu->arch.apf.halted = false; to kvm_arch_async_page_present(), no?
Adding kvm_check_async_pf_completion() to arch independent kvm_vcpu_block() constrains how other archs may implement async pf support IMO.

Um, i think it's reasonable, will fix it address your comment.

Xiao Guangrong

author

2010-10-27 09:01 UTC #8 lore raw

PATCH 5/8 KVM: don't touch vcpu stat after async pf is complete

In reply to Xiao Guangrong

Don't make a KVM_REQ_UNHALT request after async pf is completed since it can break guest's 'halt' instruction.

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/x86.c              |    5 +++++
 virt/kvm/async_pf.c             |    1 +
 virt/kvm/kvm_main.c             |    7 +++++--
 4 files changed, 12 insertions(+), 2 deletions(-)

arch/x86/include/asm/kvm_host.h +1 -0

index 1be0058..d01677b 100644
@@ -822,6 +822,7 @@ void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
 				 struct kvm_async_pf *work);
 void kvm_arch_async_page_ready(struct kvm_vcpu *vcpu,
 			       struct kvm_async_pf *work);
+void kvm_arch_async_pf_completion(struct kvm_vcpu *vcpu);
 bool kvm_arch_can_inject_async_page_present(struct kvm_vcpu *vcpu);
 extern bool kvm_find_async_pf_gfn(struct kvm_vcpu *vcpu, gfn_t gfn);

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

index 0b2c420..c0e7ad0 100644
@@ -6280,6 +6280,11 @@ void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
 	}
 }
 
+void kvm_arch_async_pf_completion(struct kvm_vcpu *vcpu)
+{
+	vcpu->arch.apf.halted = false;
+}
+
 bool kvm_arch_can_inject_async_page_present(struct kvm_vcpu *vcpu)
 {
 	if (!(vcpu->arch.apf.msr_val & KVM_ASYNC_PF_ENABLED))

virt/kvm/async_pf.c +1 -0

index e213ca4..5307a32 100644
@@ -142,6 +142,7 @@ bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
 	if (work->page)
 		put_page(work->page);
 	kmem_cache_free(async_pf_cache, work);
+	kvm_arch_async_pf_completion(vcpu);
 
 	return true;
 }

virt/kvm/kvm_main.c +5 -2

index d9aed28..23a8b06 100644
@@ -1347,11 +1347,14 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
 	for (;;) {
 		prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
 
-		if (kvm_arch_vcpu_runnable(vcpu) ||
-		     kvm_check_async_pf_completion(vcpu)) {
+		if (kvm_arch_vcpu_runnable(vcpu)) {
 			kvm_make_request(KVM_REQ_UNHALT, vcpu);
 			break;
 		}
+
+		if (kvm_check_async_pf_completion(vcpu))
+			break;
+
 		if (kvm_cpu_has_pending_timer(vcpu))
 			break;
 		if (signal_pending(current))

Signature · 1 line

1.7.0.4

Gleb Natapov

2010-10-27 10:44 UTC #9 lore raw

Re: [PATCH 5/8] KVM: don't touch vcpu stat after async pf is complete

Quoted 3 lines · Xiao Guangrong

Don't make a KVM_REQ_UNHALT request after async pf is completed since it can break guest's 'halt' instruction.

Why is it a problem? CPU may be unhalted by different events so OS shouldn't depend on it.

Quoted 76 lines

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/x86.c              |    5 +++++
 virt/kvm/async_pf.c             |    1 +
 virt/kvm/kvm_main.c             |    7 +++++--
 4 files changed, 12 insertions(+), 2 deletions(-)

arch/x86/include/asm/kvm_host.h +1 -0

index 1be0058..d01677b 100644
@@ -822,6 +822,7 @@ void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
 				 struct kvm_async_pf *work);
 void kvm_arch_async_page_ready(struct kvm_vcpu *vcpu,
 			       struct kvm_async_pf *work);
+void kvm_arch_async_pf_completion(struct kvm_vcpu *vcpu);
 bool kvm_arch_can_inject_async_page_present(struct kvm_vcpu *vcpu);
 extern bool kvm_find_async_pf_gfn(struct kvm_vcpu *vcpu, gfn_t gfn);

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

index 0b2c420..c0e7ad0 100644
@@ -6280,6 +6280,11 @@ void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
 	}
 }
 
+void kvm_arch_async_pf_completion(struct kvm_vcpu *vcpu)
+{
+	vcpu->arch.apf.halted = false;
+}
+
 bool kvm_arch_can_inject_async_page_present(struct kvm_vcpu *vcpu)
 {
 	if (!(vcpu->arch.apf.msr_val & KVM_ASYNC_PF_ENABLED))

virt/kvm/async_pf.c +1 -0

index e213ca4..5307a32 100644
@@ -142,6 +142,7 @@ bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
 	if (work->page)
 		put_page(work->page);
 	kmem_cache_free(async_pf_cache, work);
+	kvm_arch_async_pf_completion(vcpu);
 
 	return true;
 }

virt/kvm/kvm_main.c +5 -2

index d9aed28..23a8b06 100644
@@ -1347,11 +1347,14 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
 	for (;;) {
 		prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
 
-		if (kvm_arch_vcpu_runnable(vcpu) ||
-		     kvm_check_async_pf_completion(vcpu)) {
+		if (kvm_arch_vcpu_runnable(vcpu)) {
 			kvm_make_request(KVM_REQ_UNHALT, vcpu);
 			break;
 		}
+
+		if (kvm_check_async_pf_completion(vcpu))
+			break;
+
 		if (kvm_cpu_has_pending_timer(vcpu))
 			break;
 		if (signal_pending(current))

Signature · 6 lines

1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Gleb.

Xiao Guangrong

author

2010-10-28 07:30 UTC #10 lore raw

Re: [PATCH 5/8] KVM: don't touch vcpu stat after async pf is complete

Quoted 7 lines · Gleb Natapov

Quoted 3 lines · Xiao Guangrong
Don't make a KVM_REQ_UNHALT request after async pf is completed since it can break guest's 'halt' instruction.
Why is it a problem? CPU may be unhalted by different events so OS shouldn't depend on it.

We don't know how guest OS handles it after HLT instruction is completed, according to X86's spec, only NMI/INTR/RESET/INIT/SMI can break halt state, it violations the hardware behavior if we allow other event break this state. Your opinion? :-)

Gleb Natapov

2010-10-28 07:41 UTC #11 lore raw

Re: [PATCH 5/8] KVM: don't touch vcpu stat after async pf is complete

Quoted 5 lines · Xiao Guangrong

Quoted 4 lines · Gleb Natapov
Quoted 3 lines · Xiao Guangrong
Don't make a KVM_REQ_UNHALT request after async pf is completed since it can break guest's 'halt' instruction.

Quoted 8 lines

Quoted 3 lines
Why is it a problem? CPU may be unhalted by different events so OS shouldn't depend on it.
We don't know how guest OS handles it after HLT instruction is completed, according to X86's spec, only NMI/INTR/RESET/INIT/SMI can break halt state, it violations the hardware behavior if we allow other event break this state. Your opinion? :-)

I agree in principle, but since SMI (which is completely out of guest OS control) can cause CPU to exit halt, in practice OS can't rely on CPU to be unhalted only by events controlled by OS itself. In the past we had a bug that any timer even unhalted vcpu even when timer interrupt was masked. The only practical problem it caused was that vcpu that executed cli; 1: hlt; jmp 1b sequence still consumed host cpu time. That said I am not against fixing it if the fix is easy. Your current fix though relies on patch 4 that I have problem with.

--
Gleb.

Xiao Guangrong

author

2010-10-27 09:03 UTC #12 lore raw

PATCH 6/8 KVM: simply wakup async pf

In reply to Xiao Guangrong

The current way is queued a complete async pf with:
asyc_pf.page = bad_page async_pf.arch.gfn = 0

It has two problems while kvm_check_async_pf_completion handle this async_pf:
- since !async_pf.page, it can retry a pseudo #PF
- it can delete gfn 0 from vcpu->arch.apf.gfns[]

Actually, we can simply record this wakeup request and let kvm_check_async_pf_completion simply break the wait

Signed-off-by Xiao Guangrong <[email protected]>

---
 include/linux/kvm_host.h |    1 +
 virt/kvm/async_pf.c      |   21 ++++++---------------
 2 files changed, 7 insertions(+), 15 deletions(-)

include/linux/kvm_host.h +1 -0

index 0c1b7c5..d91add9 100644
@@ -131,6 +131,7 @@ struct kvm_vcpu {
 		struct list_head queue;
 		struct list_head done;
 		spinlock_t lock;
+		bool wakeup;
 	} async_pf;
 #endif

virt/kvm/async_pf.c +6 ~~-15~~

index 5307a32..0d1f6c4 100644
@@ -124,6 +124,11 @@ bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
 {
 	struct kvm_async_pf *work;
 
+	if (vcpu->async_pf.wakeup) {
+		vcpu->async_pf.wakeup = false;
+		return true;
+	}
+
 	if (list_empty_careful(&vcpu->async_pf.done) ||
 	    !kvm_arch_can_inject_async_page_present(vcpu))
 		return false;

@@ -197,20 +202,6 @@ retry_sync:
 
 int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu)
 {
-	struct kvm_async_pf *work;
-
-	if (!list_empty(&vcpu->async_pf.done))
-		return 0;
-
-	work = kmem_cache_zalloc(async_pf_cache, GFP_ATOMIC);
-	if (!work)
-		return -ENOMEM;
-
-	work->page = bad_page;
-	get_page(bad_page);
-	INIT_LIST_HEAD(&work->queue); /* for list_del to work */
-
-	list_add_tail(&work->link, &vcpu->async_pf.done);
-	vcpu->async_pf.queued++;
+	vcpu->async_pf.wakeup = true;
 	return 0;
 }

Signature · 1 line

1.7.0.4

Gleb Natapov

2010-10-27 10:51 UTC #13 lore raw

Re: [PATCH 6/8] KVM: simply wakup async pf

Quoted 7 lines · Xiao Guangrong

The current way is queued a complete async pf with:
asyc_pf.page = bad_page async_pf.arch.gfn = 0
It has two problems while kvm_check_async_pf_completion handle this async_pf:
- since !async_pf.page, it can retry a pseudo #PF

kvm_arch_async_page_ready checks for is_error_page()

Quoted 1 line

- it can delete gfn 0 from vcpu->arch.apf.gfns[]

kvm_arch_async_page_present() checks for is_error_page() too and, in case of PV guest, injects special token if it is true.

After your patch special token will not be injected and migration will not work.

Quoted 3 lines

Actually, we can simply record this wakeup request and let kvm_check_async_pf_completion simply break the wait

May be wakeup_all function naming is misleading. It means wake up all PV guest processes by sending broadcast async pf notification. It is not about waking host vcpu thread.

Quoted 63 lines

Signed-off-by Xiao Guangrong <[email protected]>

---
 include/linux/kvm_host.h |    1 +
 virt/kvm/async_pf.c      |   21 ++++++---------------
 2 files changed, 7 insertions(+), 15 deletions(-)

include/linux/kvm_host.h +1 -0

index 0c1b7c5..d91add9 100644
@@ -131,6 +131,7 @@ struct kvm_vcpu {
 		struct list_head queue;
 		struct list_head done;
 		spinlock_t lock;
+		bool wakeup;
 	} async_pf;
 #endif

virt/kvm/async_pf.c +6 ~~-15~~

index 5307a32..0d1f6c4 100644
@@ -124,6 +124,11 @@ bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
 {
 	struct kvm_async_pf *work;
 
+	if (vcpu->async_pf.wakeup) {
+		vcpu->async_pf.wakeup = false;
+		return true;
+	}
+
 	if (list_empty_careful(&vcpu->async_pf.done) ||
 	    !kvm_arch_can_inject_async_page_present(vcpu))
 		return false;

@@ -197,20 +202,6 @@ retry_sync:
 
 int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu)
 {
-	struct kvm_async_pf *work;
-
-	if (!list_empty(&vcpu->async_pf.done))
-		return 0;
-
-	work = kmem_cache_zalloc(async_pf_cache, GFP_ATOMIC);
-	if (!work)
-		return -ENOMEM;
-
-	work->page = bad_page;
-	get_page(bad_page);
-	INIT_LIST_HEAD(&work->queue); /* for list_del to work */
-
-	list_add_tail(&work->link, &vcpu->async_pf.done);
-	vcpu->async_pf.queued++;
+	vcpu->async_pf.wakeup = true;
 	return 0;
 }

Signature · 6 lines

1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Gleb.

Xiao Guangrong

author

2010-10-28 07:55 UTC #14 lore raw

Re: [PATCH 6/8] KVM: simply wakup async pf

Quoted 5 lines · Gleb Natapov

Quoted 4 lines · Xiao Guangrong
The current way is queued a complete async pf with:
asyc_pf.page = bad_page async_pf.arch.gfn = 0

Quoted 9 lines

Quoted 3 lines
It has two problems while kvm_check_async_pf_completion handle this async_pf:
- since !async_pf.page, it can retry a pseudo #PF
kvm_arch_async_page_ready checks for is_error_page()
Quoted 1 line
- it can delete gfn 0 from vcpu->arch.apf.gfns[]
kvm_arch_async_page_present() checks for is_error_page() too and, in case of PV guest, injects special token if it is true.

Ah, sorry for my stupid questions.

Quoted 10 lines

After your patch special token will not be injected and migration will not work.
Quoted 3 lines
Actually, we can simply record this wakeup request and let kvm_check_async_pf_completion simply break the wait
May be wakeup_all function naming is misleading. It means wake up all PV guest processes by sending broadcast async pf notification. It is not about waking host vcpu thread.

I'm not good at the KVM PV way, i'll dig into it, please ignore this patch, thanks.

Xiao Guangrong

author

2010-10-27 09:05 UTC #15 lore raw

PATCH 7/8 KVM: make async_pf work queue lockless

In reply to Xiao Guangrong

The async_pf number is very few since only pending interrupt can let it re-enter to the guest mode.

During my test(Host 4 CPU + 4G, Guest 4 VCPU + 6G), it's no more than 10 requests in the system.

So, we can only increase the completion counter in the work queue context, and walk vcpu->async_pf.queue list to get all completed async_pf

Signed-off-by Xiao Guangrong <[email protected]>

---
 include/linux/kvm_host.h |    4 +--
 virt/kvm/async_pf.c      |   50 +++++++++++++++++++--------------------------
 2 files changed, 22 insertions(+), 32 deletions(-)

include/linux/kvm_host.h +1 -3

index d91add9..33c03c3 100644
@@ -78,7 +78,6 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
 #ifdef CONFIG_KVM_ASYNC_PF
 struct kvm_async_pf {
 	struct work_struct work;
-	struct list_head link;
 	struct list_head queue;
 	struct kvm_vcpu *vcpu;
 	struct mm_struct *mm;

@@ -127,10 +126,9 @@ struct kvm_vcpu {
 
 #ifdef CONFIG_KVM_ASYNC_PF
 	struct {
+		atomic_t done;
 		u32 queued;
 		struct list_head queue;
-		struct list_head done;
-		spinlock_t lock;
 		bool wakeup;
 	} async_pf;
 #endif

virt/kvm/async_pf.c +21 ~~-29~~

93 lines not on this page · raw message

Signature · 1 line

1.7.0.4

Gleb Natapov

2010-10-27 11:41 UTC #16 lore raw

Re: [PATCH 7/8] KVM: make async_pf work queue lockless

Quoted 10 lines · Xiao Guangrong

The async_pf number is very few since only pending interrupt can let it re-enter to the guest mode.
During my test(Host 4 CPU + 4G, Guest 4 VCPU + 6G), it's no more than 10 requests in the system.
So, we can only increase the completion counter in the work queue context, and walk vcpu->async_pf.queue list to get all completed async_pf

That depends on the load. I used memory cgroups to create very big memory pressure and I saw hundreds of apfs per second. We shouldn't optimize for very low numbers. With vcpu->async_pf.queue having more then one element I am not sure your patch is beneficial.

Quoted 110 lines

Signed-off-by Xiao Guangrong <[email protected]>

---
 include/linux/kvm_host.h |    4 +--
 virt/kvm/async_pf.c      |   50 +++++++++++++++++++--------------------------
 2 files changed, 22 insertions(+), 32 deletions(-)

include/linux/kvm_host.h +1 -3

index d91add9..33c03c3 100644
@@ -78,7 +78,6 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
 #ifdef CONFIG_KVM_ASYNC_PF
 struct kvm_async_pf {
 	struct work_struct work;
-	struct list_head link;
 	struct list_head queue;
 	struct kvm_vcpu *vcpu;
 	struct mm_struct *mm;

@@ -127,10 +126,9 @@ struct kvm_vcpu {
 
 #ifdef CONFIG_KVM_ASYNC_PF
 	struct {
+		atomic_t done;
 		u32 queued;
 		struct list_head queue;
-		struct list_head done;
-		spinlock_t lock;
 		bool wakeup;
 	} async_pf;
 #endif

virt/kvm/async_pf.c +10 ~~-24~~

index 0d1f6c4..f10de1e 100644
@@ -49,9 +49,7 @@ void kvm_async_pf_deinit(void)
 
 void kvm_async_pf_vcpu_init(struct kvm_vcpu *vcpu)
 {
-	INIT_LIST_HEAD(&vcpu->async_pf.done);
 	INIT_LIST_HEAD(&vcpu->async_pf.queue);
-	spin_lock_init(&vcpu->async_pf.lock);
 }
 
 static void async_pf_execute(struct work_struct *work)

@@ -72,11 +70,9 @@ static void async_pf_execute(struct work_struct *work)
 	up_read(&mm->mmap_sem);
 	unuse_mm(mm);
 
-	spin_lock(&vcpu->async_pf.lock);
-	list_add_tail(&apf->link, &vcpu->async_pf.done);
 	apf->page = page;
 	apf->done = true;
-	spin_unlock(&vcpu->async_pf.lock);
+	atomic_inc(&vcpu->async_pf.done);
 
 	/*
 	 * apf may be freed by kvm_check_async_pf_completion() after

@@ -101,52 +97,48 @@ void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu)
 				   typeof(*work), queue);
 		cancel_work_sync(&work->work);
 		list_del(&work->queue);
-		if (!work->done) /* work was canceled */
-			kmem_cache_free(async_pf_cache, work);
-	}
-
-	spin_lock(&vcpu->async_pf.lock);
-	while (!list_empty(&vcpu->async_pf.done)) {
-		struct kvm_async_pf *work =
-			list_entry(vcpu->async_pf.done.next,
-				   typeof(*work), link);
-		list_del(&work->link);
 		if (work->page)
 			put_page(work->page);
 		kmem_cache_free(async_pf_cache, work);
 	}
-	spin_unlock(&vcpu->async_pf.lock);
 
 	vcpu->async_pf.queued = 0;
+	atomic_set(&vcpu->async_pf.done, 0);
 }
 
 bool kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
 {
 	struct kvm_async_pf *work;
+	struct list_head *pos, *temp;
 
 	if (vcpu->async_pf.wakeup) {
 		vcpu->async_pf.wakeup = false;
 		return true;
 	}
 
-	if (list_empty_careful(&vcpu->async_pf.done) ||
+	if (!atomic_read(&vcpu->async_pf.done) ||
 	    !kvm_arch_can_inject_async_page_present(vcpu))
 		return false;
 
-	spin_lock(&vcpu->async_pf.lock);
-	work = list_first_entry(&vcpu->async_pf.done, typeof(*work), link);
-	list_del(&work->link);
-	spin_unlock(&vcpu->async_pf.lock);
+	list_for_each_safe(pos, temp, &vcpu->async_pf.queue) {
+		work = list_entry(pos, typeof(*work), queue);
+		if (!work->done)
+			continue;
 
-	if (work->page)
-		kvm_arch_async_page_ready(vcpu, work);
-	kvm_arch_async_page_present(vcpu, work);
+		if (work->page) {
+			kvm_arch_async_page_ready(vcpu, work);

virt/kvm/async_pf.c void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu) · line 139

Quoted 10 lines

virt/kvm/async_pf.c +10 -0

+			put_page(work->page);
+		}
+
+		kvm_arch_async_page_present(vcpu, work);
+
+		list_del(&work->queue);
+		vcpu->async_pf.queued--;
+		kmem_cache_free(async_pf_cache, work);
+		if (atomic_dec_and_test(&vcpu->async_pf.done))
+			break;

You should do atomic_dec() and always break. We cannot inject two apfs during one vcpu entry.

Quoted 18 lines

virt/kvm/async_pf.c +1 -5

+	}
 
-	list_del(&work->queue);
-	vcpu->async_pf.queued--;
-	if (work->page)
-		put_page(work->page);
-	kmem_cache_free(async_pf_cache, work);
 	kvm_arch_async_pf_completion(vcpu);
 
 	return true;

Signature · 7 lines

1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

--
Gleb.

Xiao Guangrong

author

2010-10-28 09:04 UTC #17 lore raw

Re: [PATCH 7/8] KVM: make async_pf work queue lockless

Quoted 7 lines · Gleb Natapov

Quoted 6 lines · Xiao Guangrong
The async_pf number is very few since only pending interrupt can let it re-enter to the guest mode.
During my test(Host 4 CPU + 4G, Guest 4 VCPU + 6G), it's no more than 10 requests in the system.

Quoted 9 lines

Quoted 4 lines
So, we can only increase the completion counter in the work queue context, and walk vcpu->async_pf.queue list to get all completed async_pf
That depends on the load. I used memory cgroups to create very big memory pressure and I saw hundreds of apfs per second. We shouldn't optimize for very low numbers. With vcpu->async_pf.queue having more then one element I am not sure your patch is beneficial.

Maybe we need a new no-lock way to record the complete apfs, i'll reproduce your test environment and improve it.

Quoted 9 lines

virt/kvm/async_pf.c void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu) · line 139
Quoted 6 lines
virt/kvm/async_pf.c +6 -0
+
+		list_del(&work->queue);
+		vcpu->async_pf.queued--;
+		kmem_cache_free(async_pf_cache, work);
+		if (atomic_dec_and_test(&vcpu->async_pf.done))
+			break;
You should do atomic_dec() and always break. We cannot inject two apfs during one vcpu entry.

Sorry, i'm little confused.

Why 'atomic_dec_and_test(&vcpu->async_pf.done)' always break? async_pf.done is used to record the complete apfs and many apfs may be completed when vcpu enters guest mode(it means vcpu->async_pf.done > 1)

Look at the current code:

void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)

{
	......
	spin_lock(&vcpu->async_pf.lock);
	work = list_first_entry(&vcpu->async_pf.done, typeof(*work), link);
	list_del(&work->link);
	spin_unlock(&vcpu->async_pf.lock);
	......
}

You only handle one complete apf, why we inject them at once? I missed something? :-(

Gleb Natapov

2010-10-28 09:14 UTC #18 lore raw

Re: [PATCH 7/8] KVM: make async_pf work queue lockless

Quoted 12 lines · Xiao Guangrong

Quoted 11 lines · Gleb Natapov
Quoted 10 lines · Xiao Guangrong
The async_pf number is very few since only pending interrupt can let it re-enter to the guest mode.
During my test(Host 4 CPU + 4G, Guest 4 VCPU + 6G), it's no more than 10 requests in the system.
So, we can only increase the completion counter in the work queue context, and walk vcpu->async_pf.queue list to get all completed async_pf

Quoted 9 lines

Quoted 5 lines
That depends on the load. I used memory cgroups to create very big memory pressure and I saw hundreds of apfs per second. We shouldn't optimize for very low numbers. With vcpu->async_pf.queue having more then one element I am not sure your patch is beneficial.
Maybe we need a new no-lock way to record the complete apfs, i'll reproduce your test environment and improve it.

That is always welcomed :)

Quoted 10 lines

Quoted 9 lines
virt/kvm/async_pf.c void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu) · line 139
Quoted 6 lines
virt/kvm/async_pf.c +6 -0
+
+		list_del(&work->queue);
+		vcpu->async_pf.queued--;
+		kmem_cache_free(async_pf_cache, work);
+		if (atomic_dec_and_test(&vcpu->async_pf.done))
+			break;
You should do atomic_dec() and always break. We cannot inject two apfs during one vcpu entry.

Quoted 3 lines

Sorry, i'm little confused.
Why 'atomic_dec_and_test(&vcpu->async_pf.done)' always break? async_pf.done is used to

In your code it is not, but it should (at least if guest is PV, read below).

Quoted 3 lines

record the complete apfs and many apfs may be completed when vcpu enters guest mode(it means vcpu->async_pf.done > 1)

Correct, but only one apf should be handled on each vcpu entry in case of PV guest. Look at kvm_arch_async_page_present(vcpu, work); that is called in a loop in your code. If vcpu->arch.apf.msr_val & KVM_ASYNC_PF_ENABLED is not null it injects exception into the guest. You can't inject more then one exception on each guest entry. If guest is not PV you are correct that we can loop here until vcpu->async_pf.done == 0.

Quoted 12 lines

Look at the current code:
void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)
{
	......
	spin_lock(&vcpu->async_pf.lock);
	work = list_first_entry(&vcpu->async_pf.done, typeof(*work), link);
	list_del(&work->link);
	spin_unlock(&vcpu->async_pf.lock);
	......
}

Quoted 1 line

You only handle one complete apf, why we inject them at once? I missed something? :-(

--
Gleb.

Xiao Guangrong

author

2010-10-27 09:06 UTC #19 lore raw

PATCH 8/8 KVM: add debugfs file to show the number of async pf

In reply to Xiao Guangrong

It can help us to see the state of async pf

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/kvm/x86.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

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

index c0e7ad0..2dd8059 100644
@@ -88,6 +88,8 @@ static u64 __read_mostly efer_reserved_bits = 0xfffffffffffffffeULL;
 
 #define VM_STAT(x) offsetof(struct kvm, stat.x), KVM_STAT_VM
 #define VCPU_STAT(x) offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
+#define VCPU_ASYNC_PF_STAT offsetof(struct kvm_vcpu, async_pf.queued), \
+							KVM_STAT_VCPU
 
 static void update_cr8_intercept(struct kvm_vcpu *vcpu);
 static int kvm_dev_ioctl_get_supported_cpuid(struct kvm_cpuid2 *cpuid,

@@ -141,6 +143,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	{ "insn_emulation_fail", VCPU_STAT(insn_emulation_fail) },
 	{ "irq_injections", VCPU_STAT(irq_injections) },
 	{ "nmi_injections", VCPU_STAT(nmi_injections) },
+	{ "async_pf_queued", VCPU_ASYNC_PF_STAT},
 	{ "mmu_shadow_zapped", VM_STAT(mmu_shadow_zapped) },
 	{ "mmu_pte_write", VM_STAT(mmu_pte_write) },
 	{ "mmu_pte_updated", VM_STAT(mmu_pte_updated) },

Signature · 1 line

1.7.0.4

Gleb Natapov

2010-10-27 10:58 UTC #20 lore raw

Re: [PATCH 8/8] KVM: add debugfs file to show the number of async pf

Quoted 2 lines · Xiao Guangrong

It can help us to see the state of async pf

I have patch to add three async pf statistics:
apf_not_present
apf_present
apf_doublefault

But Avi now wants to deprecate debugfs interface completely and move towards ftrace, so I had to drop it.

Quoted 33 lines

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/kvm/x86.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

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

index c0e7ad0..2dd8059 100644
@@ -88,6 +88,8 @@ static u64 __read_mostly efer_reserved_bits = 0xfffffffffffffffeULL;
 
 #define VM_STAT(x) offsetof(struct kvm, stat.x), KVM_STAT_VM
 #define VCPU_STAT(x) offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
+#define VCPU_ASYNC_PF_STAT offsetof(struct kvm_vcpu, async_pf.queued), \
+							KVM_STAT_VCPU
 
 static void update_cr8_intercept(struct kvm_vcpu *vcpu);
 static int kvm_dev_ioctl_get_supported_cpuid(struct kvm_cpuid2 *cpuid,

@@ -141,6 +143,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	{ "insn_emulation_fail", VCPU_STAT(insn_emulation_fail) },
 	{ "irq_injections", VCPU_STAT(irq_injections) },
 	{ "nmi_injections", VCPU_STAT(nmi_injections) },
+	{ "async_pf_queued", VCPU_ASYNC_PF_STAT},
 	{ "mmu_shadow_zapped", VM_STAT(mmu_shadow_zapped) },
 	{ "mmu_pte_write", VM_STAT(mmu_pte_write) },
 	{ "mmu_pte_updated", VM_STAT(mmu_pte_updated) },

Signature · 6 lines

1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Gleb.

Xiao Guangrong

author

2010-10-28 09:05 UTC #21 lore raw

Re: [PATCH 8/8] KVM: add debugfs file to show the number of async pf

Quoted 11 lines · Gleb Natapov

Quoted 2 lines · Xiao Guangrong
It can help us to see the state of async pf
I have patch to add three async pf statistics:
apf_not_present
apf_present
apf_doublefault
But Avi now wants to deprecate debugfs interface completely and move towards ftrace, so I had to drop it.

OK, let's ignore this patch, thanks :-)

Gleb Natapov

2010-10-27 09:59 UTC #22 lore raw

In reply to Xiao Guangrong

Quoted 6 lines · Xiao Guangrong

Tracing 'async' and *pfn is useless, since 'async' is always true, and '*pfn' is always "fault_pfn'
We can trace 'gva' and 'gfn' instead, it can help us to see the life-cycle of an async_pf

Make sense. This is leftover from earlier patches iteration where this trace function was called when async==false too.

Quoted 56 lines

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/kvm/mmu.c         |    2 +-
 include/trace/events/kvm.h |   12 +++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

arch/x86/kvm/mmu.c +1 -1

index 7ab734d..7449803 100644
@@ -2618,7 +2618,7 @@ static bool try_async_pf(struct kvm_vcpu *vcpu, bool no_apf, gfn_t gfn,
 	put_page(pfn_to_page(*pfn));
 
 	if (!no_apf && can_do_async_pf(vcpu)) {
-		trace_kvm_try_async_get_page(async, *pfn);
+		trace_kvm_try_async_get_page(gva, gfn);
 		if (kvm_find_async_pf_gfn(vcpu, gfn)) {
 			trace_kvm_async_pf_doublefault(gva, gfn);
 			kvm_make_request(KVM_REQ_APF_HALT, vcpu);

include/trace/events/kvm.h +7 -5

index 9c2cc6a..30063c6 100644
@@ -188,18 +188,20 @@ TRACE_EVENT(kvm_age_page,
 #ifdef CONFIG_KVM_ASYNC_PF
 TRACE_EVENT(
 	kvm_try_async_get_page,
-	TP_PROTO(bool async, u64 pfn),
-	TP_ARGS(async, pfn),
+	TP_PROTO(u64 gva, u64 gfn),
+	TP_ARGS(gva, gfn),
 
 	TP_STRUCT__entry(
-		__field(__u64, pfn)
+		__field(u64, gva)
+		__field(u64, gfn)
 		),
 
 	TP_fast_assign(
-		__entry->pfn = (!async) ? pfn : (u64)-1;
+		__entry->gva = gva;
+		__entry->gfn = gfn;
 		),
 
-	TP_printk("pfn %#llx", __entry->pfn)
+	TP_printk("gva = %#llx, gfn = %#llx", __entry->gva, __entry->gfn)
 );
 
 TRACE_EVENT(

Signature · 6 lines

1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

--
Gleb.