Re: [PATCH 1/2] PF_DEAD: cleanup usage

[Oleg Nesterov <oleg@tv-sign.ru>]

Linus Torvalds wrote:
> 
> I'm not entirely convinced.
> 
> The thing is, We used to have DEAD in the task state flags, ie TASK_ZOMBIE
> was it.
> 
> We started using PF_DEAD in 2003 with this commit message:
> 
> Author: Linus Torvalds <torvalds@home.osdl.org>  2003-10-26 03:16:23
> 
>     Add a sticky "PF_DEAD" task flag to keep track of dead processes.
> 
>     Use this to simplify 'finish_task_switch', but perhaps more
>     importantly we can use this to track down why some processes
>     seem to sometimes not die properly even after having been
>     marked as ZOMBIE. The "task->state" flags are too fluid to
>     allow that well.
> 
> ie the PF_DEAD flag was never really about is _needing_ it: it was all
> about being able to safely check it _without_ having to rely on
> task->state.
> 
> So putting it back into task->state is not wrong per se, but it kind of
> misses the point of why it was somewhere else in the first place (or
> rather, why it was there in the _second_ place, since it was in
> task->state in the first place and got moved out of there).

schedule:

	if (unlikely(prev->flags & PF_DEAD))
		prev->state = EXIT_DEAD;

Which means: "If PF_DEAD is set, ignore ->state value. It should be TASK_RUNNING,
but we have to change it, otherwise the task won't be deactivated. We are using
EXIT_DEAD (which should live only in ->exit_state) because other TASK_XXX values
won't work".

So in my opinion PF_DEAD has already slipped into the ->state partly. And I still
think that at least the first patch is ok, it does not change the things, but may
be considered as microoptimization.

On the other hand this microoptimization is negligible, all cleanups is a matter
of taste always, so let's forget about it if you don't change you mind.

Oleg.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/