| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4961AFE3.3080102@jp.fujitsu.com>
Date: Mon, 05 Jan 2009 15:59:47 +0900
From: Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
To: Ingo Molnar <mingo@...e.hu>
CC: Peter Zijlstra <a.p.zijlstra@...llo.nl>,
linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] posix-cpu-timers: clock_gettime(CLOCK_*_CPUTIME_ID) goes
backward
OK, this patch will follow to Peter's "remove the per-cpu-ish-ness"
patch. How about this?
Thanks,
H.Seto
posix-cpu-timers, clock_gettime(CLOCK_THREAD_CPUTIME_ID) and
clock_gettime(CLOCK_PROCESS_CPUTIME_ID) occasionally go backward.
>From user land (@HZ=250), it looks like:
prev call current call diff
(0.191195713) > (0.191200456) : 4743
(0.191200456) > (0.191205198) : 4742
(0.191205198) > (0.187213693) : -3991505
(0.187213693) > (0.191218688) : 4004995
(0.191218688) > (0.191223436) : 4748
The reasons are:
- Timer tick (and task_tick) can interrups between getting
sum_exec_runtime and task_delta_exec(p). Since task's runtime
is updated in the tick, it makes the value of cpu->sched about
a tick less than what it should be.
So, interrupts should be disabled while sampling runtime and
delta.
- Queueing task also updates runtime of task running on the rq
which a task is going to be enqueued/dequeued.
So, runqueue should be locked while sampling runtime and delta.
For thread timer (CLOCK_THREAD_CPUTIME_ID), it is easy to solve
by making a function do all in a block locked by task_rq_lock().
# clock_gettime(CLOCK_THREAD_CPUTIME_ID)
before:
cpu->sched = p->se.sum_exec_runtime + task_delta_exec(p);
after:
cpu->sched = task_total_exec(p);
For process timer (CLOCK_PROCESS_CPUTIME_ID), proposed fix here
is moving thread_group_cputime() from include/linux/sched.h to
kernel/sched.c, and let it use rq-related operations.
# clock_gettime(CLOCK_PROCESS_CPUTIME_ID)
before:
thread_group_cputime(p,&cputime); /* returns total */
cpu->sched = cputime.sum_exec_runtime + task_delta_exec(p);
after:
thread_group_cputime(p,&cputime); /* total + local delta */
cpu->sched = cputime.sum_exec_runtime;
[Note]: According to the current code, process timer returns
"((total runtime of thread group) + (delta of current thread))."
The former total is total of "banked runtime" in signal structure,
therefore technically speaking the latter delta should be sum of
delta of all running thread in the group. However requesting delta
for all cpu is quite heavy and nonsense, so I took a realistic
approach - keep it as is, as return banked total plus local delta.
In othe words, be careful that accuracy of process timer is not
so good (while clock_getres() returns a resolution of 1ns), it
can stale about tick*(max(NR_CPUS, num_thread)-1).
Signed-off-by: Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
---
include/linux/kernel_stat.h | 2 +-
include/linux/sched.h | 11 +----------
kernel/posix-cpu-timers.c | 4 ++--
kernel/sched.c | 29 +++++++++++++++++++++++++++--
4 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/include/linux/kernel_stat.h b/include/linux/kernel_stat.h
index 570d204..50fabb3 100644
--- a/include/linux/kernel_stat.h
+++ b/include/linux/kernel_stat.h
@@ -78,7 +78,7 @@ static inline unsigned int kstat_irqs(unsigned int irq)
return sum;
}
-extern unsigned long long task_delta_exec(struct task_struct *);
+extern unsigned long long task_total_exec(struct task_struct *);
extern void account_user_time(struct task_struct *, cputime_t, cputime_t);
extern void account_system_time(struct task_struct *, int, cputime_t, cputime_t);
extern void account_steal_time(cputime_t);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 29c4b8d..c6a07bf 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2179,16 +2179,7 @@ static inline int spin_needbreak(spinlock_t *lock)
* Thread group CPU time accounting.
*/
-static inline
-void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times)
-{
- struct task_cputime *totals = &tsk->signal->cputime.totals;
- unsigned long flags;
-
- spin_lock_irqsave(&totals->lock, flags);
- *times = *totals;
- spin_unlock_irqrestore(&totals->lock, flags);
-}
+extern void thread_group_cputime(struct task_struct *, struct task_cputime *);
static inline void thread_group_cputime_init(struct signal_struct *sig)
{
diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index fa07da9..56c91b9 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/posix-cpu-timers.c
@@ -224,7 +224,7 @@ static int cpu_clock_sample(const clockid_t which_clock, struct task_struct *p,
cpu->cpu = virt_ticks(p);
break;
case CPUCLOCK_SCHED:
- cpu->sched = p->se.sum_exec_runtime + task_delta_exec(p);
+ cpu->sched = task_total_exec(p);
break;
}
return 0;
@@ -251,7 +251,7 @@ static int cpu_clock_sample_group(const clockid_t which_clock,
cpu->cpu = cputime.utime;
break;
case CPUCLOCK_SCHED:
- cpu->sched = cputime.sum_exec_runtime + task_delta_exec(p);
+ cpu->sched = cputime.sum_exec_runtime;
break;
}
return 0;
diff --git a/kernel/sched.c b/kernel/sched.c
index 545c6fc..8315cd5 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -4121,10 +4121,10 @@ DEFINE_PER_CPU(struct kernel_stat, kstat);
EXPORT_PER_CPU_SYMBOL(kstat);
/*
- * Return any ns on the sched_clock that have not yet been banked in
+ * Return total of runtime which already banked and which not yet
* @p in case that task is currently running.
*/
-unsigned long long task_delta_exec(struct task_struct *p)
+unsigned long long task_total_exec(struct task_struct *p)
{
unsigned long flags;
struct rq *rq;
@@ -4140,12 +4140,37 @@ unsigned long long task_delta_exec(struct task_struct *p)
if ((s64)delta_exec > 0)
ns = delta_exec;
}
+ ns += p->se.sum_exec_runtime;
task_rq_unlock(rq, &flags);
return ns;
}
+void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times)
+{
+ struct task_cputime *totals = &tsk->signal->cputime.totals;
+ unsigned long flags;
+ struct rq *rq;
+ u64 delta_exec = 0;
+
+ rq = task_rq_lock(tsk, &flags);
+
+ spin_lock(&totals->lock);
+ *times = *totals;
+ spin_unlock(&totals->lock);
+
+ if (task_current(rq, tsk)) {
+ update_rq_clock(rq);
+ delta_exec = rq->clock - tsk->se.exec_start;
+ if ((s64)delta_exec < 0)
+ delta_exec = 0;
+ }
+ times->sum_exec_runtime += delta_exec;
+
+ task_rq_unlock(rq, &flags);
+}
+
/*
* Account user cpu time to a process.
* @p: the process that the cpu time gets accounted to
--
1.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists