linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Carlos O'Donell <carlos@systemhalted.org>
Cc: Grant Grundler <grantgrundler@gmail.com>,
	John David Anglin <dave.anglin@bell.net>,
	linux-parisc@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-arch@vger.kernel.org
Subject: Re: [parisc] double restarts on multiple signal arrivals
Date: Sun, 20 May 2012 09:40:42 +0100	[thread overview]
Message-ID: <20120520084042.GA25447@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20120518222422.GX22082@ZenIV.linux.org.uk>

	BTW, after looking through the asm glue there, I've noticed something
rather odd.  *All* architectures other than parisc have the following thing
done on syscall entry:
	* check if we have TIF_SYSCALL_TRACE in flags; bugger off to slow
path if we do.
	* there check if we have PT_PTRACED in current->ptrace and if so
do the standard song and dance with raising SIGTRAP or SIGTRAP | 0x80,
etc.

parisc does that in the opposite order - it goes to slow path if
current->ptrace & PT_PTRACED is true.  The same actions with raising
SIGTRAP, etc. are done only if TIF_SYSCALL_TRACE is there as well,
but by that point we'd already plonked a bunch of registers on the
stack and we actually stay on the slow path even in absense of SYSCALL_TRACE.

The thing is, other architectures have a good reason for looking at
SYSCALL_TRACE first and not bothering with all that fun if it's not set.
PT_PTRACED is set when the task is being traced; it does *not* imply
stopping on every syscall entry.  E.g. a perfectly normal situation is
when in gdb you've said
break foo.c:69
cont
and let the process run until it hits that breakpoint.  It has PT_PTRACED
all along.  It will be stopped when it gets a signal, including SIGTRAP
we'll get when it steps on that breakpoint.  But by that time it's very
likely to have passed through tons of syscalls without stopping on any of
them.  SYSCALL_TRACE is set *only* when we want the process to stop on the
next syscall.  That's what strace(1) and friends are using.  So it doesn't
make sense to get overhead for all processes that have PT_PTRACED;
SYSCALL_TRACE is less likely, so checking it first is an obvious approach.

Moreover, checking PT_TRACED first is not even cheaper in the common case
(i.e. when branch to .Ltracesys is not taken at all).  As it is, parisc does
        mfctl   %cr30, %r1
        LDREG   TI_TASK(%r1),%r1
        ldw     TASK_PTRACE(%r1), %r1
        bb,<,n  %r1,31,.Ltracesys
and that's actually an extra dereference compared to
        mfctl   %cr30, %r1
        LDREG   TI_FLAGS(%r1),%r1
        bb,<,n  %r1,31 - TIF_SYSCALL_TRACE,.Ltracesys
Note that tracehook_report_syscall_entry/tracehook_report_syscall_exit
are checking PT_PTRACED, so it's not like we needed to change anything
other than that spot - resulting logics will be equivalent to what we
have right now.  Looks like a fairly straightforward optimisation...
Am I missing something subtle and parisc-specific here?

  parent reply	other threads:[~2012-05-20  8:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-18 17:58 [parisc] double restarts on multiple signal arrivals Al Viro
2012-05-18 17:58 ` Al Viro
2012-05-18 18:05 ` Grant Grundler
2012-05-18 18:57   ` Al Viro
2012-05-18 18:57     ` Al Viro
2012-05-18 19:56   ` Al Viro
2012-05-18 19:56     ` Al Viro
2012-05-18 20:12     ` Grant Grundler
2012-05-18 20:15       ` Carlos O'Donell
2012-05-18 22:24         ` Al Viro
2012-05-19  1:36           ` Carlos O'Donell
2012-05-19  5:26           ` Al Viro
2012-05-19 13:37             ` Al Viro
2012-05-19 13:37               ` Al Viro
2012-05-20  8:40           ` Al Viro [this message]
2012-05-20  8:40             ` Al Viro
2012-05-20  9:04             ` Al Viro
2012-05-20  9:04               ` Al Viro
2012-05-20 18:46               ` John David Anglin
2012-05-20 18:46                 ` John David Anglin
2012-05-20 20:38                 ` Carlos O'Donell

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=20120520084042.GA25447@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=carlos@systemhalted.org \
    --cc=dave.anglin@bell.net \
    --cc=grantgrundler@gmail.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=torvalds@linux-foundation.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;
as well as URLs for NNTP newsgroup(s).