From: Roland McGrath <roland@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: oleg@redhat.com, linux-kernel@vger.kernel.org, rjw@sisk.pl,
jan.kratochvil@redhat.com
Subject: Re: [PATCH 2/4] ptrace: remove the extra wake_up_process() from ptrace_detach()
Date: Thu, 27 Jan 2011 23:02:52 -0800 (PST) [thread overview]
Message-ID: <20110128070252.709F8183C1A@magilla.sf.frob.com> (raw)
In-Reply-To: Tejun Heo's message of Thursday, 27 January 2011 14:34:12 +0100 <20110127133412.GC24925@htj.dyndns.org>
> On Mon, Jan 17, 2011 at 02:13:40PM -0800, Roland McGrath wrote:
> > Of course I agree that the current code is wrong here. But I'm still not
> > at all clear on what practical compatibility problems it introduces. This
> > change is OK if and only if we are really making the only area clean and
> > well-defined in the same release cycle.
>
> I don't follow what you meant by the above sentence. Can you please
> clarify a bit?
I think I meant to write "the whole area" rather than "the only area".
The point I meant to make is that removing this (wrong) wake-up entirely
should only be done when we are also thoroughly cleaning up and verifying
everything about the behavior of stopping/resuming in ptrace. When we get
to a clear statement of what all the rules are and we really believe that
the new code matches those rules, then fine. When we are still in the
quagmire, no change is better than an incremental change whose subtle
ramifications we can't clearly articulate.
> It's slightly more serious than that. Calling wake_up_process()
> unconditionally can cause a quite serious failure. It wakes up a
> process in any state and there are code paths which aren't prepared to
> be woken up unless certain conditions are met.
I understand that. But I don't recall having seen anyone cite an actual
scenario in the real world where it does something problematic.
> A safer choice can be changing it to wake up only tasks in interruptible
> sleep.
For ptrace-related purposes, wake_up_state(child, TASK_TRACED|TASK_STOPPED)
ought to cover anything that it is doing now that anything could reasonably
be relying on.
> I have very difficult time imagining what real user visible
> implication it would have in negative and noticeable manner, so while
> I do agree that we should proceed carefully, I think it would be
> better to take the chance and remove the erratic behavior. We have
> release cycles and testing at multiple layers after all. If we find
> out that it breaks something in serious manner, we can always revert.
"Difficult time imagining" and ptrace subtleties unfortunately often go
together. I want to be sure we are doing something right, rather than just
being thoroughly unsure that we aren't doing something wrong.
Thanks,
Roland
next prev parent reply other threads:[~2011-01-28 7:03 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-22 11:13 [PATCHSET] ptrace,signal: cleanups and fixes Tejun Heo
2010-12-22 11:13 ` [PATCH 1/4] signal: fix SIGCONT notification code Tejun Heo
2011-01-12 20:41 ` Jan Kratochvil
2011-01-13 11:27 ` Tejun Heo
2011-01-13 14:40 ` Oleg Nesterov
2010-12-22 11:13 ` [PATCH 2/4] ptrace: remove the extra wake_up_process() from ptrace_detach() Tejun Heo
2011-01-17 22:13 ` Roland McGrath
2011-01-27 13:34 ` Tejun Heo
2011-01-28 7:02 ` Roland McGrath [this message]
2011-01-28 10:59 ` Tejun Heo
2010-12-22 11:13 ` [PATCH 3/4] signal: remove superflous try_to_freeze() loop in do_signal_stop() Tejun Heo
2011-01-17 22:10 ` Roland McGrath
2010-12-22 11:13 ` [PATCH 4/4] ptrace: kill tracehook_notify_jctl() Tejun Heo
2011-01-17 22:14 ` Roland McGrath
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=20110128070252.709F8183C1A@magilla.sf.frob.com \
--to=roland@redhat.com \
--cc=jan.kratochvil@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=rjw@sisk.pl \
--cc=tj@kernel.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®