linux-kernel - Re: [PATCH 1/1] fix-task-states-in-sched

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4BF15F08.5050108@osadl.org>
Date:	Mon, 17 May 2010 17:21:44 +0200
From:	Carsten Emde <C.Emde@...dl.org>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	Steven Rostedt <rostedt@...dmis.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Frederic Weisbecker <fweisbec@...il.com>
Subject: Re: [PATCH 1/1] fix-task-states-in-sched_switch-event.patch

Hi Peter,

>>   #define TASK_RUNNING           0
>> +#define TASK_STATE_0           "R"
>> +#define TASK_STATE_NAME_0      "running"
>> +
>>   #define TASK_INTERRUPTIBLE     1
>> +#define TASK_STATE_1           "S"
>> +#define TASK_STATE_NAME_1      "sleeping"
>> +
>>   #define TASK_UNINTERRUPTIBLE   2
>> +#define TASK_STATE_2           "D"
>> +#define TASK_STATE_NAME_2      "disk sleep"
>> +
>>   #define __TASK_STOPPED         4
>> +#define TASK_STATE_4           "T"
>> +#define TASK_STATE_NAME_4      "stopped"
>> +
>>   #define __TASK_TRACED          8
>> +#define TASK_STATE_8           "t"
>> +#define TASK_STATE_NAME_8      "tracing stop"
>> +
>>   /* in tsk->exit_state */
>>   #define EXIT_ZOMBIE            16
>> +#define TASK_STATE_16          "Z"
>> +#define TASK_STATE_NAME_16     "zombie"
>> +
>>   #define EXIT_DEAD              32
>> +#define TASK_STATE_32          "X"
>> +#define TASK_STATE_NAME_32     "dead"
>> +
>>   /* in tsk->state again */
>>   #define TASK_DEAD              64
>> +#define TASK_STATE_64          "x"
>> +#define TASK_STATE_NAME_64     "dead"
>> +
>>   #define TASK_WAKEKILL          128
>> +#define TASK_STATE_128         "K"
>> +#define TASK_STATE_NAME_128    "wakekill"
>> +
>>   #define TASK_WAKING            256
>> +#define TASK_STATE_256         "W"
>> +#define TASK_STATE_NAME_256    "waking"
>
> Since we all love vile macro magic, is the below any better?
>
> include/linux/task_states.h
>
> TASK_STATE(RUNNING, "R", "running")
> TASK_STATE(INTERRUPTIBLE, "S", "sleeping")
> ...
Well, yes, this looks very nice and is perfectly readable and
maintainable.

> enum {
> #define TASK_STATE(tstate, tstate_c, tstate_s) __TASK_##tstate,
> #include<linux/task_states.h>
> #undef TASK_STATE
> };
>
> enum {
> #define TASK_STATE(tstate, tstate_c, tstate_s) \
> 	TASK_##tstate = 1<<  __TASK_##tstate,
> #include<linux/task_states.h>
> #undef TASK_STATE
> };
>
> const char *task_state_to_char =
> #define TASK_STATE(tstate, tstate_c, tstate_s) tstate_c
> #include<linux/task_states.h>
> #undef TASK_STATE
> ;
>
> const char *task_state_to_string[] = {
> #define TASK_STATE(tstate, tstate_c, tstate_s) tstate_s,
> #include<linux/task_states.h>
> #undef TASK_STATE
> };
I find this section less convincing (although certainly
indistinguishable from magic).

In addition, we need to take care of the various state name prefixes
TASK, __TASK and EXIT and name clashes:
TASK_RUNNING
TASK_INTERRUPTIBLE
TASK_UNINTERRUPTIBLE
__TASK_STOPPED
__TASK_TRACED
EXIT_ZOMBIE
EXIT_DEAD
TASK_DEAD
TASK_WAKEKILL
TASK_WAKING

And we still need to maintain the defines in include/trace/events/
sched.h:
{ 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,

If we could use a general approach for all states, I would immediately
go for your proposal. But since we anyway need to define the states
individually, I would vote for the current version of the patch.

Or would you prefer to simply apply a minimal fix to correct the
erroneous output of the sched_switch event and to leave the rest as an
exercise for the future?


	Carsten.

View attachment "fix-just-task-states-in-sched_switch-event.patch" of type "text/plain" (604 bytes)