* [PATCH] Fix delay accounting regression
@ 2007-11-02 14:22 Balbir Singh
2007-11-02 15:53 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: Balbir Singh @ 2007-11-02 14:22 UTC (permalink / raw)
To: Ingo Molnar
Cc: Balbir Singh, Peter Zijlstra, Andrew Morton,
Linux Kernel Mailing List
Fix the delay accounting regression introduced by commit
75d4ef16a6aa84f708188bada182315f80aab6fa. rq no longer has sched_info
data associated with it. task_struct sched_info structure is used by delay
accounting to provide back statistics to user space.
Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
---
kernel/sched_stats.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -puN kernel/sched_stats.h~delayacct-define-bug kernel/sched_stats.h
--- linux-2.6-latest/kernel/sched_stats.h~delayacct-define-bug 2007-11-02 19:02:07.000000000 +0530
+++ linux-2.6-latest-balbir/kernel/sched_stats.h 2007-11-02 19:51:30.000000000 +0530
@@ -127,7 +127,7 @@ rq_sched_info_depart(struct rq *rq, unsi
# define schedstat_set(var, val) do { } while (0)
#endif
-#ifdef CONFIG_SCHEDSTATS
+#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
/*
* Called when a process is dequeued from the active array and given
* the cpu. We should note that with the exception of interactive
@@ -231,5 +231,5 @@ sched_info_switch(struct task_struct *pr
#else
#define sched_info_queued(t) do { } while (0)
#define sched_info_switch(t, next) do { } while (0)
-#endif /* CONFIG_SCHEDSTATS */
+#endif /* CONFIG_SCHEDSTATS || CONFIG_TASK_DELAY_ACCT */
_
--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix delay accounting regression 2007-11-02 14:22 [PATCH] Fix delay accounting regression Balbir Singh @ 2007-11-02 15:53 ` Ingo Molnar 2007-11-02 16:05 ` Guillaume Chazarain ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Ingo Molnar @ 2007-11-02 15:53 UTC (permalink / raw) To: Balbir Singh; +Cc: Peter Zijlstra, Andrew Morton, Linux Kernel Mailing List * Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > Fix the delay accounting regression introduced by commit > 75d4ef16a6aa84f708188bada182315f80aab6fa. rq no longer has sched_info > data associated with it. task_struct sched_info structure is used by > delay accounting to provide back statistics to user space. no, this really is broken code. sched_clock() is not a generally reliable time source so at a minimum the rq->clock fix i mentioned is needed. What user-space tools are utilizing delay-accounting by the way? We have most of this data available under SCHEDSTATS - how about consolidating CONFIG_TASK_DELAY_ACCT into CONFIG_SCHEDSTATS? Ingo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix delay accounting regression 2007-11-02 15:53 ` Ingo Molnar @ 2007-11-02 16:05 ` Guillaume Chazarain 2007-11-02 16:35 ` Balbir Singh 2007-11-02 16:55 ` Balbir Singh 2 siblings, 0 replies; 6+ messages in thread From: Guillaume Chazarain @ 2007-11-02 16:05 UTC (permalink / raw) To: Ingo Molnar Cc: Balbir Singh, Peter Zijlstra, Andrew Morton, Linux Kernel Mailing List On 11/2/07, Ingo Molnar <mingo@elte.hu> wrote: > What user-space tools are utilizing delay-accounting by the way? Thanks for the plugging opportunity ;-) http://guichaz.free.fr/misc/#iotop uses the I/O side of delay-accounting. -- Guillaume ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix delay accounting regression 2007-11-02 15:53 ` Ingo Molnar 2007-11-02 16:05 ` Guillaume Chazarain @ 2007-11-02 16:35 ` Balbir Singh 2007-11-02 16:55 ` Balbir Singh 2 siblings, 0 replies; 6+ messages in thread From: Balbir Singh @ 2007-11-02 16:35 UTC (permalink / raw) To: Ingo Molnar; +Cc: Peter Zijlstra, Andrew Morton, Linux Kernel Mailing List Ingo Molnar wrote: > * Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > >> Fix the delay accounting regression introduced by commit >> 75d4ef16a6aa84f708188bada182315f80aab6fa. rq no longer has sched_info >> data associated with it. task_struct sched_info structure is used by >> delay accounting to provide back statistics to user space. > > no, this really is broken code. sched_clock() is not a generally > reliable time source so at a minimum the rq->clock fix i mentioned is > needed. What user-space tools are utilizing delay-accounting by the way? Applications I can think of are 1. iotop 2. Some typical applications are mentioned in https://ols2006.108.redhat.com/2007/Reprints/nagar-Reprint.pdf > We have most of this data available under SCHEDSTATS - how about > consolidating CONFIG_TASK_DELAY_ACCT into CONFIG_SCHEDSTATS? > > Ingo Yes, we do use schedstats - except for two cases that I know of? 1. To accumulate information in taskstats to share with user space (we copy the data from schedstats into taskstats) 2. We accumulate the data for dead threads in a thread group -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix delay accounting regression 2007-11-02 15:53 ` Ingo Molnar 2007-11-02 16:05 ` Guillaume Chazarain 2007-11-02 16:35 ` Balbir Singh @ 2007-11-02 16:55 ` Balbir Singh 2007-11-03 8:33 ` Ingo Molnar 2 siblings, 1 reply; 6+ messages in thread From: Balbir Singh @ 2007-11-02 16:55 UTC (permalink / raw) To: Ingo Molnar; +Cc: Peter Zijlstra, Andrew Morton, Linux Kernel Mailing List On Fri, Nov 02, 2007 at 04:53:58PM +0100, Ingo Molnar wrote: > > * Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > > Fix the delay accounting regression introduced by commit > > 75d4ef16a6aa84f708188bada182315f80aab6fa. rq no longer has sched_info > > data associated with it. task_struct sched_info structure is used by > > delay accounting to provide back statistics to user space. > > no, this really is broken code. sched_clock() is not a generally > reliable time source so at a minimum the rq->clock fix i mentioned is > needed. What user-space tools are utilizing delay-accounting by the way? > We have most of this data available under SCHEDSTATS - how about > consolidating CONFIG_TASK_DELAY_ACCT into CONFIG_SCHEDSTATS? > > Ingo Hi, Ingo, How about this? Changelog 1. As per Ingo Molnar's recommendation, use rq->clock instead of sched_clock(). Fix the delay accounting regression introduced by commit 75d4ef16a6aa84f708188bada182315f80aab6fa. rq no longer has sched_info data associated with it. task_struct sched_info structure is used by delay accounting to provide back statistics to user space. Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> --- kernel/sched_stats.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff -puN kernel/sched_stats.h~delayacct-define-bug kernel/sched_stats.h --- linux-2.6-latest/kernel/sched_stats.h~delayacct-define-bug 2007-11-02 19:02:07.000000000 +0530 +++ linux-2.6-latest-balbir/kernel/sched_stats.h 2007-11-02 22:11:21.000000000 +0530 @@ -127,7 +127,7 @@ rq_sched_info_depart(struct rq *rq, unsi # define schedstat_set(var, val) do { } while (0) #endif -#ifdef CONFIG_SCHEDSTATS +#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT) /* * Called when a process is dequeued from the active array and given * the cpu. We should note that with the exception of interactive @@ -155,7 +155,7 @@ static inline void sched_info_dequeued(s */ static void sched_info_arrive(struct task_struct *t) { - unsigned long long now = sched_clock(), delta = 0; + unsigned long long now = task_rq(t)->clock, delta = 0; if (t->sched_info.last_queued) delta = now - t->sched_info.last_queued; @@ -186,7 +186,7 @@ static inline void sched_info_queued(str { if (unlikely(sched_info_on())) if (!t->sched_info.last_queued) - t->sched_info.last_queued = sched_clock(); + t->sched_info.last_queued = task_rq(t)->clock; } /* @@ -195,7 +195,8 @@ static inline void sched_info_queued(str */ static inline void sched_info_depart(struct task_struct *t) { - unsigned long long delta = sched_clock() - t->sched_info.last_arrival; + unsigned long long delta = task_rq(t)->clock - + t->sched_info.last_arrival; t->sched_info.cpu_time += delta; rq_sched_info_depart(task_rq(t), delta); @@ -231,5 +232,5 @@ sched_info_switch(struct task_struct *pr #else #define sched_info_queued(t) do { } while (0) #define sched_info_switch(t, next) do { } while (0) -#endif /* CONFIG_SCHEDSTATS */ +#endif /* CONFIG_SCHEDSTATS || CONFIG_TASK_DELAY_ACCT */ _ -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix delay accounting regression 2007-11-02 16:55 ` Balbir Singh @ 2007-11-03 8:33 ` Ingo Molnar 0 siblings, 0 replies; 6+ messages in thread From: Ingo Molnar @ 2007-11-03 8:33 UTC (permalink / raw) To: Balbir Singh; +Cc: Peter Zijlstra, Andrew Morton, Linux Kernel Mailing List * Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > On Fri, Nov 02, 2007 at 04:53:58PM +0100, Ingo Molnar wrote: > > > > * Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > > > > Fix the delay accounting regression introduced by commit > > > 75d4ef16a6aa84f708188bada182315f80aab6fa. rq no longer has sched_info > > > data associated with it. task_struct sched_info structure is used by > > > delay accounting to provide back statistics to user space. > > > > no, this really is broken code. sched_clock() is not a generally > > reliable time source so at a minimum the rq->clock fix i mentioned is > > needed. What user-space tools are utilizing delay-accounting by the way? > > We have most of this data available under SCHEDSTATS - how about > > consolidating CONFIG_TASK_DELAY_ACCT into CONFIG_SCHEDSTATS? > > > > Ingo > > Hi, Ingo, > > How about this? thanks, applied. Ingo ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-11-03 8:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-11-02 14:22 [PATCH] Fix delay accounting regression Balbir Singh 2007-11-02 15:53 ` Ingo Molnar 2007-11-02 16:05 ` Guillaume Chazarain 2007-11-02 16:35 ` Balbir Singh 2007-11-02 16:55 ` Balbir Singh 2007-11-03 8:33 ` Ingo Molnar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox