[PATCH 0/1] tracing: Fix erroneous task states in sched_switch event

[PATCH 0/1] tracing: Fix erroneous task states in sched_switch event

[Carsten Emde]

A recent change in the task state definitions was omitted to apply to the
sched_switch event output.

In addition, move task state definitions to a common place. This ensures that
future changes make it to all places where needed.

        Carsten. 

[PATCH 1/1] fix-taskstates-in-sched_switch-trace.patch

[Carsten Emde]

[-- Attachment #1: fix-taskstates-in-sched_switch-trace.patch --]
[-- Type: text/plain, Size: 4630 bytes --]

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>

Index: head/include/trace/events/sched.h
===================================================================
--- head.orig/include/trace/events/sched.h
+++ head/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:
Index: head/include/linux/sched.h
===================================================================
--- head.orig/include/linux/sched.h
+++ head/include/linux/sched.h
@@ -172,7 +172,9 @@ print_cfs_rq(struct seq_file *m, int cpu
 
 /*
  * 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
  * 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)];
Index: head/fs/proc/array.c
===================================================================
--- head.orig/fs/proc/array.c
+++ head/fs/proc/array.c
@@ -129,21 +129,22 @@ static inline void task_name(struct seq_
 
 /*
  * 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)

Re: [PATCH 1/1] fix-taskstates-in-sched_switch-trace.patch

[Steven Rostedt]

I just notice that this patch was never applied.

Anyone against applying it?

-- Steve


On Thu, 2010-03-11 at 13:24 +0100, Carsten Emde wrote:
> plain text document attachment
> (fix-taskstates-in-sched_switch-trace.patch)
> 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>
> 
> Index: head/include/trace/events/sched.h
> ===================================================================
> --- head.orig/include/trace/events/sched.h
> +++ head/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:
> Index: head/include/linux/sched.h
> ===================================================================
> --- head.orig/include/linux/sched.h
> +++ head/include/linux/sched.h
> @@ -172,7 +172,9 @@ print_cfs_rq(struct seq_file *m, int cpu
>  
>  /*
>   * 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
>   * 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)];
> Index: head/fs/proc/array.c
> ===================================================================
> --- head.orig/fs/proc/array.c
> +++ head/fs/proc/array.c
> @@ -129,21 +129,22 @@ static inline void task_name(struct seq_
>  
>  /*
>   * 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)
> 

Re: [PATCH 1/1] fix-taskstates-in-sched_switch-trace.patch

[Carsten Emde]

Hi Steven,

> I just notice that this patch was never applied. Anyone against
> applying it?
Needless to say that I would like to see it applied.

This patch removes duplicate strings and moves all task state related
definitions to a single location to prevent future changes from being
made at one place and omitted at another place.

In addition, if fixes an incorrectly displayed task state of the
sched_switch trace event (which probably was the result of having the
same info at several places).

The principle of this patch made it already into PREEMPT_RT of 2.6.31
and 2.6.33 nearly 3 months ago - and is running smoothly since then.

Thanks for taking care of the patch.

	Carsten.