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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux