All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guillaume Chazarain <guichaz@yahoo.fr>
To: Ingo Molnar <mingo@elte.hu>
Cc: David Dillow <dillowda@ornl.gov>,
	linux-kernel@vger.kernel.org, linux-btrace@vger.kernel.org,
	tglx@linutronix.de, Jens Axboe <jens.axboe@oracle.com>,
	nigel@suspend2.net
Subject: Re: CONFIG_NO_HZ breaks blktrace timestamps
Date: Sat, 12 Jan 2008 00:03:53 +0000	[thread overview]
Message-ID: <20080112010353.0eeb18bf@inria.fr> (raw)
In-Reply-To: <20080111233025.57d17f6a@inria.fr>

Guillaume Chazarain <guichaz@yahoo.fr> wrote:

> FYI, I'm currently trying to track down where rq->clock started to
> overflow with nohz=off, and it seems to be before 2.6.23, so my patches
> are not at fault ;-) Or maybe I am dreaming and it was always
> overflowing. Investigating ...

And the winner is:

commit 529c77261bccd9d37f110f58b0753d95beaa9fa2
Author: Ingo Molnar <mingo@elte.hu>
Date:   Fri Aug 10 23:05:11 2007 +0200

    sched: improve rq-clock overflow logic
    
    improve the rq-clock overflow logic: limit the absolute rq->clock
    delta since the last scheduler tick, instead of limiting the delta
    itself.
    
    tested by Arjan van de Ven - whole laptop was misbehaving due to
    an incorrectly calibrated cpu_khz confusing sched_clock().
    
    Signed-off-by: Ingo Molnar <mingo@elte.hu>
    Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

diff --git a/kernel/sched.c b/kernel/sched.c
index b0afd8d..6247e4a 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -263,6 +263,7 @@ struct rq {
 
 	unsigned int clock_warps, clock_overflows;
 	unsigned int clock_unstable_events;
+	u64 tick_timestamp;
 
 	atomic_t nr_iowait;
 
@@ -341,8 +342,11 @@ static void __update_rq_clock(struct rq *rq)
 		/*
 		 * Catch too large forward jumps too:
 		 */
-		if (unlikely(delta > 2*TICK_NSEC)) {
-			clock++;
+		if (unlikely(clock + delta > rq->tick_timestamp + TICK_NSEC)) {
+			if (clock < rq->tick_timestamp + TICK_NSEC)
+				clock = rq->tick_timestamp + TICK_NSEC;
+			else
+				clock++;
 			rq->clock_overflows++;
 		} else {
 			if (unlikely(delta > rq->clock_max_delta))
@@ -3308,9 +3312,16 @@ void scheduler_tick(void)
 	int cpu = smp_processor_id();
 	struct rq *rq = cpu_rq(cpu);
 	struct task_struct *curr = rq->curr;
+	u64 next_tick = rq->tick_timestamp + TICK_NSEC;
 
 	spin_lock(&rq->lock);
 	__update_rq_clock(rq);
+	/*
+	 * Let rq->clock advance by at least TICK_NSEC:
+	 */
+	if (unlikely(rq->clock < next_tick))
+		rq->clock = next_tick;
+	rq->tick_timestamp = rq->clock;
 	update_cpu_load(rq);
 	if (curr != rq->idle) /* FIXME: needed? */
 		curr->sched_class->task_tick(rq, curr);


Seems like I originally was not the only one seeing 2 jiffies jumps ;-)
I'll adapt my patches.

-- 
Guillaume

WARNING: multiple messages have this Message-ID (diff)
From: Guillaume Chazarain <guichaz@yahoo.fr>
To: Ingo Molnar <mingo@elte.hu>
Cc: David Dillow <dillowda@ornl.gov>,
	linux-kernel@vger.kernel.org, linux-btrace@vger.kernel.org,
	tglx@linutronix.de, Jens Axboe <jens.axboe@oracle.com>,
	nigel@suspend2.net
Subject: Re: CONFIG_NO_HZ breaks blktrace timestamps
Date: Sat, 12 Jan 2008 01:03:53 +0100	[thread overview]
Message-ID: <20080112010353.0eeb18bf@inria.fr> (raw)
In-Reply-To: <20080111233025.57d17f6a@inria.fr>

Guillaume Chazarain <guichaz@yahoo.fr> wrote:

> FYI, I'm currently trying to track down where rq->clock started to
> overflow with nohz=off, and it seems to be before 2.6.23, so my patches
> are not at fault ;-) Or maybe I am dreaming and it was always
> overflowing. Investigating ...

And the winner is:

commit 529c77261bccd9d37f110f58b0753d95beaa9fa2
Author: Ingo Molnar <mingo@elte.hu>
Date:   Fri Aug 10 23:05:11 2007 +0200

    sched: improve rq-clock overflow logic
    
    improve the rq-clock overflow logic: limit the absolute rq->clock
    delta since the last scheduler tick, instead of limiting the delta
    itself.
    
    tested by Arjan van de Ven - whole laptop was misbehaving due to
    an incorrectly calibrated cpu_khz confusing sched_clock().
    
    Signed-off-by: Ingo Molnar <mingo@elte.hu>
    Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

diff --git a/kernel/sched.c b/kernel/sched.c
index b0afd8d..6247e4a 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -263,6 +263,7 @@ struct rq {
 
 	unsigned int clock_warps, clock_overflows;
 	unsigned int clock_unstable_events;
+	u64 tick_timestamp;
 
 	atomic_t nr_iowait;
 
@@ -341,8 +342,11 @@ static void __update_rq_clock(struct rq *rq)
 		/*
 		 * Catch too large forward jumps too:
 		 */
-		if (unlikely(delta > 2*TICK_NSEC)) {
-			clock++;
+		if (unlikely(clock + delta > rq->tick_timestamp + TICK_NSEC)) {
+			if (clock < rq->tick_timestamp + TICK_NSEC)
+				clock = rq->tick_timestamp + TICK_NSEC;
+			else
+				clock++;
 			rq->clock_overflows++;
 		} else {
 			if (unlikely(delta > rq->clock_max_delta))
@@ -3308,9 +3312,16 @@ void scheduler_tick(void)
 	int cpu = smp_processor_id();
 	struct rq *rq = cpu_rq(cpu);
 	struct task_struct *curr = rq->curr;
+	u64 next_tick = rq->tick_timestamp + TICK_NSEC;
 
 	spin_lock(&rq->lock);
 	__update_rq_clock(rq);
+	/*
+	 * Let rq->clock advance by at least TICK_NSEC:
+	 */
+	if (unlikely(rq->clock < next_tick))
+		rq->clock = next_tick;
+	rq->tick_timestamp = rq->clock;
 	update_cpu_load(rq);
 	if (curr != rq->idle) /* FIXME: needed? */
 		curr->sched_class->task_tick(rq, curr);


Seems like I originally was not the only one seeing 2 jiffies jumps ;-)
I'll adapt my patches.

-- 
Guillaume

  reply	other threads:[~2008-01-12  0:03 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-09 22:48 CONFIG_NO_HZ breaks blktrace timestamps David Dillow
2008-01-09 22:48 ` David Dillow
2008-01-10 20:25 ` David Dillow
2008-01-10 20:25   ` David Dillow
2008-01-10 22:44   ` Guillaume Chazarain
2008-01-10 22:44     ` Guillaume Chazarain
2008-01-11  3:04     ` David Dillow
2008-01-11  3:04       ` David Dillow
2008-01-11  9:07       ` Jens Axboe
2008-01-11  9:07         ` Jens Axboe
2008-01-11  9:23         ` Ingo Molnar
2008-01-11  9:23           ` Ingo Molnar
2008-01-11  9:25           ` Jens Axboe
2008-01-11  9:25             ` Jens Axboe
2008-01-11  9:42             ` Ingo Molnar
2008-01-11  9:42               ` Ingo Molnar
2008-01-11  9:56               ` Jens Axboe
2008-01-11  9:56                 ` Jens Axboe
2008-01-11 10:29                 ` [patch] block: fix " Ingo Molnar
2008-01-11 10:29                   ` Ingo Molnar
2008-01-11 10:47                   ` Guillaume Chazarain
2008-01-11 10:47                     ` Guillaume Chazarain
2008-01-11 10:50                     ` Ingo Molnar
2008-01-11 10:50                       ` Ingo Molnar
2008-01-11 12:28                   ` Jens Axboe
2008-01-11 12:28                     ` Jens Axboe
2008-01-11 12:42                     ` Jens Axboe
2008-01-11 12:42                       ` Jens Axboe
2008-01-11 13:21                     ` Ingo Molnar
2008-01-11 13:21                       ` Ingo Molnar
2008-01-11 17:18                       ` Jens Axboe
2008-01-11 17:18                         ` Jens Axboe
2008-01-14  7:51                         ` Ingo Molnar
2008-01-14  7:51                           ` Ingo Molnar
2008-01-14  7:59                           ` Ingo Molnar
2008-01-14  7:59                             ` Ingo Molnar
2008-01-14  8:10                             ` Jens Axboe
2008-01-14  8:10                               ` Jens Axboe
2008-01-14  8:39                           ` Christoph Hellwig
2008-01-14  8:42                             ` Jens Axboe
2008-01-14  8:42                               ` Jens Axboe
2008-01-11 15:36                   ` David Dillow
2008-01-11 15:36                     ` David Dillow
2008-01-11 16:44                     ` Ingo Molnar
2008-01-11 16:44                       ` Ingo Molnar
2008-01-11 17:26                       ` Jens Axboe
2008-01-11 17:26                         ` Jens Axboe
2008-01-11  9:29           ` CONFIG_NO_HZ breaks " Ingo Molnar
2008-01-11  9:29             ` Ingo Molnar
2008-01-11  9:34             ` Jens Axboe
2008-01-11  9:34               ` Jens Axboe
2008-01-11  9:28         ` nigel
2008-01-11  9:28           ` nigel
2008-01-11  9:32           ` Ingo Molnar
2008-01-11  9:32             ` Ingo Molnar
2008-01-13 22:54             ` nigel
2008-01-13 22:54               ` nigel
2008-01-11  9:44       ` Ingo Molnar
2008-01-11  9:44         ` Ingo Molnar
2008-01-11  9:51         ` Ingo Molnar
2008-01-11  9:51           ` Ingo Molnar
2008-01-11 10:41     ` Guillaume Chazarain
2008-01-11 10:41       ` Guillaume Chazarain
2008-01-11 10:55       ` Ingo Molnar
2008-01-11 10:55         ` Ingo Molnar
2008-01-11 22:30         ` Guillaume Chazarain
2008-01-11 22:30           ` Guillaume Chazarain
2008-01-12  0:03           ` Guillaume Chazarain [this message]
2008-01-12  0:03             ` Guillaume Chazarain
2008-01-11  9:34   ` Ingo Molnar
2008-01-11  9:34     ` Ingo Molnar
2008-01-11 15:43     ` David Dillow
2008-01-11 15:43       ` David Dillow

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=20080112010353.0eeb18bf@inria.fr \
    --to=guichaz@yahoo.fr \
    --cc=dillowda@ornl.gov \
    --cc=jens.axboe@oracle.com \
    --cc=linux-btrace@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=nigel@suspend2.net \
    --cc=tglx@linutronix.de \
    /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 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.