* [PATCH 2/4] tracing/sched: Fix task states in sched switch event
2010-05-13 1:21 [PATCH 0/4] [GIT PULL] tracing: updates Steven Rostedt
@ 2010-05-13 1:21 ` Steven Rostedt
2010-05-13 6:15 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2010-05-13 1:21 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker, Carsten Emde
[-- Attachment #1: 0002-tracing-sched-Fix-task-states-in-sched-switch-event.patch --]
[-- Type: text/plain, Size: 5038 bytes --]
From: Carsten Emde <C.Emde@osadl.org>
The sched_switch trace event displays erroneous character codes of task
states, after a new task state was added in the scheduler code but
omitted to add in the trace event code.
Define character codes of task states individually. In addition, define
task state descriptions needed in /proc at the same place. This will
help to keep the task state bits, characters and descriptions in sync
should they ever need to be changed again.
Signed-off-by: Carsten Emde <C.Emde@osadl.org>
LKML-Reference: <20100311122822.678489605@osadl.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
fs/proc/array.c | 25 +++++++++++++------------
include/linux/sched.h | 38 ++++++++++++++++++++++++++++++++++++--
include/trace/events/sched.h | 12 +++++++++---
3 files changed, 58 insertions(+), 17 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c
index e51f2ec..6ececdd 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -128,21 +128,22 @@ static inline void task_name(struct seq_file *m, struct task_struct *p)
/*
* The task state array is a strange "bitmap" of
- * reasons to sleep. Thus "running" is zero, and
- * you can test for combinations of others with
+ * reasons to sleep. Thus, the first element is zero,
+ * and you can test for combinations of others with
* simple bit tests.
*/
+#define TASK_STATE_X(num) TASK_STATE_##num " (" DESCR_TASK_STATE_##num ")"
static const char *task_state_array[] = {
- "R (running)", /* 0 */
- "S (sleeping)", /* 1 */
- "D (disk sleep)", /* 2 */
- "T (stopped)", /* 4 */
- "t (tracing stop)", /* 8 */
- "Z (zombie)", /* 16 */
- "X (dead)", /* 32 */
- "x (dead)", /* 64 */
- "K (wakekill)", /* 128 */
- "W (waking)", /* 256 */
+ TASK_STATE_X(0),
+ TASK_STATE_X(1),
+ TASK_STATE_X(2),
+ TASK_STATE_X(4),
+ TASK_STATE_X(8),
+ TASK_STATE_X(16),
+ TASK_STATE_X(32),
+ TASK_STATE_X(64),
+ TASK_STATE_X(128),
+ TASK_STATE_X(256)
};
static inline const char *get_task_state(struct task_struct *tsk)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index dad7f66..1d0b0ab 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -172,7 +172,9 @@ print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
/*
* Task state bitmask. NOTE! These bits are also
- * encoded in fs/proc/array.c: get_task_state().
+ * used in fs/proc/array.c: get_task_state() and
+ * in include/trace/events/sched.h in the
+ * sched_switch trace event.
*
* We have two separate sets of flags: task->state
* is about runnability, while task->exit_state are
@@ -181,20 +183,52 @@ print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
* mistake.
*/
#define TASK_RUNNING 0
+#define TASK_STATE_0 "R"
+#define DESCR_TASK_STATE_0 "running"
+
#define TASK_INTERRUPTIBLE 1
+#define TASK_STATE_1 "S"
+#define DESCR_TASK_STATE_1 "sleeping"
+
#define TASK_UNINTERRUPTIBLE 2
+#define TASK_STATE_2 "D"
+#define DESCR_TASK_STATE_2 "disk sleep"
+
#define __TASK_STOPPED 4
+#define TASK_STATE_4 "T"
+#define DESCR_TASK_STATE_4 "stopped"
+
#define __TASK_TRACED 8
+#define TASK_STATE_8 "t"
+#define DESCR_TASK_STATE_8 "tracing stop"
+
/* in tsk->exit_state */
#define EXIT_ZOMBIE 16
+#define TASK_STATE_16 "Z"
+#define DESCR_TASK_STATE_16 "zombie"
+
#define EXIT_DEAD 32
+#define TASK_STATE_32 "X"
+#define DESCR_TASK_STATE_32 "dead"
+
/* in tsk->state again */
#define TASK_DEAD 64
+#define TASK_STATE_64 "x"
+#define DESCR_TASK_STATE_64 "dead"
+
#define TASK_WAKEKILL 128
+#define TASK_STATE_128 "K"
+#define DESCR_TASK_STATE_128 "wakekill"
+
#define TASK_WAKING 256
+#define TASK_STATE_256 "W"
+#define DESCR_TASK_STATE_256 "waking"
+
#define TASK_STATE_MAX 512
-#define TASK_STATE_TO_CHAR_STR "RSDTtZXxKW"
+#define TASK_STATE_TO_CHAR_STR \
+ TASK_STATE_0 TASK_STATE_1 TASK_STATE_2 TASK_STATE_4 TASK_STATE_8 \
+ TASK_STATE_16 TASK_STATE_32 TASK_STATE_64 TASK_STATE_128 TASK_STATE_256
extern char ___assert_task_state[1 - 2*!!(
sizeof(TASK_STATE_TO_CHAR_STR)-1 != ilog2(TASK_STATE_MAX)+1)];
diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
index cfceb0b..d073711 100644
--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -161,11 +161,17 @@ TRACE_EVENT(sched_switch,
__entry->prev_comm, __entry->prev_pid, __entry->prev_prio,
__entry->prev_state ?
__print_flags(__entry->prev_state, "|",
- { 1, "S"} , { 2, "D" }, { 4, "T" }, { 8, "t" },
- { 16, "Z" }, { 32, "X" }, { 64, "x" },
- { 128, "W" }) : "R",
+ { 1, TASK_STATE_1} , { 2, TASK_STATE_2 },
+ { 4, TASK_STATE_4 }, { 8, TASK_STATE_8 },
+ { 16, TASK_STATE_16 }, { 32, TASK_STATE_32 },
+ { 64, TASK_STATE_64 }, { 128, TASK_STATE_128 },
+ { 256, TASK_STATE_256 }
+ ) : TASK_STATE_0,
__entry->next_comm, __entry->next_pid, __entry->next_prio)
);
+#if TASK_STATE_MAX != 512
+#error "Please add new task state array in __print_flags() above."
+#endif
/*
* Tracepoint for a task being migrated:
--
1.7.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] tracing/sched: Fix task states in sched switch event
2010-05-13 1:21 ` [PATCH 2/4] tracing/sched: Fix task states in sched switch event Steven Rostedt
@ 2010-05-13 6:15 ` Ingo Molnar
0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2010-05-13 6:15 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Andrew Morton, Frederic Weisbecker, Carsten Emde,
Peter Zijlstra
* Steven Rostedt <rostedt@goodmis.org> wrote:
> +#define TASK_STATE_X(num) TASK_STATE_##num " (" DESCR_TASK_STATE_##num ")"
> static const char *task_state_array[] = {
> - "R (running)", /* 0 */
> - "S (sleeping)", /* 1 */
> - "D (disk sleep)", /* 2 */
> - "T (stopped)", /* 4 */
> - "t (tracing stop)", /* 8 */
> - "Z (zombie)", /* 16 */
> - "X (dead)", /* 32 */
> - "x (dead)", /* 64 */
> - "K (wakekill)", /* 128 */
> - "W (waking)", /* 256 */
> + TASK_STATE_X(0),
> + TASK_STATE_X(1),
> + TASK_STATE_X(2),
> + TASK_STATE_X(4),
> + TASK_STATE_X(8),
> + TASK_STATE_X(16),
> + TASK_STATE_X(32),
> + TASK_STATE_X(64),
> + TASK_STATE_X(128),
> + TASK_STATE_X(256)
Hm, this is totally unreadable. What does 'TASK_STATE_X' mean??
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] tracing/sched: Fix task states in sched switch event
@ 2010-05-13 11:10 Carsten Emde
2010-05-13 13:35 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: Carsten Emde @ 2010-05-13 11:10 UTC (permalink / raw)
To: Ingo Molnar
Cc: Steven Rostedt, linux-kernel, And rew Morton, Frederic Weisbecker,
Peter Zijlstra
Hi Ingo,
> Hm, this is totally unreadable. What does 'TASK_STATE_X' mean??
Would this be better?
+#define MAKE_TASK_STATE_STRING(num) TASK_STATE_##num " (" DESCR_TASK_STATE_##num ")"
static const char *task_state_array[] = {
- "R (running)", /* 0 */
- "S (sleeping)", /* 1 */
- "D (disk sleep)", /* 2 */
- "T (stopped)", /* 4 */
- "t (tracing stop)", /* 8 */
- "Z (zombie)", /* 16 */
- "X (dead)", /* 32 */
- "x (dead)", /* 64 */
- "K (wakekill)", /* 128 */
- "W (waking)", /* 256 */
+ MAKE_TASK_STATE_STRING(0),
+ MAKE_TASK_STATE_STRING(1),
+ MAKE_TASK_STATE_STRING(2),
+ MAKE_TASK_STATE_STRING(4),
+ MAKE_TASK_STATE_STRING(8),
+ MAKE_TASK_STATE_STRING(16),
+ MAKE_TASK_STATE_STRING(32),
+ MAKE_TASK_STATE_STRING(64),
+ MAKE_TASK_STATE_STRING(128),
+ MAKE_TASK_STATE_STRING(256)
Carsten.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] tracing/sched: Fix task states in sched switch event
2010-05-13 11:10 [PATCH 2/4] tracing/sched: Fix task states in sched switch event Carsten Emde
@ 2010-05-13 13:35 ` Ingo Molnar
2010-05-13 15:21 ` Steven Rostedt
2010-05-25 12:33 ` Carsten Emde
0 siblings, 2 replies; 6+ messages in thread
From: Ingo Molnar @ 2010-05-13 13:35 UTC (permalink / raw)
To: Carsten Emde
Cc: Steven Rostedt, linux-kernel, And rew Morton, Frederic Weisbecker,
Peter Zijlstra
* Carsten Emde <C.Emde@osadl.org> wrote:
> Hi Ingo,
>
> > Hm, this is totally unreadable. What does 'TASK_STATE_X' mean??
> Would this be better?
> +#define MAKE_TASK_STATE_STRING(num) TASK_STATE_##num " (" DESCR_TASK_STATE_##num ")"
> static const char *task_state_array[] = {
> - "R (running)", /* 0 */
> - "S (sleeping)", /* 1 */
> - "D (disk sleep)", /* 2 */
> - "T (stopped)", /* 4 */
> - "t (tracing stop)", /* 8 */
> - "Z (zombie)", /* 16 */
> - "X (dead)", /* 32 */
> - "x (dead)", /* 64 */
> - "K (wakekill)", /* 128 */
> - "W (waking)", /* 256 */
> + MAKE_TASK_STATE_STRING(0),
> + MAKE_TASK_STATE_STRING(1),
> + MAKE_TASK_STATE_STRING(2),
> + MAKE_TASK_STATE_STRING(4),
> + MAKE_TASK_STATE_STRING(8),
> + MAKE_TASK_STATE_STRING(16),
> + MAKE_TASK_STATE_STRING(32),
> + MAKE_TASK_STATE_STRING(64),
> + MAKE_TASK_STATE_STRING(128),
> + MAKE_TASK_STATE_STRING(256)
The whole enumeration there is pointless in that .c file - it tells nothing to
the code reader.
If it cannot be expressed in a meaningful way then introduce
TASK_STATE_STRINGS_INIT construct that is defined next to the strings (in a .h
file or so) - that way it's a coherent whole.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] tracing/sched: Fix task states in sched switch event
2010-05-13 13:35 ` Ingo Molnar
@ 2010-05-13 15:21 ` Steven Rostedt
2010-05-25 12:33 ` Carsten Emde
1 sibling, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2010-05-13 15:21 UTC (permalink / raw)
To: Ingo Molnar
Cc: Carsten Emde, linux-kernel, And rew Morton, Frederic Weisbecker,
Peter Zijlstra
On Thu, 2010-05-13 at 15:35 +0200, Ingo Molnar wrote:
> * Carsten Emde <C.Emde@osadl.org> wrote:
>
> > Hi Ingo,
> >
> > > Hm, this is totally unreadable. What does 'TASK_STATE_X' mean??
> > Would this be better?
> > +#define MAKE_TASK_STATE_STRING(num) TASK_STATE_##num " (" DESCR_TASK_STATE_##num ")"
> > static const char *task_state_array[] = {
> > - "R (running)", /* 0 */
> > - "S (sleeping)", /* 1 */
> > - "D (disk sleep)", /* 2 */
> > - "T (stopped)", /* 4 */
> > - "t (tracing stop)", /* 8 */
> > - "Z (zombie)", /* 16 */
> > - "X (dead)", /* 32 */
> > - "x (dead)", /* 64 */
> > - "K (wakekill)", /* 128 */
> > - "W (waking)", /* 256 */
> > + MAKE_TASK_STATE_STRING(0),
> > + MAKE_TASK_STATE_STRING(1),
> > + MAKE_TASK_STATE_STRING(2),
> > + MAKE_TASK_STATE_STRING(4),
> > + MAKE_TASK_STATE_STRING(8),
> > + MAKE_TASK_STATE_STRING(16),
> > + MAKE_TASK_STATE_STRING(32),
> > + MAKE_TASK_STATE_STRING(64),
> > + MAKE_TASK_STATE_STRING(128),
> > + MAKE_TASK_STATE_STRING(256)
>
> The whole enumeration there is pointless in that .c file - it tells nothing to
> the code reader.
>
> If it cannot be expressed in a meaningful way then introduce
> TASK_STATE_STRINGS_INIT construct that is defined next to the strings (in a .h
> file or so) - that way it's a coherent whole.
I'm rebasing both pull requests. I'll leave this one out until there is
an agreement.
Thanks,
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] tracing/sched: Fix task states in sched switch event
2010-05-13 13:35 ` Ingo Molnar
2010-05-13 15:21 ` Steven Rostedt
@ 2010-05-25 12:33 ` Carsten Emde
1 sibling, 0 replies; 6+ messages in thread
From: Carsten Emde @ 2010-05-25 12:33 UTC (permalink / raw)
To: Ingo Molnar
Cc: Steven Rostedt, linux-kernel, Andrew Morton, Frederic Weisbecker,
Peter Zijlstra
Hi Ingo,
>>> Hm, this is totally unreadable. What does 'TASK_STATE_X' mean??
>> Would this be better?
>> +#define MAKE_TASK_STATE_STRING(num) TASK_STATE_##num " (" DESCR_TASK_STATE_##num ")"
>> static const char *task_state_array[] = {
>> - "R (running)", /* 0 */
>> - "S (sleeping)", /* 1 */
>> - "D (disk sleep)", /* 2 */
>> - "T (stopped)", /* 4 */
>> - "t (tracing stop)", /* 8 */
>> - "Z (zombie)", /* 16 */
>> - "X (dead)", /* 32 */
>> - "x (dead)", /* 64 */
>> - "K (wakekill)", /* 128 */
>> - "W (waking)", /* 256 */
>> + MAKE_TASK_STATE_STRING(0),
>> + MAKE_TASK_STATE_STRING(1),
>> + MAKE_TASK_STATE_STRING(2),
>> + MAKE_TASK_STATE_STRING(4),
>> + MAKE_TASK_STATE_STRING(8),
>> + MAKE_TASK_STATE_STRING(16),
>> + MAKE_TASK_STATE_STRING(32),
>> + MAKE_TASK_STATE_STRING(64),
>> + MAKE_TASK_STATE_STRING(128),
>> + MAKE_TASK_STATE_STRING(256)
>
> The whole enumeration there is pointless in that .c file - it tells nothing to
> the code reader.
>
> If it cannot be expressed in a meaningful way then introduce
> TASK_STATE_STRINGS_INIT construct that is defined next to the strings (in a .h
> file or so) - that way it's a coherent whole.
This is what I did and submitted some time ago. Is there anything else
you want me to change?
Thanks,
Carsten.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-05-25 12:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-13 11:10 [PATCH 2/4] tracing/sched: Fix task states in sched switch event Carsten Emde
2010-05-13 13:35 ` Ingo Molnar
2010-05-13 15:21 ` Steven Rostedt
2010-05-25 12:33 ` Carsten Emde
-- strict thread matches above, loose matches on Subject: below --
2010-05-13 1:21 [PATCH 0/4] [GIT PULL] tracing: updates Steven Rostedt
2010-05-13 1:21 ` [PATCH 2/4] tracing/sched: Fix task states in sched switch event Steven Rostedt
2010-05-13 6:15 ` Ingo Molnar
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.