From: Don Zickus <dzickus@redhat.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Cc: kvm@vger.kernel.org, pbonzini@redhat.com, gleb@redhat.com,
linux-kernel@vger.kernel.org
Subject: Re: [patch 2/3] pvclock: detect watchdog reset at pvclock read
Date: Wed, 9 Oct 2013 09:55:19 -0400 [thread overview]
Message-ID: <20131009135519.GF227855@redhat.com> (raw)
In-Reply-To: <20131008220811.GC16625@amt.cnet>
On Tue, Oct 08, 2013 at 07:08:11PM -0300, Marcelo Tosatti wrote:
> On Tue, Oct 08, 2013 at 09:37:05AM -0400, Don Zickus wrote:
> > On Mon, Oct 07, 2013 at 10:05:17PM -0300, Marcelo Tosatti wrote:
> > > Implement reset of kernel watchdogs at pvclock read time. This avoids
> > > adding special code to every watchdog.
> > >
> > > This is possible for watchdogs which measure time based on sched_clock() or
> > > ktime_get() variants.
> > >
> > > Suggested by Don Zickus.
> > >
> > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> >
> > Awesome. Thanks for figuring this out Marcelo. Does that mean we can
> > revert commit 5d1c0f4a now? :-)
>
> Unfortunately no: soft lockup watchdog does not measure time based on
> sched_clock but on hrtimer interrupt count :-(
I believe it does. See __touch_watchdog() which calls get_timestamp() -->
local_clock(). That is how it calculates the duration of the softlockup.
Now with your patch, it just sets the timestamp to zero with
touch_softlockup_watchdog_sync(), which is fine. It will just sync up the
clock, set a new timestamp, and check again in the next hrtimer interrupt.
So I guess I am confused what that commit does compared to this patch.
> (see the the softlockup code in question, perhaps you can point to
> something that i'm missing).
>
> BTW, are you OK with printing additional steal time information?
> https://lkml.org/lkml/2013/6/27/755
Well, I thought this patch was supposed to replace that patch? Why do you
still need that patch?
Perhaps my confusion is centered around which softlockups are the problem
the VM's or the host's.
>From the host perspective, I didn't think you would have any problem
because the VM is just another process that runs in its time slice.
>From the VM perspective, the whole overcommit/'wait a couple of minutes to
run again', could easily cause lockups. But I thought this patch set
detected that and touched the watchdogs early enough that when the next
iteration of the hrtimer came through, it would _not_ cause a softlockup
(it would delay it an hrtimer cycle).
So, if I am misunderstanding the problems (which I probably am :-) ), I
could use a pointer or a quick explaination to remind what the issues are
again and why you think the other patches are still necessary. :-)
Cheers,
Don
next prev parent reply other threads:[~2013-10-09 13:55 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-08 1:05 [patch 0/3] generic kernel watchdog reset at pvclock read Marcelo Tosatti
2013-10-08 1:05 ` [patch 1/3] hung_task: add method to reset detector Marcelo Tosatti
2013-10-08 13:35 ` Don Zickus
2013-10-08 1:05 ` [patch 2/3] pvclock: detect watchdog reset at pvclock read Marcelo Tosatti
2013-10-08 9:58 ` Paolo Bonzini
2013-10-09 1:22 ` Marcelo Tosatti
2013-10-09 8:39 ` Paolo Bonzini
2013-10-08 13:37 ` Don Zickus
2013-10-08 22:08 ` Marcelo Tosatti
2013-10-09 13:55 ` Don Zickus [this message]
2013-10-09 21:26 ` Marcelo Tosatti
2013-10-16 18:22 ` Don Zickus
2013-10-08 1:05 ` [patch 3/3] 01-hung-task-watchdog-reset 02-kvmclock-touch-watchdog-on-kvmclock-read series Marcelo Tosatti
2013-10-08 1:07 ` Marcelo Tosatti
2013-10-08 9:57 ` [patch 0/3] generic kernel watchdog reset at pvclock read Paolo Bonzini
2013-10-12 0:39 ` [patch 0/2] generic kernel watchdog reset at pvclock read (v2) Marcelo Tosatti
2013-10-12 0:39 ` [patch 1/2] pvclock: detect watchdog reset at pvclock read Marcelo Tosatti
2013-10-12 0:39 ` [patch 2/2] hung_task: add method to reset detector Marcelo Tosatti
2013-10-14 11:30 ` [patch 0/2] generic kernel watchdog reset at pvclock read (v2) Paolo Bonzini
2013-10-16 18:25 ` Don Zickus
2013-10-16 21:02 ` Marcelo Tosatti
2013-11-06 7:49 ` Gleb Natapov
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=20131009135519.GF227855@redhat.com \
--to=dzickus@redhat.com \
--cc=gleb@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=pbonzini@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).