Re: [PATCH 1/2] PF_DEAD: cleanup usage
From: Linus Torvalds
Date: Sat Nov 26 2005 - 13:47:25 EST
On Sat, 26 Nov 2005, Oleg Nesterov wrote:
>
> Ok, I see you point now, thanks.
Well, in all fairness, it's entirely possible (nay, likely) that PF_DEAD
just isn't relevant the way it used to be.
Looking at the history, the immediate reason for PF_DEAD was (I think)
that we had a race where we would first mark the task as a ZOMBIE, and
then if it was self-reaping, we'd mark it dead in release_task(). But we
dropped the tasklist_lock in between, which meant that the real parent
could come in and reap it just before it reaped itself, and all hell would
break lose.
That bug was fixed sixteen hours after PF_DEAD was introduced (thanks to a
bug-on on PF_DEAD that never even made the kernel history, I suspect), and
I doubt PF_DEAD has done anything for us since (and this was two years
ago).
What I'm trying to say is that it's entirely possible that your cleanup is
just way overdue, and we shouldn't have that separate PF_DEAD thing any
more. My arguments against your patch is mainly me just being nervous, and
I'm not 100% convinced they are valid and good arguments.
HOWEVER. I just noticed something strange. EXIT_DEAD should be in
"task->exit_state", not in "task->state". So there's something strange
going on in that neck of the woods _anyway_. That whole
...
if (unlikely(prev->flags & PF_DEAD))
prev->state = EXIT_DEAD;
...
in kernel/sched.c seems totally bogus.
I think we should remove that thing entirely, since it's actually a total
no-op, apart from doing something that is actively wrong. The "exit_state"
flag should already _be_ EXIT_DEAD, no? And the "task->state" flag should
never be EXIT_DEAD in the first place.
And now that "exit_state" is already separate from "state", the reason for
PF_DEAD really is gone, and it could be replaced with a
tsk->exit_state & EXIT_DEAD
test instead.
Roland added to Cc:, because the exit_state splitup was his, I think.
Oleg? Roland?
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/