All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	hch@infradead.org
Subject: Re: [PATCH 2/8] kill tracehook_notify_death()
Date: Thu, 23 Jun 2011 15:21:26 +0200	[thread overview]
Message-ID: <20110623132126.GA10410@redhat.com> (raw)
In-Reply-To: <20110623122253.GM30101@htj.dyndns.org>

Hi Tejun,

On 06/23, Tejun Heo wrote:
>
> Hello, Oleg.
>
> On Wed, Jun 22, 2011 at 11:08:34PM +0200, Oleg Nesterov wrote:
> > Kill tracehook_notify_death(), reimplement the logic in its caller,
> > exit_notify().
> >
> > This also fixes a minor bug, if the exiting task is the group_leader
> > and it is traced by its real_parent, tracehook_notify_death() returns
> > task->exit_signal or SIGCHLD depending on thread_group_empty(), this
> > looks strange.
>
> Maybe we should do the above in a separate patch?

Do you think this makes sense? OK, I can do this...

>
> > -	if (tsk->exit_signal != SIGCHLD && !task_detached(tsk) &&
> > +	if (thread_group_leader(tsk) && tsk->exit_signal != SIGCHLD &&
>
> Hmmm... it probably depends on POV but wouldn't (exit_signal != -1 &&
> exit_signal != SIGCHLD) be easier?

I disagree.

> The logic here is about demoting
> specials sigs to SIGCHLD under certain circumstances

Yes. And what is ->exit_signal? It is in fact per-process, lives in the
group_leader's task_struct. We could move it to signal_struct.

> and the check is
> there to prevent promoting -1 to SIGCHLD.

Yes, because we should never change ->exit_signal for sub-threads.

And it doesn't make sense to check exec ids (this is again per-process,
should be cleanuped) unless the task is the group leader.

> thread_group_leader() seems
> unnecessarily indirect.

This is what I disagree with. Contrary, I think thread_group_leader() exactly
explains what do we want to check. (but once again, exec_id logic should be
cleanuped, not only in this function).

Oleg.


  reply	other threads:[~2011-06-23 13:23 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-17 14:50 [PATCHSET] ptrace: kill most tracehooks Tejun Heo
2011-06-17 14:50 ` [PATCH 1/7] ptrace: kill task_ptrace() Tejun Heo
2011-06-17 14:50 ` [PATCH 2/7] ptrace: introduce ptrace_event_enabled() and simplify ptrace_event() and tracehook_prepare_clone() Tejun Heo
2011-06-17 14:50 ` [PATCH 3/7] ptrace: move SIGTRAP on exec(2) logic to ptrace_event() Tejun Heo
2011-06-20 20:25   ` Oleg Nesterov
2011-06-21  7:21     ` Tejun Heo
2011-06-21 20:40       ` Oleg Nesterov
2011-06-23  8:58         ` Tejun Heo
2011-06-17 14:50 ` [PATCH 4/7] ptrace: kill trivial tracehooks Tejun Heo
2011-06-17 14:50 ` [PATCH 5/7] ptrace: kill clone/exec tracehooks Tejun Heo
2011-06-20 20:33   ` Oleg Nesterov
2011-06-21  7:24     ` Tejun Heo
2011-06-17 14:50 ` [PATCH 6/7] ptrace: kill detah tracehooks Tejun Heo
2011-06-20 19:39   ` Oleg Nesterov
2011-06-21 20:23     ` Oleg Nesterov
2011-06-23  9:24     ` Tejun Heo
2011-06-22 21:07   ` [PATCH 0/8] kill task_detached() (Was: ptrace: kill detah tracehooks) Oleg Nesterov
2011-06-22 21:08     ` [PATCH 1/8] make do_notify_parent() return bool Oleg Nesterov
2011-06-23  9:52       ` Tejun Heo
2011-06-22 21:08     ` [PATCH 2/8] kill tracehook_notify_death() Oleg Nesterov
2011-06-23 12:22       ` Tejun Heo
2011-06-23 13:21         ` Oleg Nesterov [this message]
2011-06-23 13:27           ` Tejun Heo
2011-06-23 13:28             ` Oleg Nesterov
2011-06-23 17:06               ` Oleg Nesterov
2011-06-25 14:15                 ` Tejun Heo
2011-06-26 20:51                   ` Oleg Nesterov
2011-06-27  8:24                     ` Tejun Heo
2011-06-27 14:21                       ` Oleg Nesterov
2011-06-27 14:36                         ` Tejun Heo
2011-06-22 21:08     ` [PATCH 3/8] __ptrace_detach: avoid task_detached(), check do_notify_parent() Oleg Nesterov
2011-06-23 13:25       ` Tejun Heo
2011-06-22 21:09     ` [PATCH 4/8] make do_notify_parent() __must_check, update the callers Oleg Nesterov
2011-06-23 13:38       ` Tejun Heo
2011-06-22 21:09     ` [PATCH 5/8] reparent_leader: check EXIT_DEAD instead of task_detached() Oleg Nesterov
2011-06-22 21:09     ` [PATCH 6/8] kill task_detached() Oleg Nesterov
2011-06-22 21:10     ` [PATCH 7/8] do not change dead_task->exit_signal Oleg Nesterov
2011-06-22 21:10     ` [PATCH 8/8] redefine thread_group_leader() as exit_signal >= 0 Oleg Nesterov
2011-06-23 13:56       ` Tejun Heo
2011-06-17 14:50 ` [PATCH 7/7] ptrace: s/tracehook_tracer_task()/ptrace_parent()/ Tejun Heo
2011-06-20 20:16   ` Oleg Nesterov
2011-06-21 11:44     ` John Johansen
2011-06-23  9:14     ` Tejun Heo
2011-06-20 11:16 ` [PATCHSET] ptrace: kill most tracehooks Christoph Hellwig

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=20110623132126.GA10410@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.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.