All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Andrew Morton <akpm@osdl.org>
Cc: Jeremy Kerr <jeremy@redfishsoftware.com.au>,
	lkml - Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@osdl.org>
Subject: Re: [PATCH] Fix signal race during process exit
Date: Wed, 02 Jun 2004 16:49:48 +1000	[thread overview]
Message-ID: <1086158988.29381.277.camel@bach> (raw)
In-Reply-To: <20040601225703.6c697bed.akpm@osdl.org>

On Wed, 2004-06-02 at 15:57, Andrew Morton wrote:
> void update_process_times(int user_tick)
> {
> 	struct task_struct *p = current;
> 	int cpu = smp_processor_id(), system = user_tick ^ 1;
> versus:
> 
> void __exit_sighand(struct task_struct *tsk)
> {
> 	struct sighand_struct * sighand = tsk->sighand;

No.  tsk == current for __exit_sighand.  You know, getting current is SO
expensive, and so we should PASS IT to functions explicitly!

One of my pet gripes: this code is badly obfuscated by this.  Is it just
me?

> And there's a little window at the end of exit_notify() where the exitting
> task (which is still "current" on its CPU) can take a timer interrupt while
> in a state TASK_ZOMBIE.  The CPU which is running wait4() will run
> release_task() for the exitting task and the above race can occur.

Hmm, while we're at it, the task seems to release itself while running
here: exit_notify() -> release_task() -> put_task_struct() ->
__put_task_struct() -> BOOM?

Surely not, what am I missing?

> Right now, I see no alternative to adding locking which pins task->sighand
> while the timer handler is running.  Taking tasklist_lock on each timer
> tick will hurt - maybe a new per-process lock is needed?

Hmm, a per-cpu cache of exited tasks: one task for each CPU.  We hold a
reference to the task struct until the next exit on the same CPU
happens?  We could also reuse that cache for fork()...

Rusty.
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell


  reply	other threads:[~2004-06-02  6:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-06-02  2:13 [PATCH] Fix signal race during process exit Jeremy Kerr
2004-06-02  5:57 ` Andrew Morton
2004-06-02  6:49   ` Rusty Russell [this message]
2004-06-02  7:08     ` Andrew Morton
2004-06-02  7:16       ` Andrew Morton
2004-06-02  8:13         ` Jeremy Kerr
  -- strict thread matches above, loose matches on Subject: below --
2004-06-04  1:21 Roland McGrath
2004-06-04  1:30 ` Andrew Morton
2004-06-10  1:48   ` Roland McGrath
2004-06-10  2:20     ` Andrew Morton
2004-06-10  2:51     ` Rusty Russell

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=1086158988.29381.277.camel@bach \
    --to=rusty@rustcorp.com.au \
    --cc=akpm@osdl.org \
    --cc=jeremy@redfishsoftware.com.au \
    --cc=linux-kernel@vger.kernel.org \
    --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.