Re: [PATCH 1/1] fix-task-states-in-sched_switch-event.patch

[Peter Zijlstra <peterz@infradead.org>]

On Mon, 2010-05-17 at 17:21 +0200, Carsten Emde wrote:

> > 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

We could manually add:

#define EXIT_ZOMBIE TASK_ZOMBIE
#define EXIT_DEAD TASK_DEAD

But those two __TASK ones are unfortunate indeed.

> 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,

#define TASK_STATE(tstate, tstate_c, tstate_s) \
	{ __TASK_##tstate, tstate_c },
#include <linux/task_state.h>
#undef TASK_STATE

Should get you mostly there I guess, trick would be making the user deal
with { 0, "R" }

> 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?

Dunno, I guess we can do with your version, just wanted to mention this
method.