mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Roland McGrath <roland@redhat.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Alan Cox <alan@linux.intel.com>, Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] make task_struct->signal immutable/refcountable
Date: Fri,  9 Apr 2010 12:38:16 -0700 (PDT)	[thread overview]
Message-ID: <20100409193816.A3C36B082@magilla.sf.frob.com> (raw)
In-Reply-To: Oleg Nesterov's message of  Friday, 19 March 2010 19:40:17 +0100 <20100319184017.GA512@redhat.com>

Acked-by: Roland McGrath <roland@redhat.com>
but some comments:

> This patch adds the reference counter, sigcnt, into signal_struct. This
> reference is owned by task_struct and it is dropped in __put_task_struct().

I'm not thrilled about the field name, but whatever.  We now have three
atomic counts with subtly different meanings, and no comments anywhere
really explaining it all.

* live: created and not yet begun to exit
* count: not yet reaped
* sigcnt: task_struct not yet freed

> Perhaps it makes sense to export get/put_signal_struct() later, but
> currently I don't see the immediate reason.

We should never export those at all.  In fact, I would not even make it a
separate inline.  We're here to simplify the lifetime rules, not give new
ways to complicate them.  If anybody wants to keep a signal_struct alive,
they can do it with get_task_struct().

> 	- with or without this patch signal_struct->count should go away,
> 	  or at least it should be "int nr_threads" for fs/proc. This will
> 	  be addressed later.

Indeed.  IMHO adding yet another atomic count here is OK only on the
proviso that we will be getting rid of the old one RSN.


Thanks,
Roland

      reply	other threads:[~2010-04-09 19:38 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-19 18:40 Oleg Nesterov
2010-04-09 19:38 ` Roland McGrath [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=20100409193816.A3C36B082@magilla.sf.frob.com \
    --to=roland@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®