From: Roland McGrath <roland@redhat.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Jerome Marchand <jmarchan@redhat.com>,
Denys Vlasenko <dvlasenk@redhat.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] ptrace_untrace: use wake_up_process() instead of bogus signal_wake_up()
Date: Sun, 8 Feb 2009 19:42:55 -0800 (PST) [thread overview]
Message-ID: <20090209034255.E95A7FC330@magilla.sf.frob.com> (raw)
In-Reply-To: Oleg Nesterov's message of Monday, 9 February 2009 03:42:36 +0100 <20090209024236.GB28280@redhat.com>
> We are holding ->siglock, and task->state is TASK_TRACED. We can not do
> the "wrong" wakeup, afaics.
I guess that's true with the siglock. But you'd be wrong to think that
this sort of detailed thinking is why wake_up_process() appears *anywhere
at all* in ptrace-related code. I'm really quite sure that it's the
aforementioned (ancient) lack of detailed thinking about it that led to
wake_up_process() appearing originally--so any use of it reminds us of that
dubious past.
> Because it complicates the understanding of this code. I spent a lot
> of time trying to understand this signal_wake_up().
>
> Perhaps this is just me. But when you see the code which does something,
> it is always good to understand the reason, otherwise the code at least
> looks wrong.
I had presumed most people interpret it the way I do: it does
signal_wake_up(,1), meaning "whatever it is that works right for SIGKILL or
SIGCONT", which it seems intuitive to think is right for this case too.
You don't have to think about exactly what is always exactly right for this
case, because it makes sense to think that this is like the wakeup that
SIGCONT would do. To me, it takes much more thought to be convinced that
wake_up_process() without other considerations is correct here--because it
looks like such a scary, unconditional thing, whereas normally that is
wrapped up inside calls that handle appropriate bookkeeping--signal_wake_up()
being one of those.
Thanks,
Roland
next prev parent reply other threads:[~2009-02-09 3:43 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-08 18:47 Oleg Nesterov
2009-02-09 1:38 ` Roland McGrath
2009-02-09 2:42 ` Oleg Nesterov
2009-02-09 3:42 ` Roland McGrath [this message]
2009-02-10 21:35 ` Oleg Nesterov
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=20090209034255.E95A7FC330@magilla.sf.frob.com \
--to=roland@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=dvlasenk@redhat.com \
--cc=jmarchan@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@redhat.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 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®