public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Arun Sharma <asharma@fb.com>
Cc: linux-kernel@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@infradead.org>,
	Andrew Vagin <avagin@openvz.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Ingo Molnar <mingo@elte.hu>, Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH] trace: reset sleep/block start time on task switch
Date: Tue, 24 Jan 2012 15:27:35 +0100	[thread overview]
Message-ID: <1327415255.2614.33.camel@laptop> (raw)
In-Reply-To: <4F1DE6FE.4000603@fb.com>

On Mon, 2012-01-23 at 15:02 -0800, Arun Sharma wrote:
> > +++ b/kernel/sched/fair.c
> > @@ -1191,6 +1191,9 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> >               if (entity_is_task(se)) {
> >                       struct task_struct *tsk = task_of(se);
> >
> > +                     se->statistics.sleep_start = 0;
> > +                     se->statistics.block_start = 0;
> > +
> 
> We might still need some additional logic to ignore sleep_start if the 
> last context switch was a preemption. Test case Andrew Vagin posted on 
> 12/21:
> 
> nanosleep();
> s = time(NULL);
> while (time(NULL) - s < 4);
> 
> During the busy wait while loop, sleep_start is non-zero and the first 
> sample from sched_stat_sleeptime() and anyone else doing the (now - 
> sleep_start) computation would get a bogus value until the next dequeue. 

Bah, you're right. Also yes your proposal is too intrusive, but that can
be fixed, I actually did, but then I noticed its broken too, it doesn't
matter if the schedule that schedules a task back in preempted another
task or not, what matters is if the task we're scheduling back in was
itself preempted or recently woken. And we simply don't know.

I'm tempted to revert 1ac9bc69 for now, userspace will simply have to
correlate trace_sched_switch() and trace_sched_stat_{sleep,blocked}(),
which shouldn't be too hard.




  reply	other threads:[~2012-01-24 14:27 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-20  2:20 [PATCH] trace: reset sleep/block start time on task switch Arun Sharma
2012-01-23 11:34 ` Peter Zijlstra
2012-01-23 18:41   ` Arun Sharma
2012-01-23 21:03     ` Peter Zijlstra
2012-01-23 23:02       ` Arun Sharma
2012-01-24 14:27         ` Peter Zijlstra [this message]
2012-01-24 21:46           ` Arun Sharma
2012-01-25  9:20             ` Frederic Weisbecker
2012-01-25 19:50               ` Arun Sharma
2012-01-25 20:15                 ` Steven Rostedt
2012-01-25 22:29                   ` Arun Sharma
2012-01-26  2:27                     ` Frederic Weisbecker
2012-01-26 19:13                       ` Arun Sharma
2012-01-26  2:21                 ` Frederic Weisbecker
2012-02-10 18:43                 ` Peter Zijlstra
2012-02-10 20:07                   ` Arun Sharma

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=1327415255.2614.33.camel@laptop \
    --to=a.p.zijlstra@chello.nl \
    --cc=acme@infradead.org \
    --cc=asharma@fb.com \
    --cc=avagin@openvz.org \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rostedt@goodmis.org \
    /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