All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Linus Torvalds <torvalds@osdl.org>
Cc: Oleg Nesterov <oleg@tv-sign.ru>,
	linux-kernel@vger.kernel.org, Andrew Morton <akpm@osdl.org>
Subject: Re: [PATCH 1/2] PF_DEAD: cleanup usage
Date: Sun, 27 Nov 2005 12:55:36 +0100	[thread overview]
Message-ID: <20051127115536.GA22229@elte.hu> (raw)
In-Reply-To: <Pine.LNX.4.64.0511250950450.13959@g5.osdl.org>


* Linus Torvalds <torvalds@osdl.org> wrote:

> > > I think it is better to set EXIT_DEAD in do_exit(), along with PF_DEAD 
> > > flag.
> > 
> > nice idea - your patch looks good to me.
> 
> 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.

but this was in times when we did alot of nontrivial operations after we 
marked a task "dead". Today we do this:

        /* PF_DEAD causes final put_task_struct after we schedule. */
        preempt_disable();
        BUG_ON(tsk->flags & PF_DEAD);
        tsk->flags |= PF_DEAD;

        schedule();
        BUG();

(i introduced the above changes to make more of the exit path 
preemptable.)

PF_DEAD has zero relevance by today, and Oleg's patches are perfectly 
correct and dont add the kind of risk that they'd have meant in 2003.

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

yeah, PF_DEAD had its purpose back when we first marked a task dead, 
then we did the release_task(). We used to have problems with 
proc_pid_flush() which could sleep, which would lose the TASK_ZOMBIE or 
TASK_DEAD flag and we'd come back from the 'final' schedule().

today that's impossible, due to the code above - we only mark it PF_DEAD 
straight before going into the final schedule().

	Ingo

      parent reply	other threads:[~2005-11-27 11:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-11-24 16:02 [PATCH 1/2] PF_DEAD: cleanup usage Oleg Nesterov
2005-11-25  5:12 ` Ingo Molnar
2005-11-25 18:01   ` Linus Torvalds
2005-11-26 10:46     ` Oleg Nesterov
2005-11-26 17:55       ` Linus Torvalds
2005-11-26 19:21         ` Oleg Nesterov
2005-11-26 18:47           ` Linus Torvalds
2005-11-27 12:18             ` Ingo Molnar
2005-11-27 13:02             ` Oleg Nesterov
2005-11-27 11:55     ` Ingo Molnar [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20051127115536.GA22229@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@tv-sign.ru \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.