All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kvm: Fix accounting of interrupts during guest execution on s390
@ 2007-11-14 15:32 Christian Borntraeger
       [not found] ` <200711141632.37004.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Christian Borntraeger @ 2007-11-14 15:32 UTC (permalink / raw)
  To: kvm-devel; +Cc: Laurent Vivier, Avi Kivity

Avi, Ingo, Laurent,

what do you think about the following patch?

Currently the scheduler checks for PF_VCPU to decide if this
timeslice has to be accounted as guest time. On s390 host 
interrupts are not disabled during guest execution. This causes
these interrupts to be accounted as guest time.
Solution is to check for in_interrupt to let interrupt time 
account as soft or hardirq.

Signed-off-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
---
 kernel/sched.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: kvm/kernel/sched.c
===================================================================
--- kvm.orig/kernel/sched.c
+++ kvm/kernel/sched.c
@@ -3395,7 +3395,7 @@ void account_system_time(struct task_str
 	struct rq *rq = this_rq();
 	cputime64_t tmp;
 
-	if (p->flags & PF_VCPU) {
+	if ((p->flags & PF_VCPU) && !in_interrupt()) {
 		account_guest_time(p, cputime);
 		return;
 	}

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] kvm: Fix accounting of interrupts during guest execution on s390
       [not found] ` <200711141632.37004.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
@ 2007-11-14 15:36   ` Ingo Molnar
       [not found]     ` <20071114153651.GA3441-X9Un+BFzKDI@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Ingo Molnar @ 2007-11-14 15:36 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Laurent Vivier, Avi Kivity


* Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org> wrote:

> Avi, Ingo, Laurent,
> 
> what do you think about the following patch?

fine to me. I guess this should mostly be a NOP to KVM, right?

	Ingo

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] kvm: Fix accounting of interrupts during guest execution on s390
       [not found]     ` <20071114153651.GA3441-X9Un+BFzKDI@public.gmane.org>
@ 2007-11-14 15:47       ` Christian Borntraeger
       [not found]         ` <200711141647.45810.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Christian Borntraeger @ 2007-11-14 15:47 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: kvm-devel, Laurent Vivier, Avi Kivity

Am Mittwoch, 14. November 2007 schrieb Ingo Molnar:
> 
> * Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > Avi, Ingo, Laurent,
> > 
> > what do you think about the following patch?
> 
> fine to me. I guess this should mostly be a NOP to KVM, right?

Yes, currently kvm on x86 has interrupts disabled during vmx and svm. So
in_interrupt should be always false on x86.

Christian

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] kvm: Fix accounting of interrupts during guest execution on s390
       [not found]         ` <200711141647.45810.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
@ 2007-11-14 16:04           ` Avi Kivity
       [not found]             ` <473B1C97.2010007-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Avi Kivity @ 2007-11-14 16:04 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Laurent Vivier

Christian Borntraeger wrote:
> Am Mittwoch, 14. November 2007 schrieb Ingo Molnar:
>   
>> * Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org> wrote:
>>
>>     
>>> Avi, Ingo, Laurent,
>>>
>>> what do you think about the following patch?
>>>       
>> fine to me. I guess this should mostly be a NOP to KVM, right?
>>     
>
> Yes, currently kvm on x86 has interrupts disabled during vmx and svm. So
> in_interrupt should be always false on x86.
>   
But isn't account_system_time() called via the timer tick interrupt, and 
never directly from kvm?  So in_interrupt() would always be true.


The sequence, as I understand it, is:

  - disable interrupts
  - set PF_VCPU
  - switch to guest
  - exit guest due to timer tick
  - enable interrupts
  - dispatch timer tick
  - account_system_time()

With the fine-grained accounting ppc and s390 have, this is different.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] kvm: Fix accounting of interrupts during guest execution on s390
       [not found]             ` <473B1C97.2010007-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
@ 2007-11-14 16:07               ` Christian Borntraeger
  2007-11-15 14:10               ` [PATCH v2] " Christian Borntraeger
  1 sibling, 0 replies; 14+ messages in thread
From: Christian Borntraeger @ 2007-11-14 16:07 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel, Laurent Vivier

Am Mittwoch, 14. November 2007 schrieb Avi Kivity:
> But isn't account_system_time() called via the timer tick interrupt, and 
> never directly from kvm?  So in_interrupt() would always be true.

Ouch, yes. Please dont use that patch, I will make a new one. 

Christian

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] kvm: Fix accounting of interrupts during guest execution on s390
       [not found]             ` <473B1C97.2010007-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
  2007-11-14 16:07               ` Christian Borntraeger
@ 2007-11-15 14:10               ` Christian Borntraeger
       [not found]                 ` <200711151510.37076.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
  1 sibling, 1 reply; 14+ messages in thread
From: Christian Borntraeger @ 2007-11-15 14:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel, Laurent Vivier

Avi pointed out, that my first patch was broken, here is the 2nd try.
I tested the patch on s390 with CONFIG_VIRT_CPU_ACCOUNTING and on x86_64.
Seems to work.

Currently the scheduler checks for PF_VCPU to decide if this
timeslice has to be accounted as guest time. On s390 host 
interrupts are not disabled during guest execution. This causes
theses interrupts to be accounted as guest time if
CONFIG_VIRT_CPU_ACCOUNTING is set.
Solution is to check if an interrupt triggered account_system_time.
As the tick is timer interrupt based, we have to subtract
hardirq_offset.

Avi, Ingo, Laurent, feedback is welcome.

CC: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
CC: Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
CC: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
Signed-off-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
---
 kernel/sched.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Index: kvm/kernel/sched.c
===================================================================
--- kvm.orig/kernel/sched.c
+++ kvm/kernel/sched.c
@@ -3395,10 +3395,8 @@ void account_system_time(struct task_str
 	struct rq *rq = this_rq();
 	cputime64_t tmp;
 
-	if (p->flags & PF_VCPU) {
-		account_guest_time(p, cputime);
-		return;
-	}
+	if ((p->flags & PF_VCPU) && (irq_count() - hardirq_offset == 0))
+		return account_guest_time(p, cputime);
 
 	p->stime = cputime_add(p->stime, cputime);
 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: Fix accounting of interrupts during guest execution on s390
       [not found]                 ` <200711151510.37076.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
@ 2007-11-15 14:18                   ` Ingo Molnar
  2007-11-15 14:23                   ` Avi Kivity
  2007-11-15 15:24                   ` Laurent Vivier
  2 siblings, 0 replies; 14+ messages in thread
From: Ingo Molnar @ 2007-11-15 14:18 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Laurent Vivier, Avi Kivity


* Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org> wrote:

> Avi pointed out, that my first patch was broken, here is the 2nd try. 
> I tested the patch on s390 with CONFIG_VIRT_CPU_ACCOUNTING and on 
> x86_64. Seems to work.
> 
> Currently the scheduler checks for PF_VCPU to decide if this timeslice 
> has to be accounted as guest time. On s390 host interrupts are not 
> disabled during guest execution. This causes theses interrupts to be 
> accounted as guest time if CONFIG_VIRT_CPU_ACCOUNTING is set. Solution 
> is to check if an interrupt triggered account_system_time. As the tick 
> is timer interrupt based, we have to subtract hardirq_offset.
> 
> Avi, Ingo, Laurent, feedback is welcome.

thanks, applied. I'll push this out as part of the scheduler git tree, 
ok?

	Ingo

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: Fix accounting of interrupts during guest execution on s390
       [not found]                 ` <200711151510.37076.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
  2007-11-15 14:18                   ` Ingo Molnar
@ 2007-11-15 14:23                   ` Avi Kivity
       [not found]                     ` <473C5652.9070706-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
  2007-11-15 15:24                   ` Laurent Vivier
  2 siblings, 1 reply; 14+ messages in thread
From: Avi Kivity @ 2007-11-15 14:23 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Laurent Vivier

Christian Borntraeger wrote:
> Avi pointed out, that my first patch was broken, here is the 2nd try.
> I tested the patch on s390 with CONFIG_VIRT_CPU_ACCOUNTING and on x86_64.
> Seems to work.
>
> Currently the scheduler checks for PF_VCPU to decide if this
> timeslice has to be accounted as guest time. On s390 host 
> interrupts are not disabled during guest execution. This causes
> theses interrupts to be accounted as guest time if
> CONFIG_VIRT_CPU_ACCOUNTING is set.
> Solution is to check if an interrupt triggered account_system_time.
> As the tick is timer interrupt based, we have to subtract
> hardirq_offset.
>
> Avi, Ingo, Laurent, feedback is welcome.
>
> CC: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
> CC: Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
> CC: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
> Signed-off-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> ---
>  kernel/sched.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> Index: kvm/kernel/sched.c
> ===================================================================
> --- kvm.orig/kernel/sched.c
> +++ kvm/kernel/sched.c
> @@ -3395,10 +3395,8 @@ void account_system_time(struct task_str
>  	struct rq *rq = this_rq();
>  	cputime64_t tmp;
>  
> -	if (p->flags & PF_VCPU) {
> -		account_guest_time(p, cputime);
> -		return;
> -	}
> +	if ((p->flags & PF_VCPU) && (irq_count() - hardirq_offset == 0))
> +		return account_guest_time(p, cputime);
>  
>  	p->stime = cputime_add(p->stime, cputime);
>  
>   

What's irq_count()?  Is it defined to ignore the timer tick?  Or is that 
hardirq_offset?

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: Fix accounting of interrupts during guest execution on s390
       [not found]                     ` <473C5652.9070706-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
@ 2007-11-15 14:38                       ` Christian Borntraeger
       [not found]                         ` <200711151538.16774.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Christian Borntraeger @ 2007-11-15 14:38 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel, Laurent Vivier

Am Donnerstag, 15. November 2007 schrieb Avi Kivity:
> > +	if ((p->flags & PF_VCPU) && (irq_count() - hardirq_offset == 0))
[...]
> What's irq_count()?  Is it defined to ignore the timer tick?  Or is that 
> hardirq_offset?

hardirq_offset is used to ignore the timer tick.
irq_count is defined in hardirq.h:

#define irq_count()     (preempt_count() & (HARDIRQ_MASK | SOFTIRQ_MASK))

So irq_count() - hardirq_offset is not zero if a hardirq or a softirq is
running. By subtracting hardirq_offset (the timer tick sets it to
HARDIRQ_OFFSET) it also makes sure, that the timer tick does not count as
hardirq.
This seems to do the trick.

Christian

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: Fix accounting of interrupts during guest execution on s390
       [not found]                         ` <200711151538.16774.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
@ 2007-11-15 14:40                           ` Avi Kivity
  0 siblings, 0 replies; 14+ messages in thread
From: Avi Kivity @ 2007-11-15 14:40 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Laurent Vivier

Christian Borntraeger wrote:
> Am Donnerstag, 15. November 2007 schrieb Avi Kivity:
>   
>>> +	if ((p->flags & PF_VCPU) && (irq_count() - hardirq_offset == 0))
>>>       
> [...]
>   
>> What's irq_count()?  Is it defined to ignore the timer tick?  Or is that 
>> hardirq_offset?
>>     
>
> hardirq_offset is used to ignore the timer tick.
> irq_count is defined in hardirq.h:
>
> #define irq_count()     (preempt_count() & (HARDIRQ_MASK | SOFTIRQ_MASK))
>
> So irq_count() - hardirq_offset is not zero if a hardirq or a softirq is
> running. By subtracting hardirq_offset (the timer tick sets it to
> HARDIRQ_OFFSET) it also makes sure, that the timer tick does not count as
> hardirq.
> This seems to do the trick.
>
>   

Ah, ok.  Thanks for the explanation.


-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: Fix accounting of interrupts during guest execution on s390
       [not found]                 ` <200711151510.37076.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
  2007-11-15 14:18                   ` Ingo Molnar
  2007-11-15 14:23                   ` Avi Kivity
@ 2007-11-15 15:24                   ` Laurent Vivier
  2007-11-15 15:56                     ` Christian Borntraeger
  2 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2007-11-15 15:24 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Avi Kivity


[-- Attachment #1.1: Type: text/plain, Size: 2843 bytes --]

If I remember correctly time accounting on s390 is more accurate than on
x86 ?

Because on x86, as we make the kvm_guest_exit() after local_irq_enable()
we can also have IRQ with PF_VCPU set... and we discussed a lot on
probability to know if it was good or not. And on x86 it seems good
because it is already working like that with system and user time (we
account time to the space where tick appears).

see http://lkml.org/lkml/2007/10/15/228

Laurent

Le jeudi 15 novembre 2007 à 15:10 +0100, Christian Borntraeger a écrit :
> Avi pointed out, that my first patch was broken, here is the 2nd try.
> I tested the patch on s390 with CONFIG_VIRT_CPU_ACCOUNTING and on x86_64.
> Seems to work.
> 
> Currently the scheduler checks for PF_VCPU to decide if this
> timeslice has to be accounted as guest time. On s390 host 
> interrupts are not disabled during guest execution. This causes
> theses interrupts to be accounted as guest time if
> CONFIG_VIRT_CPU_ACCOUNTING is set.
> Solution is to check if an interrupt triggered account_system_time.
> As the tick is timer interrupt based, we have to subtract
> hardirq_offset.
> 
> Avi, Ingo, Laurent, feedback is welcome.
> 
> CC: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
> CC: Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
> CC: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
> Signed-off-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> ---
>  kernel/sched.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> Index: kvm/kernel/sched.c
> ===================================================================
> --- kvm.orig/kernel/sched.c
> +++ kvm/kernel/sched.c
> @@ -3395,10 +3395,8 @@ void account_system_time(struct task_str
>  	struct rq *rq = this_rq();
>  	cputime64_t tmp;
>  
> -	if (p->flags & PF_VCPU) {
> -		account_guest_time(p, cputime);
> -		return;
> -	}
> +	if ((p->flags & PF_VCPU) && (irq_count() - hardirq_offset == 0))
> +		return account_guest_time(p, cputime);
>  
>  	p->stime = cputime_add(p->stime, cputime);
>  
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> _______________________________________________
> kvm-devel mailing list
> kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
       "Any sufficiently advanced technology is
  indistinguishable from magic." - Arthur C. Clarke

[-- Attachment #1.2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 314 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

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

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: Fix accounting of interrupts during guest execution on s390
  2007-11-15 15:24                   ` Laurent Vivier
@ 2007-11-15 15:56                     ` Christian Borntraeger
       [not found]                       ` <200711151656.44935.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Christian Borntraeger @ 2007-11-15 15:56 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: kvm-devel, Avi Kivity

Am Donnerstag, 15. November 2007 schrieb Laurent Vivier:
> If I remember correctly time accounting on s390 is more accurate than on
> x86 ?

Yes. its done during context switches and resolution is microsecond.

> Because on x86, as we make the kvm_guest_exit() after local_irq_enable()
> we can also have IRQ with PF_VCPU set... and we discussed a lot on
> probability to know if it was good or not. And on x86 it seems good
> because it is already working like that with system and user time (we
> account time to the space where tick appears).
> see http://lkml.org/lkml/2007/10/15/228

I am not sure I fully understand your point, can you try to explain?

My patch deals with timer ticks (see hardirq_offset). So if a only a tick
comes in after local_irq_enable the time is accounted to guest time as 
before.
I made a test on x86_64 with burnP6 inside a kvm machine. top showed 100%
guest time on an otherwise idle host. So the guest  accounting itself did
still work. I tried some disk and network stress on the host but I did not
manage to bring hardirq+softirq time above 5%, guest time stayed above
90%. At least my simple testcase did not show a bias towards irq time.

> Le jeudi 15 novembre 2007 à 15:10 +0100, Christian Borntraeger a écrit :
> > Avi pointed out, that my first patch was broken, here is the 2nd try.
> > I tested the patch on s390 with CONFIG_VIRT_CPU_ACCOUNTING and on x86_64.
> > Seems to work.
> > 
> > Currently the scheduler checks for PF_VCPU to decide if this
> > timeslice has to be accounted as guest time. On s390 host 
> > interrupts are not disabled during guest execution. This causes
> > theses interrupts to be accounted as guest time if
> > CONFIG_VIRT_CPU_ACCOUNTING is set.
> > Solution is to check if an interrupt triggered account_system_time.
> > As the tick is timer interrupt based, we have to subtract
> > hardirq_offset.
> > 
> > Avi, Ingo, Laurent, feedback is welcome.
> > 
> > CC: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
> > CC: Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
> > CC: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
> > Signed-off-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> > ---
> >  kernel/sched.c |    6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > Index: kvm/kernel/sched.c
> > ===================================================================
> > --- kvm.orig/kernel/sched.c
> > +++ kvm/kernel/sched.c
> > @@ -3395,10 +3395,8 @@ void account_system_time(struct task_str
> >  	struct rq *rq = this_rq();
> >  	cputime64_t tmp;
> >  
> > -	if (p->flags & PF_VCPU) {
> > -		account_guest_time(p, cputime);
> > -		return;
> > -	}
> > +	if ((p->flags & PF_VCPU) && (irq_count() - hardirq_offset == 0))
> > +		return account_guest_time(p, cputime);
> >  
> >  	p->stime = cputime_add(p->stime, cputime);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: Fix accounting of interrupts during guest execution on s390
       [not found]                       ` <200711151656.44935.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
@ 2007-11-15 17:08                         ` Laurent Vivier
  2007-11-15 18:05                           ` Christian Borntraeger
  0 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2007-11-15 17:08 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Avi Kivity


[-- Attachment #1.1: Type: text/plain, Size: 4497 bytes --]

Le jeudi 15 novembre 2007 à 16:56 +0100, Christian Borntraeger a écrit :
> Am Donnerstag, 15. November 2007 schrieb Laurent Vivier:
> > If I remember correctly time accounting on s390 is more accurate than on
> > x86 ?
> 
> Yes. its done during context switches and resolution is microsecond.
> 
> > Because on x86, as we make the kvm_guest_exit() after local_irq_enable()
> > we can also have IRQ with PF_VCPU set... and we discussed a lot on
> > probability to know if it was good or not. And on x86 it seems good
> > because it is already working like that with system and user time (we
> > account time to the space where tick appears).
> > see http://lkml.org/lkml/2007/10/15/228
> 
> I am not sure I fully understand your point, can you try to explain?

I can try...

on x86 interrupts are accounted to guest time if they occur when PF_VCPU
is set... and this not a problem because x86 time accounting is not
really accurate (and already works like that).

So as s390 accounting is working differently, it seems normal to correct
the accounted value.

To be annoying, it should be clearer to write this like:

if ( (p->flags & PF_VCPU) &&
     !(hardirq_count() - hardirq_offset) &&
     !softirq_count() )
{
	account_guest_time(p, cputime);
	return;
}

as we have:

#define hardirq_count() (preempt_count() & HARDIRQ_MASK)
#define softirq_count() (preempt_count() & SOFTIRQ_MASK)
#define irq_count()     (preempt_count() & (HARDIRQ_MASK |
SOFTIRQ_MASK))

and in account_system_time():

...
        if (hardirq_count() - hardirq_offset)
                cpustat->irq = cputime64_add(cpustat->irq, tmp);
        else if (softirq_count())
                cpustat->softirq = cputime64_add(cpustat->softirq, tmp);
        else if (p != rq->idle) {
...

So it is easier to understand we don't account this time to guest if we
have hard irq or soft irq

But I agree with your patch.

Laurent

> My patch deals with timer ticks (see hardirq_offset). So if a only a tick
> comes in after local_irq_enable the time is accounted to guest time as 
> before.
> I made a test on x86_64 with burnP6 inside a kvm machine. top showed 100%
> guest time on an otherwise idle host. So the guest  accounting itself did
> still work. I tried some disk and network stress on the host but I did not
> manage to bring hardirq+softirq time above 5%, guest time stayed above
> 90%. At least my simple testcase did not show a bias towards irq time.
> 
> > Le jeudi 15 novembre 2007 à 15:10 +0100, Christian Borntraeger a écrit :
> > > Avi pointed out, that my first patch was broken, here is the 2nd try.
> > > I tested the patch on s390 with CONFIG_VIRT_CPU_ACCOUNTING and on x86_64.
> > > Seems to work.
> > > 
> > > Currently the scheduler checks for PF_VCPU to decide if this
> > > timeslice has to be accounted as guest time. On s390 host 
> > > interrupts are not disabled during guest execution. This causes
> > > theses interrupts to be accounted as guest time if
> > > CONFIG_VIRT_CPU_ACCOUNTING is set.
> > > Solution is to check if an interrupt triggered account_system_time.
> > > As the tick is timer interrupt based, we have to subtract
> > > hardirq_offset.
> > > 
> > > Avi, Ingo, Laurent, feedback is welcome.
> > > 
> > > CC: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
> > > CC: Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
> > > CC: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
> > > Signed-off-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> > > ---
> > >  kernel/sched.c |    6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > Index: kvm/kernel/sched.c
> > > ===================================================================
> > > --- kvm.orig/kernel/sched.c
> > > +++ kvm/kernel/sched.c
> > > @@ -3395,10 +3395,8 @@ void account_system_time(struct task_str
> > >  	struct rq *rq = this_rq();
> > >  	cputime64_t tmp;
> > >  
> > > -	if (p->flags & PF_VCPU) {
> > > -		account_guest_time(p, cputime);
> > > -		return;
> > > -	}
> > > +	if ((p->flags & PF_VCPU) && (irq_count() - hardirq_offset == 0))
> > > +		return account_guest_time(p, cputime);
> > >  
> > >  	p->stime = cputime_add(p->stime, cputime);
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
       "Any sufficiently advanced technology is
  indistinguishable from magic." - Arthur C. Clarke

[-- Attachment #1.2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 314 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

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

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: Fix accounting of interrupts during guest execution on s390
  2007-11-15 17:08                         ` Laurent Vivier
@ 2007-11-15 18:05                           ` Christian Borntraeger
  0 siblings, 0 replies; 14+ messages in thread
From: Christian Borntraeger @ 2007-11-15 18:05 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: kvm-devel, Avi Kivity

Am Donnerstag, 15. November 2007 schrieb Laurent Vivier:
> To be annoying, it should be clearer to write this like:
> 
> if ( (p->flags & PF_VCPU) &&
>      !(hardirq_count() - hardirq_offset) &&
>      !softirq_count() )
> {
> 	account_guest_time(p, cputime);
> 	return;
> }
[...]
> But I agree with your patch.
> 
> Laurent

Yes, that would have the same result. I think its a matter of taste,
if we want to have a not-so-obvious one line if, or a multi-line-if.

I dont mind which version use, here is your variant as an alternative:
Ingo, you decide :-)

From: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
From: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>

Signed-off-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
---
 kernel/sched.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: kvm/kernel/sched.c
===================================================================
--- kvm.orig/kernel/sched.c
+++ kvm/kernel/sched.c
@@ -3395,7 +3395,9 @@ void account_system_time(struct task_str
 	struct rq *rq = this_rq();
 	cputime64_t tmp;
 
-	if (p->flags & PF_VCPU) {
+	if ( (p->flags & PF_VCPU) &&
+	     !(hardirq_count() - hardirq_offset) &&
+	     !softirq_count() ) {
 		account_guest_time(p, cputime);
 		return;
 	}







-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2007-11-15 18:05 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-11-14 15:32 [PATCH] kvm: Fix accounting of interrupts during guest execution on s390 Christian Borntraeger
     [not found] ` <200711141632.37004.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
2007-11-14 15:36   ` Ingo Molnar
     [not found]     ` <20071114153651.GA3441-X9Un+BFzKDI@public.gmane.org>
2007-11-14 15:47       ` Christian Borntraeger
     [not found]         ` <200711141647.45810.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
2007-11-14 16:04           ` Avi Kivity
     [not found]             ` <473B1C97.2010007-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2007-11-14 16:07               ` Christian Borntraeger
2007-11-15 14:10               ` [PATCH v2] " Christian Borntraeger
     [not found]                 ` <200711151510.37076.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
2007-11-15 14:18                   ` Ingo Molnar
2007-11-15 14:23                   ` Avi Kivity
     [not found]                     ` <473C5652.9070706-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2007-11-15 14:38                       ` Christian Borntraeger
     [not found]                         ` <200711151538.16774.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
2007-11-15 14:40                           ` Avi Kivity
2007-11-15 15:24                   ` Laurent Vivier
2007-11-15 15:56                     ` Christian Borntraeger
     [not found]                       ` <200711151656.44935.borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
2007-11-15 17:08                         ` Laurent Vivier
2007-11-15 18:05                           ` Christian Borntraeger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.