All of lore.kernel.org
 help / color / mirror / Atom feed
From: riel@redhat.com
To: linux-kernel@vger.kernel.org
Cc: kernellwp@gmail.com, mingo@kernel.org, peterz@infradead.org,
	tglx@linutronix.de, fweisbec@redhat.com
Subject: [PATCH 2/5] nohz,cputime: remove VTIME_GEN vtime irq time code
Date: Tue,  7 Jun 2016 22:30:01 -0400	[thread overview]
Message-ID: <1465353004-15044-3-git-send-email-riel@redhat.com> (raw)
In-Reply-To: <1465353004-15044-1-git-send-email-riel@redhat.com>

From: Rik van Riel <riel@redhat.com>

The CONFIG_VIRT_CPU_ACCOUNTING_GEN irq time tracking code does not
appear to currently work right.

On CPUs that are nohz_full, people typically do not assign IRQs.
On the housekeeping CPU (when a system is booted up with nohz_full),
sampling should work ok to determine irq and softirq time use, but
that only covers the housekeeping CPU itself, not the other
non-nohz_full CPUs.

On CPUs that are nohz_idle (the typical way a distro kernel is
booted), irq time is not accounted at all while the CPU is idle,
due to the lack of timer ticks.

Remove the VTIME_GEN vtime irq time code. The next patch will
allow NO_HZ_FULL kernels to use the IRQ_TIME_ACCOUNTING code.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 include/linux/vtime.h  | 32 +++++++++++++----------------
 kernel/sched/cputime.c | 55 ++++++++++++++++++++++++++++----------------------
 2 files changed, 45 insertions(+), 42 deletions(-)

diff --git a/include/linux/vtime.h b/include/linux/vtime.h
index fa2196990f84..3b384bf5ce1a 100644
--- a/include/linux/vtime.h
+++ b/include/linux/vtime.h
@@ -64,17 +64,6 @@ extern void vtime_account_system(struct task_struct *tsk);
 extern void vtime_account_idle(struct task_struct *tsk);
 extern void vtime_account_user(struct task_struct *tsk);
 
-#ifdef __ARCH_HAS_VTIME_ACCOUNT
-extern void vtime_account_irq_enter(struct task_struct *tsk);
-#else
-extern void vtime_common_account_irq_enter(struct task_struct *tsk);
-static inline void vtime_account_irq_enter(struct task_struct *tsk)
-{
-	if (vtime_accounting_cpu_enabled())
-		vtime_common_account_irq_enter(tsk);
-}
-#endif /* __ARCH_HAS_VTIME_ACCOUNT */
-
 #else /* !CONFIG_VIRT_CPU_ACCOUNTING */
 
 static inline void vtime_task_switch(struct task_struct *prev) { }
@@ -85,13 +74,8 @@ static inline void vtime_account_irq_enter(struct task_struct *tsk) { }
 
 #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN
 extern void arch_vtime_task_switch(struct task_struct *tsk);
-extern void vtime_gen_account_irq_exit(struct task_struct *tsk);
-
-static inline void vtime_account_irq_exit(struct task_struct *tsk)
-{
-	if (vtime_accounting_cpu_enabled())
-		vtime_gen_account_irq_exit(tsk);
-}
+static inline void vtime_account_irq_enter(struct task_struct *tsk) { }
+static inline void vtime_account_irq_exit(struct task_struct *tsk) { }
 
 extern void vtime_user_enter(struct task_struct *tsk);
 
@@ -113,6 +97,18 @@ static inline void vtime_user_exit(struct task_struct *tsk) { }
 static inline void vtime_guest_enter(struct task_struct *tsk) { }
 static inline void vtime_guest_exit(struct task_struct *tsk) { }
 static inline void vtime_init_idle(struct task_struct *tsk, int cpu) { }
+
+#ifdef __ARCH_HAS_VTIME_ACCOUNT
+extern void vtime_account_irq_enter(struct task_struct *tsk);
+#else
+extern void vtime_common_account_irq_enter(struct task_struct *tsk);
+static inline void vtime_account_irq_enter(struct task_struct *tsk)
+{
+	if (vtime_accounting_cpu_enabled())
+		vtime_common_account_irq_enter(tsk);
+}
+#endif /* __ARCH_HAS_VTIME_ACCOUNT */
+
 #endif
 
 #ifdef CONFIG_IRQ_TIME_ACCOUNTING
diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 4bd6d1b774ab..2f862dfdb520 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -103,10 +103,10 @@ static unsigned long irqtime_account_si_update(void)
 	u64 softirq;
 
 	local_irq_save(flags);
-	delta = this_cpu_read(cpu_softirq_time) - cpustat[CPUTIME_SOFTIRQ];
-	si_jiffies = cputime_to_jiffies(delta);
+	softirq = this_cpu_read(cpu_softirq_time) - cpustat[CPUTIME_SOFTIRQ];
+	si_jiffies = cputime_to_jiffies(softirq);
 	if (si_jiffies)
-		cpustat[CPUSTIME_SOFTIRQ] += jiffies_to_cputime(si_jiffies);
+		cpustat[CPUTIME_SOFTIRQ] += jiffies_to_cputime(si_jiffies);
 	local_irq_restore(flags);
 	return si_jiffies;
 }
@@ -115,6 +115,16 @@ static unsigned long irqtime_account_si_update(void)
 
 #define sched_clock_irqtime	(0)
 
+static unsigned long irqtime_account_hi_update(void)
+{
+	return 0;
+}
+
+static unsigned long irqtime_account_si_update(void)
+{
+	return 0;
+}
+
 #endif /* !CONFIG_IRQ_TIME_ACCOUNTING */
 
 static inline void task_group_account_field(struct task_struct *p, int index,
@@ -346,7 +356,6 @@ static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
 {
 	cputime_t scaled = cputime_to_scaled(cputime_one_jiffy);
 	u64 cputime = (__force u64) cputime_one_jiffy;
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
 	unsigned long other;
 
 	/*
@@ -703,16 +712,26 @@ static cputime_t get_vtime_delta(struct task_struct *tsk)
 	return jiffies_to_cputime(delta);
 }
 
+/* Account per-cpu irq, softirq, and steal time. Not accounted to a task. */
+static cputime_t __vtime_account_other(void)
+{
+	unsigned long ticks;
+
+	ticks = steal_account_process_tick();
+	ticks += irqtime_account_hi_update();
+	ticks += irqtime_account_si_update();
+
+	return jiffies_to_cputime(ticks);
+}
+
 static void __vtime_account_system(struct task_struct *tsk)
 {
-	cputime_t steal_time;
 	cputime_t delta_cpu = get_vtime_delta(tsk);
-	unsigned long delta_st = steal_account_process_tick();
-	steal_time = jiffies_to_cputime(delta_st);
+	cputime_t other = __vtime_account_other();
 
-	if (steal_time >= delta_cpu)
+	if (other >= delta_cpu)
 		return;
-	delta_cpu -= steal_time;
+	delta_cpu -= other;
 	account_system_time(tsk, irq_count(), delta_cpu, cputime_to_scaled(delta_cpu));
 }
 
@@ -726,16 +745,6 @@ void vtime_account_system(struct task_struct *tsk)
 	write_seqcount_end(&tsk->vtime_seqcount);
 }
 
-void vtime_gen_account_irq_exit(struct task_struct *tsk)
-{
-	write_seqcount_begin(&tsk->vtime_seqcount);
-	if (vtime_delta(tsk))
-		__vtime_account_system(tsk);
-	if (context_tracking_in_user())
-		tsk->vtime_snap_whence = VTIME_USER;
-	write_seqcount_end(&tsk->vtime_seqcount);
-}
-
 void vtime_account_user(struct task_struct *tsk)
 {
 	cputime_t delta_cpu;
@@ -743,16 +752,14 @@ void vtime_account_user(struct task_struct *tsk)
 	write_seqcount_begin(&tsk->vtime_seqcount);
 	tsk->vtime_snap_whence = VTIME_SYS;
 	if (vtime_delta(tsk)) {
-		cputime_t steal_time;
-		unsigned long delta_st = steal_account_process_tick();
+		cputime_t other = __vtime_account_other();
 		delta_cpu = get_vtime_delta(tsk);
-		steal_time = jiffies_to_cputime(delta_st);
 
-		if (steal_time >= delta_cpu) {
+		if (other >= delta_cpu) {
 			write_seqcount_end(&tsk->vtime_seqcount);
 			return;
 		}
-		delta_cpu -= steal_time;
+		delta_cpu -= other;
 		account_user_time(tsk, delta_cpu, cputime_to_scaled(delta_cpu));
 	}
 	write_seqcount_end(&tsk->vtime_seqcount);
-- 
2.5.5

  parent reply	other threads:[~2016-06-08  2:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-08  2:29 [PATCH RFC 0/5] sched,time: make irq time accounting work for nohz_idle riel
2016-06-08  2:30 ` [PATCH 1/5] sched,time: count actually elapsed irq & softirq time riel
2016-06-08  2:30 ` riel [this message]
2016-06-08  2:30 ` [PATCH 3/5] cputime: allow irq time accounting to be selected as an option riel
2016-06-08  2:30 ` [PATCH 4/5] irqtime: add irq type parameter to irqtime_account_irq riel
2016-06-08  2:30 ` [PATCH 5/5] irqtime: drop local_irq_save/restore from irqtime_account_irq riel
  -- strict thread matches above, loose matches on Subject: below --
2016-06-16 16:06 [PATCH 0/5] sched,time: fix irq time accounting with nohz_idle riel
2016-06-16 16:06 ` [PATCH 2/5] nohz,cputime: remove VTIME_GEN vtime irq time code riel
2016-06-23  2:25 [PATCH v2 0/5] sched,time: fix irq time accounting with nohz_idle riel
2016-06-23  2:25 ` [PATCH 2/5] nohz,cputime: remove VTIME_GEN vtime irq time code riel
2016-06-27 23:21   ` Frederic Weisbecker
2016-06-27 23:31     ` Rik van Riel
2016-06-27 23:51       ` 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=1465353004-15044-3-git-send-email-riel@redhat.com \
    --to=riel@redhat.com \
    --cc=fweisbec@redhat.com \
    --cc=kernellwp@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --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.