All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: jan.kratochvil@redhat.com, vda.linux@googlemail.com,
	linux-kernel@vger.kernel.org, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, indan@nul.nu, bdonlan@gmail.com
Subject: Re: [PATCH 10/10] ptrace: implement group stop notification for ptracer
Date: Thu, 19 May 2011 19:13:27 +0200	[thread overview]
Message-ID: <20110519171327.GA19698@redhat.com> (raw)
In-Reply-To: <20110519165722.GP627@htj.dyndns.org>

On 05/19, Tejun Heo wrote:
>
> Hey,
>
> On Thu, May 19, 2011 at 06:32:46PM +0200, Oleg Nesterov wrote:
> > > +static void ptrace_trap_notify(struct task_struct *t)
> > > +{
> > > +	siginfo_t *si = t->last_siginfo;
> > > +
> > > +	WARN_ON_ONCE(!(t->ptrace & PT_SEIZED));
> > > +	assert_spin_locked(&t->sighand->siglock);
> > > +
> > > +	/*
> > > +	 * @t is being ptraced and new SEIZE behavior is in effect.
> > > +	 * Schedule sticky trap which will clear on the next GETSIGINFO.
> > > +	 */
> > > +	t->jobctl |= JOBCTL_TRAP_NOTIFY;
> >
> > This is also set by do_signal_stop(). Cleared by PTRACE_GETSIGINFO.
> >
> > How can this work? Doesn't this mean PTRACE_GETSIGINFO becomes mandatory
> > before PTRACE_CONT? IOW, unless the tracee does PTRACE_GETSIGINFO to clear
> > this bit, PTRACE_CONT just leads to another trap, no?
>
> Yes, group stop state change raises a sticky trap condition which is
> cleared by GETSIGINFO.

Hmm. At least now I understand the meaining what "sticky" means in
this discussion ;) I was confused.

> > > +	if (task_is_traced(t) && si && si->si_code == PTRACE_STOP_SI_CODE) {
> >
> > OK, this PTRACE_STOP_SI_CODE check is clear. But the same check in
> > ptrace_check_attach() looks confusing, why can't we set BLOCK_NOTIFY
> > unconditionally?
>
> It's an optimization.  If we set the flag, we'll have to acquire
> siglock

OK, I see.

> > > +		t->jobctl |= JOBCTL_TRAPPING;
> > > +		if (!(t->jobctl & JOBCTL_BLOCK_NOTIFY))
> > > +			signal_wake_up(t, true);
> >
> > Could you please remind me why we can't avoid the awful ptrace_wait_trapping()
> > in do_wait() paths? Assuming that ptrace_check_attach() does this. I got lost
> > a bit.
>
> Please consider the following scenario.
>
> 1. Tracee is in group stop and stops at TRAP_STOP notifying the
>    tracer.
>
> 2. Tracer does WNOWAIT wait(2) and determines that the tracee is
>    trapped in TRAP_STOP.
>
> 3. Something generates SIGCONT which finishes the group stop and
>    triggers the notification re-trapping.
>
> 4. While tracee is re-trapping, tracer issues WNOHANG

OK. I still hope we can avoid this somehow. May be play with exit_code
so that do_wait() can succeed even if the JOBCTL_TRAPPING tracee is
running. Perhaps.

If only we could notify the tracer from ptrace_trap_notify... IIUC,
this is the only problem? I mean, apart from this there is no need
to wake up the tracee.

Oleg.


  reply	other threads:[~2011-05-19 17:15 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-16 18:17 [PATCHSET ptrace] ptrace: implement PTRACE_SEIZE/INTERRUPT and group stop notification, take#2 Tejun Heo
2011-05-16 18:17 ` [PATCH 01/10] signal: remove three noop tracehooks Tejun Heo
2011-05-17 16:22   ` Christoph Hellwig
2011-05-17 16:27     ` Tejun Heo
2011-05-18 18:45   ` Oleg Nesterov
2011-05-19 12:11     ` Tejun Heo
2011-05-19 16:10       ` Oleg Nesterov
2011-05-16 18:17 ` [PATCH 02/10] job control: introduce JOBCTL_TRAP_STOP and use it for group stop trap Tejun Heo
2011-05-18 16:48   ` Oleg Nesterov
2011-05-18 16:57     ` Oleg Nesterov
2011-05-19 10:19     ` Tejun Heo
2011-05-19 16:19       ` Oleg Nesterov
2011-05-16 18:17 ` [PATCH 03/10] ptrace: implement PTRACE_SEIZE Tejun Heo
2011-05-18  0:40   ` Denys Vlasenko
2011-05-18  9:55     ` Tejun Heo
2011-05-18 10:44       ` Denys Vlasenko
2011-05-18 11:14         ` Tejun Heo
2011-05-19 14:17       ` Tejun Heo
2011-05-19 15:02         ` Tejun Heo
2011-05-19 19:31         ` Pedro Alves
2011-05-19 22:42           ` Denys Vlasenko
2011-05-19 23:00             ` Pedro Alves
2011-05-20  1:44               ` Denys Vlasenko
2011-05-20  8:56                 ` Pedro Alves
2011-05-20  9:12                   ` Tejun Heo
2011-05-20  9:07               ` Tejun Heo
2011-05-20  9:27                 ` Pedro Alves
2011-05-20  9:31                   ` Tejun Heo
2011-05-24  9:49                     ` Pedro Alves
2011-05-24 12:00                       ` Tejun Heo
2011-05-24 12:36                         ` Pedro Alves
2011-05-24 14:02                           ` Tejun Heo
2011-05-24 14:55                             ` Pedro Alves
2011-05-25 18:18                             ` Oleg Nesterov
2011-05-26  9:10                               ` Tejun Heo
2011-05-26 10:01                                 ` Pedro Alves
2011-05-26 10:11                                   ` Tejun Heo
2011-05-26 14:55                                 ` Oleg Nesterov
2011-05-23 13:09         ` Oleg Nesterov
2011-05-23 12:43       ` Oleg Nesterov
2011-05-24 10:28         ` Tejun Heo
2011-05-25 18:29           ` Oleg Nesterov
2011-05-26  9:14             ` Tejun Heo
2011-05-26 15:01               ` Oleg Nesterov
2011-05-27 18:21                 ` Tejun Heo
2011-05-30 19:22                   ` Oleg Nesterov
     [not found]                     ` <BANLkTimupSd774N-VBoswOj+Dza=5ofvWQ@mail.gmail.com>
2011-05-31 19:08                       ` Oleg Nesterov
2011-05-31 21:32                         ` Linus Torvalds
2011-06-01 20:04                           ` Oleg Nesterov
2011-06-01  5:34                         ` Tejun Heo
2011-06-01 20:08                           ` Oleg Nesterov
2011-06-02  5:01                             ` Tejun Heo
2011-05-18 18:17   ` Oleg Nesterov
2011-05-19 10:34     ` Tejun Heo
2011-05-16 18:17 ` [PATCH 04/10] ptrace: implement PTRACE_INTERRUPT Tejun Heo
2011-05-18 18:38   ` Oleg Nesterov
2011-05-19 12:07     ` Tejun Heo
2011-05-19 16:21       ` Oleg Nesterov
2011-05-16 18:17 ` [PATCH 05/10] ptrace: restructure ptrace_getsiginfo() Tejun Heo
2011-05-16 18:17 ` [PATCH 06/10] ptrace: add siginfo.si_pt_flags Tejun Heo
2011-05-16 18:17 ` [PATCH 07/10] ptrace: make group stop state visible via PTRACE_GETSIGINFO Tejun Heo
2011-05-19 16:27   ` Oleg Nesterov
2011-05-19 16:40     ` Tejun Heo
2011-05-16 18:17 ` [PATCH 08/10] ptrace: don't let PTRACE_SETSIGINFO override __SI_TRAP siginfo Tejun Heo
2011-05-16 18:17 ` [PATCH 09/10] ptrace: add JOBCTL_BLOCK_NOTIFY Tejun Heo
2011-05-19 16:32   ` Oleg Nesterov
2011-05-19 16:44     ` Tejun Heo
2011-05-19 16:48       ` Oleg Nesterov
2011-05-19 16:58         ` Tejun Heo
2011-05-16 18:17 ` [PATCH 10/10] ptrace: implement group stop notification for ptracer Tejun Heo
2011-05-19 16:32   ` Oleg Nesterov
2011-05-19 16:57     ` Tejun Heo
2011-05-19 17:13       ` Oleg Nesterov [this message]
2011-05-19 22:48         ` Denys Vlasenko
2011-05-20  8:59           ` Tejun Heo
2011-05-23 13:34             ` Oleg Nesterov
2011-05-20  8:46         ` Tejun Heo
2011-05-19 16:58     ` Oleg Nesterov
2011-05-23 11:45       ` Oleg Nesterov
2011-05-24 13:44         ` Tejun Heo
2011-05-24 15:44           ` Tejun Heo
2011-05-26 14:44           ` Oleg Nesterov
2011-05-28  7:32             ` Tejun Heo
2011-05-18 18:50 ` [PATCHSET ptrace] ptrace: implement PTRACE_SEIZE/INTERRUPT and group stop notification, take#2 Oleg Nesterov
2011-05-19 12:08   ` Tejun Heo
2011-05-19 15:04 ` Linus Torvalds
2011-05-19 15:19   ` Tejun Heo
2011-05-19 22:45   ` Denys Vlasenko

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=20110519171327.GA19698@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bdonlan@gmail.com \
    --cc=indan@nul.nu \
    --cc=jan.kratochvil@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vda.linux@googlemail.com \
    /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.