Message ID | 1618298169-3831-2-git-send-email-wanpengli@tencent.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM: Properly account for guest CPU time | expand |
On 13.04.21 09:16, Wanpeng Li wrote: [...] > @@ -145,6 +155,13 @@ static __always_inline void guest_exit_irqoff(void) > } > > #else > +static __always_inline void context_guest_enter_irqoff(void) > +{ > + instrumentation_begin(); > + rcu_virt_note_context_switch(smp_processor_id()); > + instrumentation_end(); > +} > + > static __always_inline void guest_enter_irqoff(void) > { > /* > @@ -155,10 +172,13 @@ static __always_inline void guest_enter_irqoff(void) > instrumentation_begin(); > vtime_account_kernel(current); > current->flags |= PF_VCPU; > - rcu_virt_note_context_switch(smp_processor_id()); > instrumentation_end(); > + > + context_guest_enter_irqoff(); So we now do instrumentation_begin 2 times?
On Tue, 13 Apr 2021 at 15:35, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > > > > On 13.04.21 09:16, Wanpeng Li wrote: > [...] > > > @@ -145,6 +155,13 @@ static __always_inline void guest_exit_irqoff(void) > > } > > > > #else > > +static __always_inline void context_guest_enter_irqoff(void) > > +{ > > + instrumentation_begin(); > > + rcu_virt_note_context_switch(smp_processor_id()); > > + instrumentation_end(); > > +} > > + > > static __always_inline void guest_enter_irqoff(void) > > { > > /* > > @@ -155,10 +172,13 @@ static __always_inline void guest_enter_irqoff(void) > > instrumentation_begin(); > > vtime_account_kernel(current); > > current->flags |= PF_VCPU; > > - rcu_virt_note_context_switch(smp_processor_id()); > > instrumentation_end(); > > + > > + context_guest_enter_irqoff(); > > So we now do instrumentation_begin 2 times? Similar to context_guest_enter_irqoff() ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN. Wanpeng
On 13.04.21 09:38, Wanpeng Li wrote: > On Tue, 13 Apr 2021 at 15:35, Christian Borntraeger > <borntraeger@de.ibm.com> wrote: >> >> >> >> On 13.04.21 09:16, Wanpeng Li wrote: >> [...] >> >>> @@ -145,6 +155,13 @@ static __always_inline void guest_exit_irqoff(void) >>> } >>> >>> #else THis is the else part >>> +static __always_inline void context_guest_enter_irqoff(void) >>> +{ >>> + instrumentation_begin(); 2nd on >>> + rcu_virt_note_context_switch(smp_processor_id()); >>> + instrumentation_end(); 2nd off >>> +} >>> + >>> static __always_inline void guest_enter_irqoff(void) >>> { >>> /* >>> @@ -155,10 +172,13 @@ static __always_inline void guest_enter_irqoff(void) >>> instrumentation_begin(); first on >>> vtime_account_kernel(current); >>> current->flags |= PF_VCPU; >>> - rcu_virt_note_context_switch(smp_processor_id()); >>> instrumentation_end(); first off >>> + >>> + context_guest_enter_irqoff(); here we call the 2nd on and off. >> >> So we now do instrumentation_begin 2 times? > > Similar to context_guest_enter_irqoff() ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN. For the ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN part context_guest_enter_irqoff() does not have instrumentation_begin/end. Or did I miss anything.
On Tue, 13 Apr 2021 at 15:48, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > > > > On 13.04.21 09:38, Wanpeng Li wrote: > > On Tue, 13 Apr 2021 at 15:35, Christian Borntraeger > > <borntraeger@de.ibm.com> wrote: > >> > >> > >> > >> On 13.04.21 09:16, Wanpeng Li wrote: > >> [...] > >> > >>> @@ -145,6 +155,13 @@ static __always_inline void guest_exit_irqoff(void) > >>> } > >>> > >>> #else > THis is the else part > > > >>> +static __always_inline void context_guest_enter_irqoff(void) > >>> +{ > >>> + instrumentation_begin(); > > 2nd on > >>> + rcu_virt_note_context_switch(smp_processor_id()); > >>> + instrumentation_end(); > 2nd off > >>> +} > >>> + > >>> static __always_inline void guest_enter_irqoff(void) > >>> { > >>> /* > >>> @@ -155,10 +172,13 @@ static __always_inline void guest_enter_irqoff(void) > >>> instrumentation_begin(); > > first on > >>> vtime_account_kernel(current); > >>> current->flags |= PF_VCPU; > >>> - rcu_virt_note_context_switch(smp_processor_id()); > >>> instrumentation_end(); > > first off > >>> + > >>> + context_guest_enter_irqoff(); > here we call the 2nd on and off. > >> > >> So we now do instrumentation_begin 2 times? > > > > Similar to context_guest_enter_irqoff() ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN. > > For the > ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN part > context_guest_enter_irqoff() > does not have instrumentation_begin/end. > > Or did I miss anything. I mean the if (!context_tracking_enabled_this_cpu()) part in the function context_guest_enter_irqoff() ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN. :) Wanpeng
On 13.04.21 09:52, Wanpeng Li wrote: >> Or did I miss anything. > > I mean the if (!context_tracking_enabled_this_cpu()) part in the > function context_guest_enter_irqoff() ifdef > CONFIG_VIRT_CPU_ACCOUNTING_GEN. :) Ah I missed that. Thanks.
diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h index bceb064..d8ad844 100644 --- a/include/linux/context_tracking.h +++ b/include/linux/context_tracking.h @@ -104,16 +104,8 @@ static inline void context_tracking_init(void) { } #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN -/* must be called with irqs disabled */ -static __always_inline void guest_enter_irqoff(void) +static __always_inline void context_guest_enter_irqoff(void) { - instrumentation_begin(); - if (vtime_accounting_enabled_this_cpu()) - vtime_guest_enter(current); - else - current->flags |= PF_VCPU; - instrumentation_end(); - if (context_tracking_enabled()) __context_tracking_enter(CONTEXT_GUEST); @@ -131,10 +123,28 @@ static __always_inline void guest_enter_irqoff(void) } } -static __always_inline void guest_exit_irqoff(void) +/* must be called with irqs disabled */ +static __always_inline void guest_enter_irqoff(void) +{ + instrumentation_begin(); + if (vtime_accounting_enabled_this_cpu()) + vtime_guest_enter(current); + else + current->flags |= PF_VCPU; + instrumentation_end(); + + context_guest_enter_irqoff(); +} + +static __always_inline void context_guest_exit_irqoff(void) { if (context_tracking_enabled()) __context_tracking_exit(CONTEXT_GUEST); +} + +static __always_inline void guest_exit_irqoff(void) +{ + context_guest_exit_irqoff(); instrumentation_begin(); if (vtime_accounting_enabled_this_cpu()) @@ -145,6 +155,13 @@ static __always_inline void guest_exit_irqoff(void) } #else +static __always_inline void context_guest_enter_irqoff(void) +{ + instrumentation_begin(); + rcu_virt_note_context_switch(smp_processor_id()); + instrumentation_end(); +} + static __always_inline void guest_enter_irqoff(void) { /* @@ -155,10 +172,13 @@ static __always_inline void guest_enter_irqoff(void) instrumentation_begin(); vtime_account_kernel(current); current->flags |= PF_VCPU; - rcu_virt_note_context_switch(smp_processor_id()); instrumentation_end(); + + context_guest_enter_irqoff(); } +static __always_inline void context_guest_exit_irqoff(void) { } + static __always_inline void guest_exit_irqoff(void) { instrumentation_begin();