kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Ulrich Obergfell <uobergfe@redhat.com>,
	Glauber Costa <glommer@redhat.com>, Avi Kivity <avi@redhat.com>,
	kvm <kvm@vger.kernel.org>, qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] Re: [RFC: 0/2] patch for QEMU HPET periodic timer emulation to alleviate time drift
Date: Mon, 07 Feb 2011 08:54:49 -0600	[thread overview]
Message-ID: <4D5007B9.7060806@codemonkey.ws> (raw)
In-Reply-To: <4D5004FC.80000@siemens.com>

On 02/07/2011 08:43 AM, Jan Kiszka wrote:
> On 2011-02-07 15:28, Anthony Liguori wrote:
>    
>> On 02/07/2011 08:10 AM, Jan Kiszka wrote:
>>      
>>> Again: please not in an ad-hoc fashion but as a generic services usable
>>> by _all_ periodic timer sources that want to implement compensation.
>>> This infrastructure should also be designed to once integrate IRQ
>>> coalescing information as well.
>>>
>>> The point why I'm insisting on a broader solution is that both sources
>>> for lost ticks (iothread and vcpu) end up in the same output: an
>>> adjustment of the injection frequency of the affected timer device.
>>> There is not "HPET" or "RTC" or "PIT" in this, all this may apply to the
>>> SoC timer of some emulated ARM board as well.
>>>
>>>        
>> Fair enough, how about:
>>
>> typedef struct PeriodicTimer PeriodicTimer;
>>
>> /**
>>    * @accumulated_ticks:  the number of unacknowledged ticks in total
>> since the creation of the timer
>>    **/
>> typedef void (PeriodicTimer)(void *opaque, int accumulated_ticks);
>>      
> I guess you mean PeriodicTimerFunc.

Yes.

>   Why the accumulated_ticks argument?
>    

Then the missing ticks is stored in the PeriodicTimer instead of storing 
it in the device state.  That means we won't forget to save it in vmstate.

It's convenient because then if we lose ticks in the PeriodicTimer 
layer, the devices have instance access to that info.  When you do a 
read() from timerfd, it returns the number of coalesced events.  That's 
the interface I had in my mind.

We could just add a getter for PeriodicTimer and it would serve the same 
purpose.

Regards,

Anthony Liguori

>> PeriodicTimer *periodic_timer_new(PeriodicTimerFunc *cb, void *opaque);
>>
>> void periodic_timer_mod(PeriodicTimer *timer, int64_t interval, TimeUnit
>> unit);
>>
>> /**
>>    * @policy: the drift catch-up policy
>>    *                DRIFT_COMP_FAST, deliver next tick as soon as any
>> tick is acknowledged if accumulated_ticks>  1
>>    *                DRIFT_COMP_NONE, do not change interval regardless of
>> accumulated ticks
>>    *                DRIFT_COMP_GRADUAL, shorten interval by half until
>> accumulated_ticks<= 1
>>    */
>> void periodic_timer_set_policy(PeriodicTimer *timer,
>> DriftCompensationPolicy policy);
>>
>> /**
>>    * @ticks: number of ticks to acknowledge that are currently outstanding.
>>    **/
>> void periodic_timer_ack(PeriodicTimer *timer, int ticks);
>>
>>      
> Looks reasonable otherwise.
>
> Jan
>
>    


  reply	other threads:[~2011-02-07 14:54 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <480481933.225059.1296734409954.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com>
2011-02-03 13:43 ` [RFC: 0/2] patch for QEMU HPET periodic timer emulation to alleviate time drift Ulrich Obergfell
2011-02-03 15:28   ` Jan Kiszka
2011-02-03 20:07     ` [Qemu-devel] " Anthony Liguori
2011-02-03 21:24       ` Jan Kiszka
2011-02-04  2:06         ` Anthony Liguori
2011-02-04  8:56           ` Jan Kiszka
2011-02-07 12:34             ` Avi Kivity
2011-02-07 13:11               ` Anthony Liguori
2011-02-07 13:14                 ` Avi Kivity
2011-02-07 13:23                   ` Anthony Liguori
2011-02-07 13:34                     ` Avi Kivity
2011-02-07 13:41                     ` Gleb Natapov
2011-02-07 13:46                       ` Avi Kivity
2011-02-07 13:48                         ` Gleb Natapov
2011-02-07 13:51                           ` Avi Kivity
2011-02-07 13:54                             ` Gleb Natapov
2011-02-07 14:10                     ` Jan Kiszka
2011-02-07 14:28                       ` Anthony Liguori
2011-02-07 14:43                         ` Jan Kiszka
2011-02-07 14:54                           ` Anthony Liguori [this message]
2011-02-07 14:57                             ` Jan Kiszka
2011-02-07 15:01                               ` Anthony Liguori
2011-02-07 15:08                                 ` Jan Kiszka
2011-02-07 15:13                                 ` Avi Kivity
2011-02-07 15:17                                   ` Jan Kiszka
2011-02-07 15:29                                     ` Avi Kivity
2011-02-07 19:30                                       ` Anthony Liguori
2011-02-08  9:11                                         ` Avi Kivity
2011-02-07 15:20                                   ` Jan Kiszka
2011-02-07 15:30                                     ` Avi Kivity
2011-02-07 19:28                                     ` Anthony Liguori
2011-02-07 14:58                             ` Avi Kivity
2011-02-07 15:01                               ` Jan Kiszka
2011-02-07 15:08                                 ` Avi Kivity
2011-02-07 15:14                                   ` Jan Kiszka
2011-02-07 15:16                                     ` Avi Kivity
2011-02-07 15:22                                   ` Anthony Liguori
2011-02-07 15:18                               ` Anthony Liguori
2011-02-04  9:52           ` Ulrich Obergfell
2011-02-07 10:44       ` Ulrich Obergfell
2011-02-07 13:24       ` Anthony Liguori

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=4D5007B9.7060806@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=avi@redhat.com \
    --cc=glommer@redhat.com \
    --cc=jan.kiszka@siemens.com \
    --cc=kvm@vger.kernel.org \
    --cc=qemu-devel@nongnu.org \
    --cc=uobergfe@redhat.com \
    /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;
as well as URLs for NNTP newsgroup(s).