All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <frederic@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Wei Li <liwei391@huawei.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>,
	Yu Liao <liaoyu15@huawei.com>, Hillf Danton <hdanton@sina.com>,
	Ingo Molnar <mingo@kernel.org>
Subject: [PATCH 3/8] timers/nohz: Protect idle/iowait sleep time under seqcount
Date: Wed, 22 Feb 2023 15:46:44 +0100	[thread overview]
Message-ID: <20230222144649.624380-4-frederic@kernel.org> (raw)
In-Reply-To: <20230222144649.624380-1-frederic@kernel.org>

Reading idle/io sleep time (eg: from /proc/stat) can race with idle exit
updates because the state machine handling the stats is not atomic and
requires a coherent read batch.

As a result reading the sleep time may report irrelevant or backward
values.

Fix this with protecting the simple state machine within a seqcount.
This is expected to be cheap enough not to add measurable performance
impact on the idle path.

Note this only fixes reader VS writer condition partitially. A race
remains that involves remote updates of the CPU iowait task counter. It
can hardly be fixed.

Reported-by: Yu Liao <liaoyu15@huawei.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Hillf Danton <hdanton@sina.com>
Cc: Yu Liao <liaoyu15@huawei.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wei Li <liwei391@huawei.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 kernel/time/tick-sched.c | 22 ++++++++++++++++------
 kernel/time/tick-sched.h |  1 +
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 9058b9eb8bc1..90d9b7b29875 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -646,6 +646,7 @@ static void tick_nohz_stop_idle(struct tick_sched *ts, ktime_t now)
 
 	delta = ktime_sub(now, ts->idle_entrytime);
 
+	write_seqcount_begin(&ts->idle_sleeptime_seq);
 	if (nr_iowait_cpu(smp_processor_id()) > 0)
 		ts->iowait_sleeptime = ktime_add(ts->iowait_sleeptime, delta);
 	else
@@ -653,14 +654,18 @@ static void tick_nohz_stop_idle(struct tick_sched *ts, ktime_t now)
 
 	ts->idle_entrytime = now;
 	ts->idle_active = 0;
+	write_seqcount_end(&ts->idle_sleeptime_seq);
 
 	sched_clock_idle_wakeup_event();
 }
 
 static void tick_nohz_start_idle(struct tick_sched *ts)
 {
+	write_seqcount_begin(&ts->idle_sleeptime_seq);
 	ts->idle_entrytime = ktime_get();
 	ts->idle_active = 1;
+	write_seqcount_end(&ts->idle_sleeptime_seq);
+
 	sched_clock_idle_sleep_event();
 }
 
@@ -668,6 +673,7 @@ static u64 get_cpu_sleep_time_us(struct tick_sched *ts, ktime_t *sleeptime,
 				 bool compute_delta, u64 *last_update_time)
 {
 	ktime_t now, idle;
+	unsigned int seq;
 
 	if (!tick_nohz_active)
 		return -1;
@@ -676,13 +682,17 @@ static u64 get_cpu_sleep_time_us(struct tick_sched *ts, ktime_t *sleeptime,
 	if (last_update_time)
 		*last_update_time = ktime_to_us(now);
 
-	if (ts->idle_active && compute_delta) {
-		ktime_t delta = ktime_sub(now, ts->idle_entrytime);
+	do {
+		seq = read_seqcount_begin(&ts->idle_sleeptime_seq);
 
-		idle = ktime_add(*sleeptime, delta);
-	} else {
-		idle = *sleeptime;
-	}
+		if (ts->idle_active && compute_delta) {
+			ktime_t delta = ktime_sub(now, ts->idle_entrytime);
+
+			idle = ktime_add(*sleeptime, delta);
+		} else {
+			idle = *sleeptime;
+		}
+	} while (read_seqcount_retry(&ts->idle_sleeptime_seq, seq));
 
 	return ktime_to_us(idle);
 
diff --git a/kernel/time/tick-sched.h b/kernel/time/tick-sched.h
index c6663254d17d..5ed5a9d41d5a 100644
--- a/kernel/time/tick-sched.h
+++ b/kernel/time/tick-sched.h
@@ -75,6 +75,7 @@ struct tick_sched {
 	ktime_t				idle_waketime;
 
 	/* Idle entry */
+	seqcount_t			idle_sleeptime_seq;
 	ktime_t				idle_entrytime;
 
 	/* Tick stop */
-- 
2.34.1


  parent reply	other threads:[~2023-02-22 14:47 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-22 14:46 [PATCH 0/8] timers/nohz: Fixes and cleanups v3 Frederic Weisbecker
2023-02-22 14:46 ` [PATCH 1/8] timers/nohz: Restructure and reshuffle struct tick_sched Frederic Weisbecker
2023-04-18 14:53   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2023-02-22 14:46 ` [PATCH 2/8] timers/nohz: Only ever update sleeptime from idle exit Frederic Weisbecker
2023-04-18 14:53   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2023-02-22 14:46 ` Frederic Weisbecker [this message]
2023-04-18 14:53   ` [tip: timers/core] timers/nohz: Protect idle/iowait sleep time under seqcount tip-bot2 for Frederic Weisbecker
2023-02-22 14:46 ` [PATCH 4/8] timers/nohz: Add a comment about broken iowait counter update race Frederic Weisbecker
2023-04-18 14:53   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2023-02-22 14:46 ` [PATCH 5/8] timers/nohz: Remove middle-function __tick_nohz_idle_stop_tick() Frederic Weisbecker
2023-04-18 14:53   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2023-02-22 14:46 ` [PATCH 6/8] MAINTAINERS: Remove stale email address Frederic Weisbecker
2023-04-18 14:53   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2023-02-22 14:46 ` [PATCH 7/8] selftests/proc: Remove idle time monotonicity assertions Frederic Weisbecker
2023-04-18 14:53   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2023-02-22 14:46 ` [PATCH 8/8] selftests/proc: Assert clock_gettime(CLOCK_BOOTTIME) VS /proc/uptime monotonicity Frederic Weisbecker
2023-03-08 15:59   ` Mirsad Todorovac
2023-03-21 12:44     ` Frederic Weisbecker
2023-03-26 20:03       ` Mirsad Goran Todorovac
2023-04-18 14:53   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker

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=20230222144649.624380-4-frederic@kernel.org \
    --to=frederic@kernel.org \
    --cc=adobriyan@gmail.com \
    --cc=hdanton@sina.com \
    --cc=liaoyu15@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liwei391@huawei.com \
    --cc=mingo@kernel.org \
    --cc=mirsad.todorovac@alu.unizg.hr \
    --cc=peterz@infradead.org \
    --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.