All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Denys Vlasenko <vda.linux@googlemail.com>
Cc: Oleg Nesterov <oleg@redhat.com>,
	Roland McGrath <roland@redhat.com>,
	jan.kratochvil@redhat.com, linux-kernel@vger.kernel.org,
	torvalds@linux-foundation.org, akpm@linux-foundation.org
Subject: Re: [RFC] Proposal for ptrace improvements
Date: Wed, 2 Mar 2011 15:43:22 +0100	[thread overview]
Message-ID: <20110302144322.GJ3319@htj.dyndns.org> (raw)
In-Reply-To: <AANLkTi=tQ0kLB_MiTArpNAWHUT0id_dbXXLu9UpPhRfN@mail.gmail.com>

Hi,

On Wed, Mar 02, 2011 at 12:48:56PM +0100, Denys Vlasenko wrote:
> > Of course, all ptrace traps are SIGTRAPs.
> 
> Except for those SIGSTOPs in children on auto-attach
> via PTRACE_O_TRACE[V]FORK / PTRACE_O_TRACECLONE options...

Aren't they just using SIGSTOP's to trigger signal delivery path?  The
signal can be delivered, right?  Checking the source code.... Yeah, it
genuinely generates SIGSTOP.

> >> Why not SIGCONT? This event is, after all, caused by SIGCONT.
> >> It would be so much nicer to be able to detect it with single if()
> >> in the debugger...
> >
> > I disagree.  It's a ptrace trap.  It should use SIGTRAP.  We just need
> > well defined siginfo output to distinguish between them.  It's not
> > like we can avoid siginfo anyway.
> 
> Performance problem here. Strace is already suffering from being
> rather slow, especially for multi-threaded processes.
> 
> So far strace was able to avoid querying siginfo on every stop.
> 
> In order to make job control stop work properly, it will now need
> to query siginfo, but only if signo==SIGSTOP. SIGSTOPs don't
> occur too often, definitely not twice per syscall as SIGTRAPs do,
> so it's not a problem.
> 
> With your proposal to show resume-from-job-control-stop-via-SIGCONT
> as SIGTRAP, *every* SIGTRAP stop needs to be followed
> by PTRACE_GETSIGINFO.

I don't think it's a good idea to make these basic design choices
based on optimization concerns like the above.  We're not talking
about read/write(2) here.  If PTRACE_GETSIGINFO really becomes that
much of a performance bottleneck, we can put it in vdso, but I really
doubt it would come to that.

> int main()
> {
>        signal(SIGSTOP, sig);
>        signal(SIGCONT, sig);
>        signal(SIGWINCH, sig);
>        signal(SIGABRT, sig);
>  again:
>        printf("PID: %d\n", getpid());
>        fflush(NULL);
>        errno = 0;
>        sleep(30);
>        int e = errno;
>        printf("after sleep: errno=%d %s\n", e, strerror(e));
>        if (e) goto again;
>        return 0;
> }
> 
> # ./a.out
> PID: 16382
>  <------ kill -STOP 16382
>  <------ kill -ABRT 16382
>  <------ kill -WINCH 16382
>  <------ kill -CONT 16382
> sig: 28 Window changed
> sig: 18 Continued
> sig: 6 Aborted
> after sleep: errno=4 Interrupted system call
> PID: 16382
> 
> 
> Therefore we also need to think about this aspect of SIGCONT behavior
> under debuggers.
> 
> Do we provide for the mechanism for debuggers to
> prevent execution of *SIGCONT userspace handler*?

Yeah, it's not different from any other signal.  Just squash the
signal when ptrace signal delivery trap is taken, which is completely
separate from termination of job control stop triggered by _emission_
of SIGCONT.  The two are separate.  The proposed changes don't affect
the delivery path at all.  I really can't understand what your point
is.

> And, looking at the example above, I see that on resume from stop,
> *SIGCONT userspace handler* actually doesn't run as *the first handler*
> after SIGCONT. Other pending signal's handlers may be executed before it.

Signal delivery is not FIFO.  There are some rules that the code
describes.  If you're interested, take a look at the code but in
general it would be better to avoid assuming fixed order between
signal generations and deliveries.

> How would the above example look under ptraced process? Particularly,
> this sequence:
>  <------ kill -STOP 16382
>  <------ kill -ABRT 16382
>  <------ kill -WINCH 16382
>  <------ kill -CONT 16382
> sig: 28 Window changed
> sig: 18 Continued
> sig: 6 Aborted

There's NO difference regarding signal delivery.  It stays the SAME.

Thanks.

-- 
tejun

  reply	other threads:[~2011-03-02 14:43 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-01 15:24 [RFC] Proposal for ptrace improvements Tejun Heo
2011-03-01 16:57 ` Denys Vlasenko
2011-03-01 17:09   ` Tejun Heo
2011-03-01 17:12     ` Tejun Heo
2011-03-01 17:21     ` Denys Vlasenko
2011-03-01 18:34       ` Tejun Heo
2011-03-01 23:51         ` Denys Vlasenko
2011-03-02  7:10           ` Tejun Heo
2011-03-02  5:07         ` Indan Zupancic
2011-03-02  7:44           ` Tejun Heo
2011-03-02 11:32             ` Indan Zupancic
2011-03-02 11:52               ` Denys Vlasenko
2011-03-02 14:50               ` Tejun Heo
2011-03-02 13:32             ` Oleg Nesterov
2011-03-03  0:47               ` Indan Zupancic
2011-03-03  1:30                 ` Denys Vlasenko
2011-03-03  1:55                   ` Indan Zupancic
2011-03-03  7:03                     ` Tejun Heo
2011-03-01 19:06 ` Jan Kratochvil
2011-03-01 22:14   ` Denys Vlasenko
2011-03-02  7:28     ` Tejun Heo
2011-03-02 10:58       ` Denys Vlasenko
2011-03-04 16:14     ` Jan Kratochvil
2011-03-04 16:41       ` Denys Vlasenko
2011-03-04 17:07       ` Oleg Nesterov
2011-03-04 18:12         ` Jan Kratochvil
2011-03-05  8:47           ` Tejun Heo
2011-03-01 22:59 ` Denys Vlasenko
2011-03-02  7:32   ` Tejun Heo
2011-03-02 11:02     ` Denys Vlasenko
2011-03-02 11:23       ` Tejun Heo
2011-03-03 19:26         ` Oleg Nesterov
2011-03-01 23:16 ` Denys Vlasenko
2011-03-02  7:37   ` Tejun Heo
2011-03-02 11:21     ` Denys Vlasenko
2011-03-02 11:27       ` Tejun Heo
2011-03-02 11:48         ` Denys Vlasenko
2011-03-02 14:43           ` Tejun Heo [this message]
2011-03-02 15:16             ` Denys Vlasenko
2011-03-02 15:25               ` Tejun Heo
2011-03-03 17:34 ` Oleg Nesterov
2011-03-03 20:22   ` Oleg Nesterov
2011-03-04  8:23     ` Tejun Heo
2011-03-04 18:16       ` Oleg Nesterov
2011-03-05  8:33         ` Tejun Heo
2011-03-04 13:01     ` Denys Vlasenko
2011-03-04 13:41       ` Tejun Heo
2011-03-04 13:59         ` Denys Vlasenko
2011-03-04 14:07           ` Tejun Heo
2011-03-04 14:31             ` Denys Vlasenko
2011-03-04 14:40               ` Tejun Heo
2011-03-04 17:05                 ` Denys Vlasenko
2011-03-04 17:12                   ` Linus Torvalds
2011-03-04 18:59                     ` Denys Vlasenko
2011-03-04 19:24                       ` Linus Torvalds
2011-03-04 16:13               ` Oleg Nesterov
2011-03-04 16:30                 ` Oleg Nesterov
2011-03-04  8:44   ` Tejun Heo
2011-03-04 16:01     ` Oleg Nesterov
2011-03-04 16:15       ` Tejun Heo
2011-03-04 16:26         ` Oleg Nesterov
2011-03-07 15:08 ` PTRACE_SEIZE/INTERRUPT: " Oleg Nesterov
2011-03-09  9:41   ` Tejun Heo
2011-03-09 17:30     ` Oleg Nesterov
2011-03-07 20:43 ` Roland McGrath
2011-03-09 10:28   ` Tejun Heo
2011-03-10 18:33     ` Steven Rostedt
2011-03-11  8:13       ` Tejun Heo
2011-03-11  8:22       ` Ingo Molnar
2011-03-11  9:35         ` Srikar Dronamraju
2011-03-11  9:43           ` Ingo Molnar
2011-03-14  1:03     ` Frank Ch. Eigler
2011-03-10 15:55   ` Steven Rostedt

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=20110302144322.GJ3319@htj.dyndns.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=roland@redhat.com \
    --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.