All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: Denys Vlasenko <vda.linux@googlemail.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: [PATCH 1/1] ptrace: make sure do_wait() won't hang after PTRACE_ATTACH
Date: Thu, 24 Feb 2011 22:08:19 +0100	[thread overview]
Message-ID: <20110224210819.GB12258@redhat.com> (raw)
In-Reply-To: <20110222162454.GB31267@htj.dyndns.org>

On 02/22, Tejun Heo wrote:
>
> Okay, I think I finally caught up with the discussion (hopefully).
>
> On Mon, Feb 21, 2011 at 04:28:55PM +0100, Oleg Nesterov wrote:
> > However, I am starting to agree that TASK_TRACED looks more clean.
> >
> > What is important, I think ptrace should respect SIGNAL_STOP_STOPPED.
> > IOW, when the tracee is group-stopped (TASK_STOPPED or TASK_TRACED,
> > doesn't matter), ptrace_resume() should not wake it up, but merely
> > do set_task_state(TASK_STATE) and make it resumeable by SIGCONT.
>
> I don't think that's gonna fly.  It first is a very user-visible
> change to how ptrace_resume() works

Yes. But can't resist, this is a bit unfair ;) It was you who convinced
me we should cleanup this horror somehow, even if we break some corner
cases.

However, again, I can't argue. Perhaps this change is too radical.

In particular, if Jan thinks this is not acceptable - I'll shut up
immediately.

> and it removes a lot of debugging
> capability.

Well. I don't think it limits the current ptrace interface somehow,
but:

> As Jan's examples showed, there are things which the
> debugger does behind group stop's back and some of them are quite
> legitimate and useful things to do like running some code

Yes. This can surprise a user which runs the unmodified debugger.

> If you mix ptrace trap and group stop and then fix group stop
> notification, not only multithreaded debugging becomes quite
> cumbersome (suddenly ptracing becomes per-process thing instead of
> per-thread),

It should be, imho. Like SIGKILL, SIGSTOP/SIGCONT are not per-thread.
This is per-process thing.

> it becomes almost impossible to debug jctl behaviors.
> Jctl becomes completely intertwined with ptracing and the real parent
> would get numerous notifications during the course of debugging.

Again, I think this is a win. The real parent should know that, say,
its child becomes running after it was stopped. It does not matter
why it was CLD_CONTINUED, it was resumed and that is all.

> I think they belong to different layers and they should stack instead
> of mix.  I'll try to write up a summary for how I think it can be done
> later

OK. You know, we already spent sooooooooooooooooooooooooooooooooooooooo
much time discussing this, I have the strong desire to agree in advance
with anything new ;)

> but in short I think we just need two more PTRACE calls (one for
> combined SIGSTOPless attach + INTERRUPT

Yes, we are discussing these requests on archer,

> and the other for jctl
> monitoring)

Of course, we can add the new requests to help gdb/strace/whatever
to handle jctl. In fact I think we should in any case.

But this is "easy". In the context of this discussion, my only concern
is the current behaviour.

> and there doesn't need to be any fundamental revolt in how
> ptrace and jctl interact with each other. The current ptrace behavior
> is quirky and rough on the edges but I think the fundamentals are
> correct in that it's something which is fundamentally bound to a task
> (not task group)

Aha. see above. I feel this is not true. But, as usual, can't prove.

> That way, most current users won't notice (e.g. entering TASK_TRACED
> directly on SIGSTOP doesn't make any different to strace or gdb,

Just in case, let me repeat...  Yes, I think you are right, TASK_TRACE
looks more clean if the tracee does do_signal_stop().

Oleg.


  reply	other threads:[~2011-02-24 21:17 UTC|newest]

Thread overview: 160+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-28 15:08 [PATCHSET] ptrace,signal: group stop / ptrace updates Tejun Heo
2011-01-28 15:08 ` [PATCH 01/10] signal: fix SIGCONT notification code Tejun Heo
2011-01-28 15:08 ` [PATCH 02/10] ptrace: remove the extra wake_up_process() from ptrace_detach() Tejun Heo
2011-01-28 18:46   ` Roland McGrath
2011-01-31 10:38     ` Tejun Heo
2011-02-01 10:26       ` [PATCH] ptrace: use safer wake up on ptrace_detach() Tejun Heo
2011-02-01 13:40         ` Oleg Nesterov
2011-02-01 15:07           ` Tejun Heo
2011-02-01 19:17             ` Oleg Nesterov
2011-02-02  5:31             ` Roland McGrath
2011-02-02 10:35               ` Tejun Heo
2011-02-02  0:27         ` Andrew Morton
2011-02-02  5:33           ` Roland McGrath
2011-02-02  5:38             ` Andrew Morton
2011-02-02 10:34               ` Tejun Heo
2011-02-02 19:33                 ` Andrew Morton
2011-02-02 20:01                   ` Tejun Heo
2011-02-02 21:40             ` Oleg Nesterov
2011-02-02  5:29         ` Roland McGrath
2011-02-02  5:28       ` [PATCH 02/10] ptrace: remove the extra wake_up_process() from ptrace_detach() Roland McGrath
2011-01-28 15:08 ` [PATCH 03/10] signal: remove superflous try_to_freeze() loop in do_signal_stop() Tejun Heo
2011-01-28 18:46   ` Roland McGrath
2011-01-28 15:08 ` [PATCH 04/10] ptrace: kill tracehook_notify_jctl() Tejun Heo
2011-01-28 21:09   ` Roland McGrath
2011-01-28 15:08 ` [PATCH 05/10] ptrace: add @why to ptrace_stop() Tejun Heo
2011-01-28 18:48   ` Roland McGrath
2011-01-28 15:08 ` [PATCH 06/10] signal: fix premature completion of group stop when interfered by ptrace Tejun Heo
2011-01-28 21:22   ` Roland McGrath
2011-01-31 11:00     ` Tejun Heo
2011-02-02  5:44       ` Roland McGrath
2011-02-02 10:56         ` Tejun Heo
2011-01-28 15:08 ` [PATCH 07/10] signal: use GROUP_STOP_PENDING to stop once for a single group stop Tejun Heo
2011-01-28 15:08 ` [PATCH 08/10] ptrace: participate in group stop from ptrace_stop() iff the task is trapping for " Tejun Heo
2011-01-28 21:30   ` Roland McGrath
2011-01-31 11:26     ` Tejun Heo
2011-02-02  5:57       ` Roland McGrath
2011-02-02 10:53         ` Tejun Heo
2011-02-03 10:02           ` Tejun Heo
2011-02-01 19:36     ` Oleg Nesterov
2011-01-28 15:08 ` [PATCH 09/10] ptrace: make do_signal_stop() use ptrace_stop() if the task is being ptraced Tejun Heo
2011-01-28 15:08 ` [PATCH 10/10] ptrace: clean transitions between TASK_STOPPED and TRACED Tejun Heo
2011-02-03 20:41   ` [PATCH 0/1] (Was: ptrace: clean transitions between TASK_STOPPED and TRACED) Oleg Nesterov
2011-02-03 20:41     ` [PATCH 1/1] ptrace: make sure do_wait() won't hang after PTRACE_ATTACH Oleg Nesterov
2011-02-03 21:36       ` Roland McGrath
2011-02-03 21:44         ` Oleg Nesterov
2011-02-04 10:53           ` Tejun Heo
2011-02-04 13:04             ` Oleg Nesterov
2011-02-04 14:48               ` Tejun Heo
2011-02-04 17:06                 ` Oleg Nesterov
2011-02-05 13:39                   ` Tejun Heo
2011-02-07 13:42                     ` Oleg Nesterov
2011-02-07 14:11                       ` Tejun Heo
2011-02-07 15:37                         ` Oleg Nesterov
2011-02-07 16:31                           ` Tejun Heo
2011-02-07 17:48                             ` Oleg Nesterov
2011-02-09 14:18                               ` Tejun Heo
2011-02-09 14:21                                 ` Tejun Heo
2011-02-09 21:25                                 ` Oleg Nesterov
2011-02-13 23:01                                   ` Denys Vlasenko
2011-02-14  9:03                                     ` Jan Kratochvil
2011-02-14 11:39                                       ` Denys Vlasenko
2011-02-14 17:32                                         ` Oleg Nesterov
2011-02-14 16:01                                       ` Oleg Nesterov
2011-02-26  3:59                                       ` Pavel Machek
2011-02-14 15:51                                     ` Oleg Nesterov
2011-02-14 14:50                                   ` Tejun Heo
2011-02-14 18:53                                     ` Oleg Nesterov
2011-02-13 22:25                                 ` Denys Vlasenko
2011-02-14 15:13                                   ` Tejun Heo
2011-02-14 16:15                                     ` Oleg Nesterov
2011-02-14 16:33                                       ` Tejun Heo
2011-02-14 17:23                                         ` Oleg Nesterov
2011-02-14 17:20                                     ` Denys Vlasenko
2011-02-14 17:30                                       ` Tejun Heo
2011-02-14 17:45                                         ` Oleg Nesterov
2011-02-14 17:54                                         ` Denys Vlasenko
2011-02-21 15:16                                           ` Tejun Heo
2011-02-21 15:28                                             ` Oleg Nesterov
2011-02-21 16:11                                               ` [pseudo patch] ptrace should respect the group stop Oleg Nesterov
2011-02-22 16:24                                               ` [PATCH 1/1] ptrace: make sure do_wait() won't hang after PTRACE_ATTACH Tejun Heo
2011-02-24 21:08                                                 ` Oleg Nesterov [this message]
2011-02-25 15:45                                                   ` Tejun Heo
2011-02-25 17:42                                                     ` Roland McGrath
2011-02-28 15:23                                                     ` Oleg Nesterov
2011-02-14 17:51                                       ` Oleg Nesterov
2011-02-14 18:55                                         ` Denys Vlasenko
2011-02-14 19:01                                           ` Oleg Nesterov
2011-02-14 19:42                                             ` Denys Vlasenko
2011-02-14 20:01                                               ` Oleg Nesterov
2011-02-15 15:24                                                 ` Tejun Heo
2011-02-15 15:58                                                   ` Oleg Nesterov
2011-02-15 17:31                                                   ` Roland McGrath
2011-02-15 20:27                                                     ` Oleg Nesterov
2011-02-18 17:02                                                       ` Tejun Heo
2011-02-18 19:37                                                         ` Oleg Nesterov
2011-02-21 16:22                                                           ` Tejun Heo
2011-02-21 16:49                                                             ` Oleg Nesterov
2011-02-21 16:59                                                               ` Tejun Heo
2011-02-23 19:31                                                                 ` Oleg Nesterov
2011-02-25 15:10                                                                   ` Tejun Heo
2011-02-24 20:29                                                             ` Oleg Nesterov
2011-02-25 15:51                                                               ` Tejun Heo
2011-02-26  2:48                                                                 ` Denys Vlasenko
2011-02-28 12:56                                                                   ` Tejun Heo
2011-02-28 13:16                                                                     ` Denys Vlasenko
2011-02-28 13:29                                                                       ` Tejun Heo
2011-02-28 13:41                                                                         ` Denys Vlasenko
2011-02-28 13:53                                                                           ` Tejun Heo
2011-02-28 14:25                                                                             ` Denys Vlasenko
2011-02-28 14:39                                                                               ` Tejun Heo
2011-02-28 16:48                                                                                 ` Oleg Nesterov
2011-02-28 14:36                                                                   ` Oleg Nesterov
2011-02-16 21:51                                       ` Jan Kratochvil
2011-02-17  3:37                                         ` Denys Vlasenko
2011-02-17 19:19                                           ` Oleg Nesterov
2011-02-18 21:11                                             ` Jan Kratochvil
2011-02-19 20:16                                               ` Oleg Nesterov
2011-02-17 16:49                                         ` Oleg Nesterov
2011-02-17 18:58                                           ` Roland McGrath
2011-02-17 19:33                                             ` Oleg Nesterov
2011-02-18 21:34                                           ` Jan Kratochvil
2011-02-19 20:06                                             ` Oleg Nesterov
2011-02-20  9:40                                               ` Jan Kratochvil
2011-02-20 17:06                                                 ` Denys Vlasenko
2011-02-20 17:48                                                   ` Oleg Nesterov
2011-02-20 19:10                                                   ` Jan Kratochvil
2011-02-20 19:16                                                     ` Oleg Nesterov
2011-02-20 17:16                                                 ` Oleg Nesterov
2011-02-20 18:52                                                   ` Jan Kratochvil
2011-02-20 20:38                                                     ` Oleg Nesterov
2011-02-20 21:06                                                       ` `(T) stopped' preservation after _exit() [Re: [PATCH 1/1] ptrace: make sure do_wait() won't hang after PTRACE_ATTACH] Jan Kratochvil
2011-02-20 21:19                                                         ` Oleg Nesterov
2011-02-20 21:20                                                       ` [PATCH 1/1] ptrace: make sure do_wait() won't hang after PTRACE_ATTACH Jan Kratochvil
2011-02-21 14:23                                                         ` Oleg Nesterov
2011-02-23 16:44                                                           ` Jan Kratochvil
2011-02-14 15:31                                   ` Oleg Nesterov
2011-02-14 17:24                                     ` Denys Vlasenko
2011-02-14 17:39                                       ` Oleg Nesterov
2011-02-14 17:57                                         ` Denys Vlasenko
2011-02-14 18:00                                           ` Oleg Nesterov
2011-02-14 18:06                                             ` Oleg Nesterov
2011-02-14 18:59                                         ` Denys Vlasenko
2011-02-13 21:24                 ` Denys Vlasenko
2011-02-14 15:06                   ` Oleg Nesterov
2011-02-14 15:19                     ` Tejun Heo
2011-02-14 16:20                       ` Oleg Nesterov
2011-02-14 17:05                     ` Denys Vlasenko
2011-02-14 17:18                       ` Oleg Nesterov
2011-01-28 16:54 ` [PATCHSET] ptrace,signal: group stop / ptrace updates Ingo Molnar
2011-01-28 17:41   ` Thomas Gleixner
2011-01-28 18:04     ` Anca Emanuel
2011-01-28 18:36       ` Mathieu Desnoyers
2011-01-28 17:55   ` Oleg Nesterov
2011-01-28 18:29     ` Bash not reacting to Ctrl-C Ingo Molnar
2011-02-05 20:34       ` Oleg Nesterov
2011-02-07 13:08         ` Oleg Nesterov
2011-02-09  6:17           ` Michael Witten
2011-02-09 14:53             ` Ingo Molnar
2011-02-09 19:37               ` Michael Witten
2011-02-11 14:41           ` Pavel Machek

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=20110224210819.GB12258@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roland@redhat.com \
    --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.