public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Laurent Vivier <Laurent.Vivier@bull.net>
To: Avi Kivity <avi@qumranet.com>
Cc: Ingo Molnar <mingo@elte.hu>, linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND 2][PATCH 4/4] Modify KVM to update guest time	accounting.
Date: Mon, 15 Oct 2007 13:37:49 +0200	[thread overview]
Message-ID: <4713510D.2050208@bull.net> (raw)
In-Reply-To: <47134BCE.1080306@qumranet.com>

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

Avi Kivity wrote:
> Laurent Vivier wrote:
>> Avi Kivity wrote:
>>  
>>> Ingo Molnar wrote:
>>>    
>>>> * Avi Kivity <avi@qumranet.com> wrote:
>>>>
>>>>  
>>>>      
>>>>> This bit can go; for the external module I can add it back in
>>>>> external-module-compat.h.  No need to pollute mainline with backward
>>>>> compatibility stuff.
>>>>>             
>>>> hm:
>>>>
>>>> static inline void kvm_guest_enter(void)
>>>> {
>>>>         current->flags |= PF_VCPU;
>>>> }
>>>>
>>>> static inline void kvm_guest_exit(void)
>>>> {
>>>> }
>>>>
>>>> shouldnt PF_VCPU be cleared in kvm_guest_exit()?
>>>>         
>>> IIRC the accounting code clears it, but yes, it may not have been called
>>> at all, so clearing it here is needed.
>>>
>>>     
>>
>> No, It must not be cleared here because we can't enter in the
>> accounting code
>> between kvm_guest_enter(void) and kvm_guest_exit(void).
>>
>>   
> 
> Right.
> 
>> This is why the accounting code clears it.
>>   
> 
> But if we didn't get an interrupt in that time?
> 
> We can clear it a bit later, after local_irq_enable() in __vcpu_run(). 
> However we need a nop instruction first because "sti" keeps interrupts
> disabled for one more instruction.

IMHO, I think it is better to let kvm_guest_exit() empty (you can remove it, if
you want):

1st case:
- unset PF_VCPU in kvm_guest_exit(), all the tick is always for system time.
Guest time is always 0.

1st case and half:

- like 1st case but we move kvm_guest_exit() as you propose and the reason of
the interrupt is the tick interrupt. The tick is for guest time only. I think
the probability is very low.

2nd case:
- don't unset PF_VCPU in kvm_guest_exit(), all the tick is for guest time.

I proposed a patch allowing to be more accurate, but it introduces more
complexity and system and user time accounting are not very accurate too (the
tick if for system if it appears whereas we are in system, for user if it
appears whereas we are in user).

Laurent
-- 
---------------- Laurent.Vivier@bull.net  -----------------
"Given enough eyeballs, all bugs are shallow" E. S. Raymond


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

  parent reply	other threads:[~2007-10-15 11:38 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-10 14:13 [RESEND 2][PATCH 4/4] Modify KVM to update guest time accounting Laurent Vivier
2007-10-15  9:38 ` Ingo Molnar
2007-10-15  9:47   ` Avi Kivity
2007-10-15  9:50     ` Ingo Molnar
2007-10-15  9:51     ` Ingo Molnar
2007-10-15 10:02       ` Avi Kivity
2007-10-15 10:53         ` Laurent Vivier
2007-10-15 11:15           ` Avi Kivity
2007-10-15 11:33             ` Christian Borntraeger
2007-10-15 11:38               ` Laurent Vivier
2007-10-15 14:39                 ` Christian Borntraeger
2007-10-15 14:45                   ` Laurent Vivier
2007-10-15 11:37             ` Laurent Vivier [this message]
2007-10-15 12:07               ` Avi Kivity
2007-10-15 12:29                 ` Laurent Vivier
2007-10-15 16:46                   ` Avi Kivity
2007-10-15 19:45                     ` Laurent Vivier
2007-10-17 13:08                   ` [PATCH] clear PF_VCPU in kvm_guest_exit() Laurent Vivier
2007-10-17 13:18                     ` Christian Borntraeger
2007-10-17 14:16                     ` Avi Kivity
2007-10-17 15:09                       ` Laurent Vivier
2007-10-18 12:39                     ` Use virtual cpu accounting if available for guest times Christian Borntraeger
2007-10-18 12:41                       ` Avi Kivity
2007-10-19 16:57                       ` [kvm-devel] " Hollis Blanchard
2007-10-19 17:18                         ` Hollis Blanchard
2007-10-18 13:19                   ` [PATCH] move kvm_guest_exit() after local_irq_enable() Laurent Vivier
2007-10-18 13:32                     ` [kvm-devel] " Avi Kivity
2007-10-18 13:49                       ` Laurent Vivier
2007-10-22  8:26                     ` Ingo Molnar
2007-10-22  8:51                       ` Avi Kivity
2007-10-22  8:57                         ` Ingo Molnar
2007-10-22  9:10                           ` [kvm-devel] " Avi Kivity
2007-10-15 11:19           ` [RESEND 2][PATCH 4/4] Modify KVM to update guest time accounting Christian Borntraeger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4713510D.2050208@bull.net \
    --to=laurent.vivier@bull.net \
    --cc=avi@qumranet.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox