mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] Finer granularity and task/cgroup irq time accounting
@ 2010-05-25  0:11 Venkatesh Pallipadi
  2010-05-25  0:11 ` [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time Venkatesh Pallipadi
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Venkatesh Pallipadi @ 2010-05-25  0:11 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	Balbir Singh, Paul Menage
  Cc: linux-kernel, Paul Turner

Currently, the softirq and hardirq time reporting is only done at the
CPU level. There are usecases where reporting this time against task
or task groups or cgroups will be useful for user/administrator
in terms of resource planning and utilization charging. Also, as the
accoounting is already done at the CPU level, reporting the same at
the task level does not add any significant computational overhead
other than task level storage (patch 1).

The softirq/hardirq statistics commonly done based on tick based sampling.
Though some archs have CONFIG_VIRT_CPU_ACCOUNTING based fine granularity
accounting. Having similar mechanism to get fine granularity accounting
on x86 will be a major challenge, given the state of TSC reliability
on various platforms and also the overhead it may add in common paths
like syscall entry exit.

An alternative is to have a generic (sched_clock based) and configurable
fine-granularity accounting of si and hi time which can be reported
over the /proc/<pid>/stat API (patch 2).

Patch 3 and 4 are exporting this info at the cgroup level.

Does exposing this additional info to user makes sense? Any feedback on
the way it is done in this patchset?

This precise irq time based on sched_clock() provides some potential
opportunities to handle the softirq time charging in a more fair way.
Specifically cases where an unrelated task is being penalized for
irq load on that CPU.
* With network Receive Flow Steering, for example; We can potentially
do things like not charge receive softirq time to the process that is
currently running and charge it instead to the actual consumer of
the receive (in recvmsg, for example).
* We can reduce the power of the CPU to account for softirq/hardirq
load, in order to increase the scheduler fairness for tasks running on
that CPU.

Comments?

Thanks,
Venki


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time
  2010-05-25  0:11 [RFC PATCH 0/4] Finer granularity and task/cgroup irq time accounting Venkatesh Pallipadi
@ 2010-05-25  0:11 ` Venkatesh Pallipadi
  2010-05-25  0:11   ` [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task Venkatesh Pallipadi
  2010-05-25  9:13   ` [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time Balbir Singh
  2010-05-25  7:00 ` [RFC PATCH 0/4] Finer granularity and task/cgroup irq time accounting Balbir Singh
  2010-05-25  8:14 ` Peter Zijlstra
  2 siblings, 2 replies; 13+ messages in thread
From: Venkatesh Pallipadi @ 2010-05-25  0:11 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	Balbir Singh, Paul Menage
  Cc: linux-kernel, Paul Turner, Venkatesh Pallipadi

Currently, kernel does not have accounting mechanism for softirq and hardirq
times at the task level. There is irq time info in kstat_cpu which is
accumulated at the cpu level.

Without the task level information, the non irq run time of task(s) would
have to be guessed based on their exec time and CPU on which they were
running recently and assuming that the CPU irq time reported are spread
across all the tasks running there. And this guess can be widely off the mark.

Sample case, considering just the softirq:

If there are varied workloads running on a CPU, say a CPU bound task (loop)
and a network IO bound task (nc) along with the network softirq load,
there is no way for the administrator/user to know the non-irq runtime of each
of these tasks. Only information available is the total runtime for each of the
tasks and kstat_cpu softirq time for the CPU.

In this example, considering a 10 second sample, both loop and nc would have
total run time of ~5s. And kstat_cpu softirq on this cpu increase was
355 (~3.5s).

So, all the information the user gets is that both the tasks are running for
roughly the same amount of time and softirq is around 35%. As a result user
may conclude that irq overhead for both tasks are equal (1.75s) and the
non-irq runtime of both the tasks are around ~3.25s. Yes. There is another
factor of system and user time reported for these tasks that I am ignoring
as that is tough to correlate with irq time, in cases where the tasks have
significant non-irq system time.

This change adds tracking of softirq time on each task and task group.
This information is exported in /proc/<pid>/stat.

So, the user can get info like below, looking at exec_time and si_time in
appropriate /proc/<pid>/stat.
(Taken for a 10s interval)
task exec_time softirqtime (in USER_HZ)
(loop)  (nc)
505 0   500 359
502 1   501 363
503 0   502 354
504 0   499 359
503 3   500 360

with this, user can get the non-irq run time as 5s and ~1.45s for
loop and nc, respectively.

Signed-off-by: Venkatesh Pallipadi <venki@google.com>
---
 Documentation/filesystems/proc.txt |    5 ++++-
 fs/proc/array.c                    |   17 +++++++++++++++--
 include/linux/sched.h              |    4 ++++
 kernel/exit.c                      |    2 ++
 kernel/sched.c                     |    9 ++++++---
 5 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 9fb6cbe..295f1e3 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -255,7 +255,7 @@ Table 1-3: Contents of the statm files (as of 2.6.8-rc3)
 ..............................................................................
 
 
-Table 1-4: Contents of the stat files (as of 2.6.30-rc7)
+Table 1-4: Contents of the stat files (current kernel version)
 ..............................................................................
  Field          Content
   pid           process id
@@ -303,6 +303,9 @@ Table 1-4: Contents of the stat files (as of 2.6.30-rc7)
   blkio_ticks   time spent waiting for block IO
   gtime         guest time of the task in jiffies
   cgtime        guest time of the task children in jiffies
+  exec_time	execution time as accounted by scheduler
+  si_time	softirq time that was accounted to this task (or taskgroup)
+  hi_time	hardirq time that was accounted to this task (or taskgroup)
 ..............................................................................
 
 The /proc/PID/maps file containing the currently mapped memory regions and
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 885ab55..401a1c0 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -380,6 +380,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	unsigned long rsslim = 0;
 	char tcomm[sizeof(task->comm)];
 	unsigned long flags;
+	cputime64_t exec_time = 0, si_time = 0, hi_time = 0;
 
 	state = *get_task_state(task);
 	vsize = eip = esp = 0;
@@ -427,6 +428,9 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 				min_flt += t->min_flt;
 				maj_flt += t->maj_flt;
 				gtime = cputime_add(gtime, t->gtime);
+				si_time = cputime64_add(si_time, t->si_time);
+				hi_time = cputime64_add(hi_time, t->hi_time);
+				exec_time += t->se.sum_exec_runtime;
 				t = next_thread(t);
 			} while (t != task);
 
@@ -434,6 +438,9 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 			maj_flt += sig->maj_flt;
 			thread_group_times(task, &utime, &stime);
 			gtime = cputime_add(gtime, sig->gtime);
+			si_time = cputime64_add(si_time, sig->si_time);
+			hi_time = cputime64_add(hi_time, sig->hi_time);
+			exec_time += sig->sum_sched_runtime;
 		}
 
 		sid = task_session_nr_ns(task, ns);
@@ -448,6 +455,9 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	if (!whole) {
 		min_flt = task->min_flt;
 		maj_flt = task->maj_flt;
+		si_time = task->si_time;
+		hi_time = task->hi_time;
+		exec_time = task->se.sum_exec_runtime;
 		task_times(task, &utime, &stime);
 		gtime = task->gtime;
 	}
@@ -467,7 +477,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 
 	seq_printf(m, "%d (%s) %c %d %d %d %d %d %u %lu \
 %lu %lu %lu %lu %lu %ld %ld %ld %ld %d 0 %llu %lu %ld %lu %lu %lu %lu %lu \
-%lu %lu %lu %lu %lu %lu %lu %lu %d %d %u %u %llu %lu %ld\n",
+%lu %lu %lu %lu %lu %lu %lu %lu %d %d %u %u %llu %lu %ld %llu %llu %llu\n",
 		pid_nr_ns(pid, ns),
 		tcomm,
 		state,
@@ -514,7 +524,10 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 		task->policy,
 		(unsigned long long)delayacct_blkio_ticks(task),
 		cputime_to_clock_t(gtime),
-		cputime_to_clock_t(cgtime));
+		cputime_to_clock_t(cgtime),
+		nsec_to_clock_t(exec_time),
+		(unsigned long long)cputime64_to_clock_t(si_time),
+		(unsigned long long)cputime64_to_clock_t(hi_time));
 	if (mm)
 		mmput(mm);
 	return 0;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index b55e988..7bd82ab 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -593,6 +593,8 @@ struct signal_struct {
 	 */
 	cputime_t utime, stime, cutime, cstime;
 	cputime_t gtime;
+	cputime64_t si_time;
+	cputime64_t hi_time;
 	cputime_t cgtime;
 #ifndef CONFIG_VIRT_CPU_ACCOUNTING
 	cputime_t prev_utime, prev_stime;
@@ -1284,6 +1286,8 @@ struct task_struct {
 
 	cputime_t utime, stime, utimescaled, stimescaled;
 	cputime_t gtime;
+	cputime64_t si_time;
+	cputime64_t hi_time;
 #ifndef CONFIG_VIRT_CPU_ACCOUNTING
 	cputime_t prev_utime, prev_stime;
 #endif
diff --git a/kernel/exit.c b/kernel/exit.c
index eabca5a..de988b3 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -115,6 +115,8 @@ static void __exit_signal(struct task_struct *tsk)
 		sig->utime = cputime_add(sig->utime, tsk->utime);
 		sig->stime = cputime_add(sig->stime, tsk->stime);
 		sig->gtime = cputime_add(sig->gtime, tsk->gtime);
+		sig->si_time = cputime64_add(sig->si_time, tsk->si_time);
+		sig->hi_time = cputime64_add(sig->hi_time, tsk->hi_time);
 		sig->min_flt += tsk->min_flt;
 		sig->maj_flt += tsk->maj_flt;
 		sig->nvcsw += tsk->nvcsw;
diff --git a/kernel/sched.c b/kernel/sched.c
index d9c0368..b410d5f 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -3253,12 +3253,15 @@ void account_system_time(struct task_struct *p, int hardirq_offset,
 
 	/* Add system time to cpustat. */
 	tmp = cputime_to_cputime64(cputime);
-	if (hardirq_count() - hardirq_offset)
+	if (hardirq_count() - hardirq_offset) {
 		cpustat->irq = cputime64_add(cpustat->irq, tmp);
-	else if (softirq_count())
+		p->hi_time = cputime64_add(p->hi_time, tmp);
+	} else if (softirq_count()) {
 		cpustat->softirq = cputime64_add(cpustat->softirq, tmp);
-	else
+		p->si_time = cputime64_add(p->si_time, tmp);
+	} else {
 		cpustat->system = cputime64_add(cpustat->system, tmp);
+	}
 
 	cpuacct_update_stats(p, CPUACCT_STAT_SYSTEM, cputime);
 
-- 
1.7.0.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task
  2010-05-25  0:11 ` [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time Venkatesh Pallipadi
@ 2010-05-25  0:11   ` Venkatesh Pallipadi
  2010-05-25  0:11     ` [RFC PATCH 3/4] sched: Generalize cpuacct usage tracking making it simpler to add new stats Venkatesh Pallipadi
  2010-05-25  6:35     ` [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task Peter Zijlstra
  2010-05-25  9:13   ` [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time Balbir Singh
  1 sibling, 2 replies; 13+ messages in thread
From: Venkatesh Pallipadi @ 2010-05-25  0:11 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	Balbir Singh, Paul Menage
  Cc: linux-kernel, Paul Turner, Venkatesh Pallipadi

Some archs have support for CONFIG_VIRT_CPU_ACCOUNTING which does
the fine granularity accounting of user, system, hardirq, softirq times.
Adding that option on archs like x86 may be challenging however, given the
state of TSC reliability on various platforms and also the overhead it may
ass in syscall entry exit.

Instead, add an option that only does finer accounting of hardirq softirq,
providing precise irq times (instead of timer ticks based samples). This
accounting is added with a new config option CONFIG_IRQ_TIME_ACCOUNTING
so that there wont be any overhead for users not interested in paying the
perf penalty. And this accounting is based on sched_clock, so other archs
may find it useful as well.

Note that the kstat_cpu irq times are still based on tick based samples
and only the task irq times report this new finer granularity irq time.
The reason being that the kstat irq also includes system time and
changing only irq time to have finer granularity can result in inconsistency
like sum kstat time adding up to more than 100% etc.

Continuing with the example from previous patch, without finer
granularity accounting, exec_time and si_time in 10s intervals would be
(appropriate fields of /proc/<pid>/stat)
(loop)  (nc)
505 0   500 359
502 1   501 363
503 0   502 354
504 0   499 359
503 3   500 360

And with finer granularity accounting they would be
(loop)  (nc)
503 9   502 301
502 8   502 303
502 9   501 302
502 8   502 302
503 9   501 302

Signed-off-by: Venkatesh Pallipadi <venki@google.com>
---
 arch/x86/Kconfig        |   11 +++++++++++
 fs/proc/array.c         |   11 +++++++++--
 include/linux/hardirq.h |    4 +++-
 kernel/sched.c          |   35 +++++++++++++++++++++++++++++++++--
 4 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a2d3a5f..d34e305 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -364,6 +364,17 @@ endif
 # This is an alphabetically sorted list of 64 bit extended platforms
 # Please maintain the alphabetic order if and when there are additions
 
+config IRQ_TIME_ACCOUNTING
+	bool "Fine granularity task level IRQ time accounting"
+	default n
+	help
+	  Select this option to enable fine granularity task irq time
+	  accounting. This is done by reading a timestamp on each
+	  transitions between softirq and hardirq state, so there is a
+	  small performance impact.
+
+	  If in doubt, say N here.
+
 config X86_VSMP
 	bool "ScaleMP vSMP"
 	select PARAVIRT
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 401a1c0..16b755e 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -360,6 +360,13 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 	return 0;
 }
 
+#ifdef CONFIG_IRQ_TIME_ACCOUNTING
+/* [hi|si]_time are stored in ns units as cputime may not be fine enough */
+#define IRQTIME_TO_CLOCK_T(x)	nsec_to_clock_t(x)
+#else
+#define IRQTIME_TO_CLOCK_T(x)	cputime64_to_clock_t(x)
+#endif
+
 static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 			struct pid *pid, struct task_struct *task, int whole)
 {
@@ -526,8 +533,8 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 		cputime_to_clock_t(gtime),
 		cputime_to_clock_t(cgtime),
 		nsec_to_clock_t(exec_time),
-		(unsigned long long)cputime64_to_clock_t(si_time),
-		(unsigned long long)cputime64_to_clock_t(hi_time));
+		(unsigned long long)IRQTIME_TO_CLOCK_T(si_time),
+		(unsigned long long)IRQTIME_TO_CLOCK_T(hi_time));
 	if (mm)
 		mmput(mm);
 	return 0;
diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h
index d5b3876..bfafd29 100644
--- a/include/linux/hardirq.h
+++ b/include/linux/hardirq.h
@@ -132,10 +132,12 @@ extern void synchronize_irq(unsigned int irq);
 
 struct task_struct;
 
-#ifndef CONFIG_VIRT_CPU_ACCOUNTING
+#if !defined(CONFIG_VIRT_CPU_ACCOUNTING) && !defined(CONFIG_IRQ_TIME_ACCOUNTING)
 static inline void account_system_vtime(struct task_struct *tsk)
 {
 }
+#else
+extern void account_system_vtime(struct task_struct *tsk);
 #endif
 
 #if defined(CONFIG_NO_HZ)
diff --git a/kernel/sched.c b/kernel/sched.c
index b410d5f..e6090ff 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -3228,6 +3228,13 @@ static void account_guest_time(struct task_struct *p, cputime_t cputime,
 	}
 }
 
+#ifdef CONFIG_IRQ_TIME_ACCOUNTING
+/* In this case, we charge the task at irq time for finer accounting */
+#define TASK_IRQ_CHARGE_AT_TICK(x, y)	(x)
+#else
+#define TASK_IRQ_CHARGE_AT_TICK(x, y)	cputime64_add(x, y)
+#endif
+
 /*
  * Account system cpu time to a process.
  * @p: the process that the cpu time gets accounted to
@@ -3255,10 +3262,10 @@ void account_system_time(struct task_struct *p, int hardirq_offset,
 	tmp = cputime_to_cputime64(cputime);
 	if (hardirq_count() - hardirq_offset) {
 		cpustat->irq = cputime64_add(cpustat->irq, tmp);
-		p->hi_time = cputime64_add(p->hi_time, tmp);
+		p->hi_time = TASK_IRQ_CHARGE_AT_TICK(p->hi_time, tmp);
 	} else if (softirq_count()) {
 		cpustat->softirq = cputime64_add(cpustat->softirq, tmp);
-		p->si_time = cputime64_add(p->si_time, tmp);
+		p->si_time = TASK_IRQ_CHARGE_AT_TICK(p->si_time, tmp);
 	} else {
 		cpustat->system = cputime64_add(cpustat->system, tmp);
 	}
@@ -8967,3 +8974,27 @@ void synchronize_sched_expedited(void)
 EXPORT_SYMBOL_GPL(synchronize_sched_expedited);
 
 #endif /* #else #ifndef CONFIG_SMP */
+
+#ifdef CONFIG_IRQ_TIME_ACCOUNTING
+
+static DEFINE_PER_CPU(u64, irq_start_time);
+
+void account_system_vtime(struct task_struct *tsk)
+{
+	unsigned long flags;
+	int cpu;
+	u64 now;
+
+	local_irq_save(flags);
+	cpu = task_cpu(tsk);
+	now = sched_clock_cpu(cpu);
+	if (hardirq_count())
+		tsk->hi_time += now - per_cpu(irq_start_time, cpu);
+	else if (softirq_count())
+		tsk->si_time += now - per_cpu(irq_start_time, cpu);
+
+	per_cpu(irq_start_time, cpu) = now;
+	local_irq_restore(flags);
+}
+
+#endif
-- 
1.7.0.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 3/4] sched: Generalize cpuacct usage tracking making it simpler to add new stats
  2010-05-25  0:11   ` [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task Venkatesh Pallipadi
@ 2010-05-25  0:11     ` Venkatesh Pallipadi
  2010-05-25  0:11       ` [RFC PATCH 4/4] sched: Export irq times through cpuacct cgroup Venkatesh Pallipadi
  2010-05-25  6:35     ` [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task Peter Zijlstra
  1 sibling, 1 reply; 13+ messages in thread
From: Venkatesh Pallipadi @ 2010-05-25  0:11 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	Balbir Singh, Paul Menage
  Cc: linux-kernel, Paul Turner, Venkatesh Pallipadi

Generalize cpuacct usage, making it easier to add new stats in the following
patch.

Also adds alloc_percpu_array() interface in percpu.h

Signed-off-by: Venkatesh Pallipadi <venki@google.com>
---
 include/linux/percpu.h |    4 ++++
 kernel/sched.c         |   39 ++++++++++++++++++++++++++-------------
 kernel/sched_fair.c    |    2 +-
 kernel/sched_rt.c      |    2 +-
 4 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/include/linux/percpu.h b/include/linux/percpu.h
index d3a38d6..216f96a 100644
--- a/include/linux/percpu.h
+++ b/include/linux/percpu.h
@@ -167,6 +167,10 @@ extern phys_addr_t per_cpu_ptr_to_phys(void *addr);
 #define alloc_percpu(type)	\
 	(typeof(type) __percpu *)__alloc_percpu(sizeof(type), __alignof__(type))
 
+#define alloc_percpu_array(type, size)	\
+	(typeof(type) __percpu *)__alloc_percpu(sizeof(type) * size, \
+						__alignof__(type))
+
 /*
  * Optional methods for optimized non-lvalue per-cpu variable access.
  *
diff --git a/kernel/sched.c b/kernel/sched.c
index e6090ff..d7d7efe 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -1402,12 +1402,20 @@ enum cpuacct_stat_index {
 	CPUACCT_STAT_NSTATS,
 };
 
+enum cpuacct_charge_index {
+	CPUACCT_CHARGE_USAGE,	/* ... execution time */
+
+	CPUACCT_CHARGE_NCHARGES,
+};
+
 #ifdef CONFIG_CGROUP_CPUACCT
-static void cpuacct_charge(struct task_struct *tsk, u64 cputime);
+static void cpuacct_charge(struct task_struct *tsk,
+		enum cpuacct_charge_index idx, u64 cputime);
 static void cpuacct_update_stats(struct task_struct *tsk,
 		enum cpuacct_stat_index idx, cputime_t val);
 #else
-static inline void cpuacct_charge(struct task_struct *tsk, u64 cputime) {}
+static inline void cpuacct_charge(struct task_struct *tsk,
+		enum cpuacct_charge_index idx, u64 cputime) {}
 static inline void cpuacct_update_stats(struct task_struct *tsk,
 		enum cpuacct_stat_index idx, cputime_t val) {}
 #endif
@@ -8640,7 +8648,7 @@ struct cgroup_subsys cpu_cgroup_subsys = {
 /* track cpu usage of a group of tasks and its child groups */
 struct cpuacct {
 	struct cgroup_subsys_state css;
-	/* cpuusage holds pointer to a u64-type object on every cpu */
+	/* cpuusage holds pointer to a u64-type array object on every cpu */
 	u64 __percpu *cpuusage;
 	struct percpu_counter cpustat[CPUACCT_STAT_NSTATS];
 	struct cpuacct *parent;
@@ -8672,7 +8680,7 @@ static struct cgroup_subsys_state *cpuacct_create(
 	if (!ca)
 		goto out;
 
-	ca->cpuusage = alloc_percpu(u64);
+	ca->cpuusage = alloc_percpu_array(u64, CPUACCT_CHARGE_NCHARGES);
 	if (!ca->cpuusage)
 		goto out_free_ca;
 
@@ -8708,9 +8716,10 @@ cpuacct_destroy(struct cgroup_subsys *ss, struct cgroup *cgrp)
 	kfree(ca);
 }
 
-static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu)
+static u64 cpuacct_cpuusage_read(struct cpuacct *ca,
+		enum cpuacct_charge_index idx, int cpu)
 {
-	u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
+	u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu) + idx;
 	u64 data;
 
 #ifndef CONFIG_64BIT
@@ -8727,9 +8736,10 @@ static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu)
 	return data;
 }
 
-static void cpuacct_cpuusage_write(struct cpuacct *ca, int cpu, u64 val)
+static void cpuacct_cpuusage_write(struct cpuacct *ca,
+		enum cpuacct_charge_index idx, int cpu, u64 val)
 {
-	u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
+	u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu) + idx;
 
 #ifndef CONFIG_64BIT
 	/*
@@ -8751,7 +8761,7 @@ static u64 cpuusage_read(struct cgroup *cgrp, struct cftype *cft)
 	int i;
 
 	for_each_present_cpu(i)
-		totalcpuusage += cpuacct_cpuusage_read(ca, i);
+		totalcpuusage += cpuacct_cpuusage_read(ca, cft->private, i);
 
 	return totalcpuusage;
 }
@@ -8769,7 +8779,7 @@ static int cpuusage_write(struct cgroup *cgrp, struct cftype *cftype,
 	}
 
 	for_each_present_cpu(i)
-		cpuacct_cpuusage_write(ca, i, 0);
+		cpuacct_cpuusage_write(ca, cftype->private, i, 0);
 
 out:
 	return err;
@@ -8783,7 +8793,7 @@ static int cpuacct_percpu_seq_read(struct cgroup *cgroup, struct cftype *cft,
 	int i;
 
 	for_each_present_cpu(i) {
-		percpu = cpuacct_cpuusage_read(ca, i);
+		percpu = cpuacct_cpuusage_read(ca, cft->private, i);
 		seq_printf(m, "%llu ", (unsigned long long) percpu);
 	}
 	seq_printf(m, "\n");
@@ -8814,10 +8824,12 @@ static struct cftype files[] = {
 		.name = "usage",
 		.read_u64 = cpuusage_read,
 		.write_u64 = cpuusage_write,
+		.private = CPUACCT_CHARGE_USAGE,
 	},
 	{
 		.name = "usage_percpu",
 		.read_seq_string = cpuacct_percpu_seq_read,
+		.private = CPUACCT_CHARGE_USAGE,
 	},
 	{
 		.name = "stat",
@@ -8835,7 +8847,8 @@ static int cpuacct_populate(struct cgroup_subsys *ss, struct cgroup *cgrp)
  *
  * called with rq->lock held.
  */
-static void cpuacct_charge(struct task_struct *tsk, u64 cputime)
+static void cpuacct_charge(struct task_struct *tsk,
+		enum cpuacct_charge_index idx, u64 cputime)
 {
 	struct cpuacct *ca;
 	int cpu;
@@ -8850,7 +8863,7 @@ static void cpuacct_charge(struct task_struct *tsk, u64 cputime)
 	ca = task_ca(tsk);
 
 	for (; ca; ca = ca->parent) {
-		u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
+		u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu) + idx;
 		*cpuusage += cputime;
 	}
 
diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 217e4a9..09e8dd1 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -541,7 +541,7 @@ static void update_curr(struct cfs_rq *cfs_rq)
 		struct task_struct *curtask = task_of(curr);
 
 		trace_sched_stat_runtime(curtask, delta_exec, curr->vruntime);
-		cpuacct_charge(curtask, delta_exec);
+		cpuacct_charge(curtask, CPUACCT_CHARGE_USAGE, delta_exec);
 		account_group_exec_runtime(curtask, delta_exec);
 	}
 }
diff --git a/kernel/sched_rt.c b/kernel/sched_rt.c
index 8afb953..12adcfe 100644
--- a/kernel/sched_rt.c
+++ b/kernel/sched_rt.c
@@ -619,7 +619,7 @@ static void update_curr_rt(struct rq *rq)
 	account_group_exec_runtime(curr, delta_exec);
 
 	curr->se.exec_start = rq->clock;
-	cpuacct_charge(curr, delta_exec);
+	cpuacct_charge(curr, CPUACCT_CHARGE_USAGE, delta_exec);
 
 	sched_rt_avg_update(rq, delta_exec);
 
-- 
1.7.0.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 4/4] sched: Export irq times through cpuacct cgroup
  2010-05-25  0:11     ` [RFC PATCH 3/4] sched: Generalize cpuacct usage tracking making it simpler to add new stats Venkatesh Pallipadi
@ 2010-05-25  0:11       ` Venkatesh Pallipadi
  0 siblings, 0 replies; 13+ messages in thread
From: Venkatesh Pallipadi @ 2010-05-25  0:11 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	Balbir Singh, Paul Menage
  Cc: linux-kernel, Paul Turner, Venkatesh Pallipadi

Adds hi_time, si_time, hi_time_percpu and si_time_percpu info in cpuacct
cgroup.

The info will be fine granularity timings when either
CONFIG_IRQ_TIME_ACCOUNTING or CONFIG_VIRT_CPU_ACCOUNTING is enabled.
Otherwise the info will be based on tick samples.

Looked at adding this under cpuacct.stat. But, this information is useful
to the administrator in percpu format, so that any hi or si activity
on a particular CPU can be noted and some config change
(move the irq away, assign a different CPU to this cgroup, etc)
may be done based on that info.

Signed-off-by: Venkatesh Pallipadi <venki@google.com>
---
 kernel/sched.c |   55 +++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index d7d7efe..19e4d5d 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -1404,6 +1404,8 @@ enum cpuacct_stat_index {
 
 enum cpuacct_charge_index {
 	CPUACCT_CHARGE_USAGE,	/* ... execution time */
+	CPUACCT_CHARGE_SI_TIME,	/* ... softirq time */
+	CPUACCT_CHARGE_HI_TIME,	/* ... hardirq time */
 
 	CPUACCT_CHARGE_NCHARGES,
 };
@@ -3238,9 +3240,15 @@ static void account_guest_time(struct task_struct *p, cputime_t cputime,
 
 #ifdef CONFIG_IRQ_TIME_ACCOUNTING
 /* In this case, we charge the task at irq time for finer accounting */
-#define TASK_IRQ_CHARGE_AT_TICK(x, y)	(x)
+static inline void task_cpuacct_irq_charge(struct task_struct *p,
+			cputime64_t *ptime, int idx, cputime64_t delta) { }
 #else
-#define TASK_IRQ_CHARGE_AT_TICK(x, y)	cputime64_add(x, y)
+static inline void task_cpuacct_irq_charge(struct task_struct *p,
+			cputime64_t *ptime, int idx, cputime64_t delta)
+{
+	*ptime = cputime64_add(*ptime, delta);
+	cpuacct_charge(p, idx, delta);
+}
 #endif
 
 /*
@@ -3270,10 +3278,12 @@ void account_system_time(struct task_struct *p, int hardirq_offset,
 	tmp = cputime_to_cputime64(cputime);
 	if (hardirq_count() - hardirq_offset) {
 		cpustat->irq = cputime64_add(cpustat->irq, tmp);
-		p->hi_time = TASK_IRQ_CHARGE_AT_TICK(p->hi_time, tmp);
+		task_cpuacct_irq_charge(p, &p->hi_time,
+					CPUACCT_CHARGE_HI_TIME, tmp);
 	} else if (softirq_count()) {
 		cpustat->softirq = cputime64_add(cpustat->softirq, tmp);
-		p->si_time = TASK_IRQ_CHARGE_AT_TICK(p->si_time, tmp);
+		task_cpuacct_irq_charge(p, &p->si_time,
+					CPUACCT_CHARGE_SI_TIME, tmp);
 	} else {
 		cpustat->system = cputime64_add(cpustat->system, tmp);
 	}
@@ -8832,6 +8842,28 @@ static struct cftype files[] = {
 		.private = CPUACCT_CHARGE_USAGE,
 	},
 	{
+		.name = "si_time",
+		.read_u64 = cpuusage_read,
+		.write_u64 = cpuusage_write,
+		.private = CPUACCT_CHARGE_SI_TIME,
+	},
+	{
+		.name = "si_time_percpu",
+		.read_seq_string = cpuacct_percpu_seq_read,
+		.private = CPUACCT_CHARGE_SI_TIME,
+	},
+	{
+		.name = "hi_time",
+		.read_u64 = cpuusage_read,
+		.write_u64 = cpuusage_write,
+		.private = CPUACCT_CHARGE_HI_TIME,
+	},
+	{
+		.name = "hi_time_percpu",
+		.read_seq_string = cpuacct_percpu_seq_read,
+		.private = CPUACCT_CHARGE_HI_TIME,
+	},
+	{
 		.name = "stat",
 		.read_map = cpuacct_stats_show,
 	},
@@ -8996,17 +9028,20 @@ void account_system_vtime(struct task_struct *tsk)
 {
 	unsigned long flags;
 	int cpu;
-	u64 now;
+	u64 now, delta;
 
 	local_irq_save(flags);
 	cpu = task_cpu(tsk);
 	now = sched_clock_cpu(cpu);
-	if (hardirq_count())
-		tsk->hi_time += now - per_cpu(irq_start_time, cpu);
-	else if (softirq_count())
-		tsk->si_time += now - per_cpu(irq_start_time, cpu);
-
+	delta = now - per_cpu(irq_start_time, cpu);
 	per_cpu(irq_start_time, cpu) = now;
+	if (hardirq_count()) {
+		tsk->hi_time += delta;
+		cpuacct_charge(tsk, CPUACCT_CHARGE_HI_TIME, delta);
+	} else if (softirq_count()) {
+		tsk->si_time += delta;
+		cpuacct_charge(tsk, CPUACCT_CHARGE_SI_TIME, delta);
+	}
 	local_irq_restore(flags);
 }
 
-- 
1.7.0.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task
  2010-05-25  0:11   ` [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task Venkatesh Pallipadi
  2010-05-25  0:11     ` [RFC PATCH 3/4] sched: Generalize cpuacct usage tracking making it simpler to add new stats Venkatesh Pallipadi
@ 2010-05-25  6:35     ` Peter Zijlstra
  2010-05-25 21:40       ` Venkatesh Pallipadi
  1 sibling, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2010-05-25  6:35 UTC (permalink / raw)
  To: Venkatesh Pallipadi
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Balbir Singh,
	Paul Menage, linux-kernel, Paul Turner, Martin Schwidefsky,
	Heiko Carstens, Paul Mackerras

On Mon, 2010-05-24 at 17:11 -0700, Venkatesh Pallipadi wrote:
> +void account_system_vtime(struct task_struct *tsk)
> +{
> +       unsigned long flags;
> +       int cpu;
> +       u64 now;
> +
> +       local_irq_save(flags);
> +       cpu = task_cpu(tsk);
> +       now = sched_clock_cpu(cpu);
> +       if (hardirq_count())
> +               tsk->hi_time += now - per_cpu(irq_start_time, cpu);
> +       else if (softirq_count())
> +               tsk->si_time += now - per_cpu(irq_start_time, cpu);
> +
> +       per_cpu(irq_start_time, cpu) = now;
> +       local_irq_restore(flags);
> +} 

Right, so this gets called from irq_enter/exit() and __do_softirq().

The reason I never pressed onwards with this (I had patches to add IRQ
time accounting) is that it sucks terribly for anything falling back to
jiffies -- maybe find some smart way to disable the whole call when
there's no TSC available, preferably without adding conditionals, using
alternatives maybe?

I guess you mostly side-stepped that by adding a IRQ_TIME_ACCOUNTING
config (but forgot to make it depend on X86_TSC).

Another thing I dislike about this account_system_vtime() is that its
the same call for both IRQ and SoftIRQ, leaving us to add conditionals
inside the call to figure out what context we got called from.

[ Adedd the s390 and ppc guys who already use this stuff ]

Anyway, once we have this, please also add it to sched_rt_avg_update()
(which should really be called sched_!fair_avg_update()).

Also, did you measure the overhead of doing this? sched_clock_cpu() adds
a cmpxchg64 on all systems that don't have a rock solid TSC (ie. most of
todays machines).

Another thing that would be real nice is if you could find a way to not
make all of this x86 specific.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 0/4] Finer granularity and task/cgroup irq time accounting
  2010-05-25  0:11 [RFC PATCH 0/4] Finer granularity and task/cgroup irq time accounting Venkatesh Pallipadi
  2010-05-25  0:11 ` [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time Venkatesh Pallipadi
@ 2010-05-25  7:00 ` Balbir Singh
  2010-05-25  8:14 ` Peter Zijlstra
  2 siblings, 0 replies; 13+ messages in thread
From: Balbir Singh @ 2010-05-25  7:00 UTC (permalink / raw)
  To: Venkatesh Pallipadi
  Cc: Peter Zijlstra, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	Paul Menage, linux-kernel, Paul Turner

* Venkatesh Pallipadi <venki@google.com> [2010-05-24 17:11:18]:

> Currently, the softirq and hardirq time reporting is only done at the
> CPU level. There are usecases where reporting this time against task
> or task groups or cgroups will be useful for user/administrator
> in terms of resource planning and utilization charging. Also, as the
> accoounting is already done at the CPU level, reporting the same at
> the task level does not add any significant computational overhead
> other than task level storage (patch 1).
> 
> The softirq/hardirq statistics commonly done based on tick based sampling.
> Though some archs have CONFIG_VIRT_CPU_ACCOUNTING based fine granularity
> accounting. Having similar mechanism to get fine granularity accounting
> on x86 will be a major challenge, given the state of TSC reliability
> on various platforms and also the overhead it may add in common paths
> like syscall entry exit.
> 
> An alternative is to have a generic (sched_clock based) and configurable
> fine-granularity accounting of si and hi time which can be reported
> over the /proc/<pid>/stat API (patch 2).
> 
> Patch 3 and 4 are exporting this info at the cgroup level.
> 
> Does exposing this additional info to user makes sense? Any feedback on
> the way it is done in this patchset?
> 
> This precise irq time based on sched_clock() provides some potential
> opportunities to handle the softirq time charging in a more fair way.
> Specifically cases where an unrelated task is being penalized for
> irq load on that CPU.
> * With network Receive Flow Steering, for example; We can potentially
> do things like not charge receive softirq time to the process that is
> currently running and charge it instead to the actual consumer of
> the receive (in recvmsg, for example).
> * We can reduce the power of the CPU to account for softirq/hardirq
> load, in order to increase the scheduler fairness for tasks running on
> that CPU.

Could you also add to this section or the documentation, what
interfaces are exported or impacted.

> 
> Comments?
> 
> Thanks,
> Venki
> 

-- 
	Three Cheers,
	Balbir

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 0/4] Finer granularity and task/cgroup irq time accounting
  2010-05-25  0:11 [RFC PATCH 0/4] Finer granularity and task/cgroup irq time accounting Venkatesh Pallipadi
  2010-05-25  0:11 ` [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time Venkatesh Pallipadi
  2010-05-25  7:00 ` [RFC PATCH 0/4] Finer granularity and task/cgroup irq time accounting Balbir Singh
@ 2010-05-25  8:14 ` Peter Zijlstra
  2010-05-25 17:05   ` Venkatesh Pallipadi
  2 siblings, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2010-05-25  8:14 UTC (permalink / raw)
  To: Venkatesh Pallipadi
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Balbir Singh,
	Paul Menage, linux-kernel, Paul Turner


Argh - you managed to use chain-reply with git-1.7? 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time
  2010-05-25  0:11 ` [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time Venkatesh Pallipadi
  2010-05-25  0:11   ` [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task Venkatesh Pallipadi
@ 2010-05-25  9:13   ` Balbir Singh
  2010-05-25 21:45     ` Venkatesh Pallipadi
  1 sibling, 1 reply; 13+ messages in thread
From: Balbir Singh @ 2010-05-25  9:13 UTC (permalink / raw)
  To: Venkatesh Pallipadi
  Cc: Peter Zijlstra, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	Paul Menage, linux-kernel, Paul Turner

* Venkatesh Pallipadi <venki@google.com> [2010-05-24 17:11:19]:

> Currently, kernel does not have accounting mechanism for softirq and hardirq
> times at the task level. There is irq time info in kstat_cpu which is
> accumulated at the cpu level.
> 
> Without the task level information, the non irq run time of task(s) would
> have to be guessed based on their exec time and CPU on which they were
> running recently and assuming that the CPU irq time reported are spread
> across all the tasks running there. And this guess can be widely off the mark.
> 
> Sample case, considering just the softirq:
> 
> If there are varied workloads running on a CPU, say a CPU bound task (loop)
> and a network IO bound task (nc) along with the network softirq load,
> there is no way for the administrator/user to know the non-irq runtime of each
> of these tasks. Only information available is the total runtime for each of the
> tasks and kstat_cpu softirq time for the CPU.
> 
> In this example, considering a 10 second sample, both loop and nc would have
> total run time of ~5s. And kstat_cpu softirq on this cpu increase was
> 355 (~3.5s).
> 
> So, all the information the user gets is that both the tasks are running for
> roughly the same amount of time and softirq is around 35%. As a result user
> may conclude that irq overhead for both tasks are equal (1.75s) and the
> non-irq runtime of both the tasks are around ~3.25s. Yes. There is another
> factor of system and user time reported for these tasks that I am ignoring
> as that is tough to correlate with irq time, in cases where the tasks have
> significant non-irq system time.
> 
> This change adds tracking of softirq time on each task and task group.
> This information is exported in /proc/<pid>/stat.
> 
> So, the user can get info like below, looking at exec_time and si_time in
> appropriate /proc/<pid>/stat.
> (Taken for a 10s interval)
> task exec_time softirqtime (in USER_HZ)
> (loop)  (nc)
> 505 0   500 359
> 502 1   501 363
> 503 0   502 354
> 504 0   499 359
> 503 3   500 360
> 
> with this, user can get the non-irq run time as 5s and ~1.45s for
> loop and nc, respectively.

Have you noticed any overheads after these changes? Otherwise, the
changes look correct to me.


-- 
	Three Cheers,
	Balbir

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 0/4] Finer granularity and task/cgroup irq time  accounting
  2010-05-25  8:14 ` Peter Zijlstra
@ 2010-05-25 17:05   ` Venkatesh Pallipadi
  0 siblings, 0 replies; 13+ messages in thread
From: Venkatesh Pallipadi @ 2010-05-25 17:05 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Balbir Singh,
	Paul Menage, linux-kernel, Paul Turner

On Tue, May 25, 2010 at 1:14 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> Argh - you managed to use chain-reply with git-1.7?
>

Sorry. My fault.
Had chainreplyto turned on in my gitconfig. :-(. Fixing it now...

Thanks,
Venki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of  irq time to task
  2010-05-25  6:35     ` [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task Peter Zijlstra
@ 2010-05-25 21:40       ` Venkatesh Pallipadi
  2010-05-26  6:54         ` Peter Zijlstra
  0 siblings, 1 reply; 13+ messages in thread
From: Venkatesh Pallipadi @ 2010-05-25 21:40 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Balbir Singh,
	Paul Menage, linux-kernel, Paul Turner, Martin Schwidefsky,
	Heiko Carstens, Paul Mackerras

On Mon, May 24, 2010 at 11:35 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, 2010-05-24 at 17:11 -0700, Venkatesh Pallipadi wrote:
>> +void account_system_vtime(struct task_struct *tsk)
>> +{
>> +       unsigned long flags;
>> +       int cpu;
>> +       u64 now;
>> +
>> +       local_irq_save(flags);
>> +       cpu = task_cpu(tsk);
>> +       now = sched_clock_cpu(cpu);
>> +       if (hardirq_count())
>> +               tsk->hi_time += now - per_cpu(irq_start_time, cpu);
>> +       else if (softirq_count())
>> +               tsk->si_time += now - per_cpu(irq_start_time, cpu);
>> +
>> +       per_cpu(irq_start_time, cpu) = now;
>> +       local_irq_restore(flags);
>> +}
>
> Right, so this gets called from irq_enter/exit() and __do_softirq().
>
> The reason I never pressed onwards with this (I had patches to add IRQ
> time accounting) is that it sucks terribly for anything falling back to
> jiffies -- maybe find some smart way to disable the whole call when
> there's no TSC available, preferably without adding conditionals, using
> alternatives maybe?
>
> I guess you mostly side-stepped that by adding a IRQ_TIME_ACCOUNTING
> config (but forgot to make it depend on X86_TSC).

Yes. The TSC dependency is unfortunately not CONFIG time. Even with X86_TSC,
there is support for tsc_disabled. So, this should be a run time
conditional or an alternative.

> Another thing I dislike about this account_system_vtime() is that its
> the same call for both IRQ and SoftIRQ, leaving us to add conditionals
> inside the call to figure out what context we got called from.
>
> [ Adedd the s390 and ppc guys who already use this stuff ]

We can probably add a parameter on from where it is being called. Even
with that we still need some conditionals to handle hardirq overlapping a
softirq. That part kind of happens naturally with the current code.

> Anyway, once we have this, please also add it to sched_rt_avg_update()
> (which should really be called sched_!fair_avg_update()).

You mean take out the hi and si time from rt_delta?

> Also, did you measure the overhead of doing this? sched_clock_cpu() adds
> a cmpxchg64 on all systems that don't have a rock solid TSC (ie. most of
> todays machines).
>
> Another thing that would be real nice is if you could find a way to not
> make all of this x86 specific.

Yes. Thats one of the reason I made this based of sched_clock. May be
we can have
a has_fast_sched_clock feature that archs and opt in to that enables
this with an
alternative?

overhead: My initial testing was on reliable TSC system, where sched_clock_cpu()
takes <50 cycles. So, no noticable overhead there. I still have to run
this on other
platforms, will post the data once I do that.

Thanks,
Venki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq  time
  2010-05-25  9:13   ` [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time Balbir Singh
@ 2010-05-25 21:45     ` Venkatesh Pallipadi
  0 siblings, 0 replies; 13+ messages in thread
From: Venkatesh Pallipadi @ 2010-05-25 21:45 UTC (permalink / raw)
  To: balbir
  Cc: Peter Zijlstra, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
	Paul Menage, linux-kernel, Paul Turner

On Tue, May 25, 2010 at 2:13 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> * Venkatesh Pallipadi <venki@google.com> [2010-05-24 17:11:19]:
>
>> Currently, kernel does not have accounting mechanism for softirq and hardirq
>> times at the task level. There is irq time info in kstat_cpu which is
>> accumulated at the cpu level.
>>
>> Without the task level information, the non irq run time of task(s) would
>> have to be guessed based on their exec time and CPU on which they were
>> running recently and assuming that the CPU irq time reported are spread
>> across all the tasks running there. And this guess can be widely off the mark.
>>
>> Sample case, considering just the softirq:
>>
>> If there are varied workloads running on a CPU, say a CPU bound task (loop)
>> and a network IO bound task (nc) along with the network softirq load,
>> there is no way for the administrator/user to know the non-irq runtime of each
>> of these tasks. Only information available is the total runtime for each of the
>> tasks and kstat_cpu softirq time for the CPU.
>>
>> In this example, considering a 10 second sample, both loop and nc would have
>> total run time of ~5s. And kstat_cpu softirq on this cpu increase was
>> 355 (~3.5s).
>>
>> So, all the information the user gets is that both the tasks are running for
>> roughly the same amount of time and softirq is around 35%. As a result user
>> may conclude that irq overhead for both tasks are equal (1.75s) and the
>> non-irq runtime of both the tasks are around ~3.25s. Yes. There is another
>> factor of system and user time reported for these tasks that I am ignoring
>> as that is tough to correlate with irq time, in cases where the tasks have
>> significant non-irq system time.
>>
>> This change adds tracking of softirq time on each task and task group.
>> This information is exported in /proc/<pid>/stat.
>>
>> So, the user can get info like below, looking at exec_time and si_time in
>> appropriate /proc/<pid>/stat.
>> (Taken for a 10s interval)
>> task exec_time softirqtime (in USER_HZ)
>> (loop)  (nc)
>> 505 0   500 359
>> 502 1   501 363
>> 503 0   502 354
>> 504 0   499 359
>> 503 3   500 360
>>
>> with this, user can get the non-irq run time as 5s and ~1.45s for
>> loop and nc, respectively.
>
> Have you noticed any overheads after these changes? Otherwise, the
> changes look correct to me.
>

Haven't noticed any significant overhead yet. But, I am yet to run this on wide
array of systems. Will post the data once I have them.

Thanks,
Venki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of  irq time to task
  2010-05-25 21:40       ` Venkatesh Pallipadi
@ 2010-05-26  6:54         ` Peter Zijlstra
  0 siblings, 0 replies; 13+ messages in thread
From: Peter Zijlstra @ 2010-05-26  6:54 UTC (permalink / raw)
  To: Venkatesh Pallipadi
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Balbir Singh,
	Paul Menage, linux-kernel, Paul Turner, Martin Schwidefsky,
	Heiko Carstens, Paul Mackerras

On Tue, 2010-05-25 at 14:40 -0700, Venkatesh Pallipadi wrote:
> On Mon, May 24, 2010 at 11:35 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Mon, 2010-05-24 at 17:11 -0700, Venkatesh Pallipadi wrote:
> >> +void account_system_vtime(struct task_struct *tsk)
> >> +{
> >> +       unsigned long flags;
> >> +       int cpu;
> >> +       u64 now;
> >> +
> >> +       local_irq_save(flags);
> >> +       cpu = task_cpu(tsk);
> >> +       now = sched_clock_cpu(cpu);
> >> +       if (hardirq_count())
> >> +               tsk->hi_time += now - per_cpu(irq_start_time, cpu);
> >> +       else if (softirq_count())
> >> +               tsk->si_time += now - per_cpu(irq_start_time, cpu);
> >> +
> >> +       per_cpu(irq_start_time, cpu) = now;
> >> +       local_irq_restore(flags);
> >> +}
> >
> > Right, so this gets called from irq_enter/exit() and __do_softirq().
> >
> > The reason I never pressed onwards with this (I had patches to add IRQ
> > time accounting) is that it sucks terribly for anything falling back to
> > jiffies -- maybe find some smart way to disable the whole call when
> > there's no TSC available, preferably without adding conditionals, using
> > alternatives maybe?
> >
> > I guess you mostly side-stepped that by adding a IRQ_TIME_ACCOUNTING
> > config (but forgot to make it depend on X86_TSC).
> 
> Yes. The TSC dependency is unfortunately not CONFIG time. Even with X86_TSC,
> there is support for tsc_disabled. So, this should be a run time
> conditional or an alternative.

Right.

> > Another thing I dislike about this account_system_vtime() is that its
> > the same call for both IRQ and SoftIRQ, leaving us to add conditionals
> > inside the call to figure out what context we got called from.
> >
> > [ Adedd the s390 and ppc guys who already use this stuff ]
> 
> We can probably add a parameter on from where it is being called. Even
> with that we still need some conditionals to handle hardirq overlapping a
> softirq. That part kind of happens naturally with the current code.

Ah, indeed. Yes that nesting stuff will give some conditionals.

> > Anyway, once we have this, please also add it to sched_rt_avg_update()
> > (which should really be called sched_!fair_avg_update()).
> 
> You mean take out the hi and si time from rt_delta?

No add it in. The thing we do with rt_delta is compute how much time is
not available to sched_fair, so that we can normalize the runqueue
weights across cpus to ensure tasks still get an equal amount of
service.

Currently it only accounts for the time spend running RT tasks, but
ideally it should be everything not running sched_fair which of course
includes all the IRQ overhead.

> > Also, did you measure the overhead of doing this? sched_clock_cpu() adds
> > a cmpxchg64 on all systems that don't have a rock solid TSC (ie. most of
> > todays machines).
> >
> > Another thing that would be real nice is if you could find a way to not
> > make all of this x86 specific.
> 
> Yes. Thats one of the reason I made this based of sched_clock. May be
> we can have a has_fast_sched_clock feature that archs and opt in to that
> enables this with an alternative?

Right, we can already use CONFIG_HACE_UNSTABLE_SCHED_CLOCK and
sched_clock_stable, but we need more.

Also, what will the stats show when we do fallback to jiffies? Simply
not account them and show 0?

> overhead: My initial testing was on reliable TSC system, where sched_clock_cpu()
> takes <50 cycles. So, no noticable overhead there. I still have to run
> this on other platforms, will post the data once I do that.

Right, I guess the Nehalem class desktops with single chips, hopefully
even the laptop chips will have usable TSC, but pretty much all the
Core2 chips out there are iffy.



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2010-05-26  6:54 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-25  0:11 [RFC PATCH 0/4] Finer granularity and task/cgroup irq time accounting Venkatesh Pallipadi
2010-05-25  0:11 ` [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time Venkatesh Pallipadi
2010-05-25  0:11   ` [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task Venkatesh Pallipadi
2010-05-25  0:11     ` [RFC PATCH 3/4] sched: Generalize cpuacct usage tracking making it simpler to add new stats Venkatesh Pallipadi
2010-05-25  0:11       ` [RFC PATCH 4/4] sched: Export irq times through cpuacct cgroup Venkatesh Pallipadi
2010-05-25  6:35     ` [RFC PATCH 2/4] x86: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time to task Peter Zijlstra
2010-05-25 21:40       ` Venkatesh Pallipadi
2010-05-26  6:54         ` Peter Zijlstra
2010-05-25  9:13   ` [RFC PATCH 1/4] sched: Track and export per task [hard|soft]irq time Balbir Singh
2010-05-25 21:45     ` Venkatesh Pallipadi
2010-05-25  7:00 ` [RFC PATCH 0/4] Finer granularity and task/cgroup irq time accounting Balbir Singh
2010-05-25  8:14 ` Peter Zijlstra
2010-05-25 17:05   ` Venkatesh Pallipadi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®