From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Dale Farnsworth <dale@farnsworth.org>
Cc: linuxppc-dev@ozlabs.org, Johannes Berg <johannes@sipsolutions.net>
Subject: Re: [PATCH v2] powerpc: Add irqtrace support for 32-bit powerpc
Date: Sat, 05 Apr 2008 09:46:24 +1100 [thread overview]
Message-ID: <1207349184.10388.438.camel@pasglop> (raw)
In-Reply-To: <20080404223522.GA16958@farnsworth.org>
> Right, it's not needed on 4xx/BookE, but I didn't think it worth
> optimizing at this point, since it will split the code into 4xx/BookE
> and classic versions. Let's get it working solid first.
Yup, it's just that I spotted it while reading the code.
> That's what I originally thought and had in my first version.
> However, in the BookE case, we must save at least r3, r4, and r5.
> (See data_access: in head_fsl_booke.S.) It isn't clear what the
> rules are, and I didn't want to set a trap for when a handler is
> added that uses a fourth argument.
Ok, this definitely is worth some rework around the edges. For now, I
suppose keeping it stable will do.
> If you think it's worth it, I could test a version that saves
> r3, r4, and r5 and restores the others from ptregs.
Don't bother for now. I'll see if we can do things differently later.
> > Also, only the system call really cares about -restoring- them. Maybe
> > you could stick that in an ifdef CONFIG_TRACE_IRQFLAGS section in
> > DoSyscall pulling them back off the ptregs in the stackframe.
>
> Another optimization that I'm not convinced is worth the trouble
> for this tracing code. I'll try to take a look at it though.
> As you say below, it's scary code.
Yup. The RESTOREALL case doesn't write the result to the PT_REGS but I'm
not yet sure if that's a big issue to do it regardless or not.
Ben.
next prev parent reply other threads:[~2008-04-04 22:47 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-04-04 21:39 [PATCH v2] powerpc: Add irqtrace support for 32-bit powerpc Dale Farnsworth
2008-04-04 22:07 ` Benjamin Herrenschmidt
2008-04-04 22:35 ` Dale Farnsworth
2008-04-04 22:46 ` Benjamin Herrenschmidt [this message]
2008-04-04 23:31 ` Johannes Berg
2008-04-05 5:14 ` Benjamin Herrenschmidt
2008-04-05 5:41 ` Dale Farnsworth
2008-04-07 4:49 ` Benjamin Herrenschmidt
2008-04-07 13:10 ` Johannes Berg
2008-04-07 16:21 ` Dale Farnsworth
2008-04-07 16:16 ` Dale Farnsworth
2008-04-07 17:14 ` [PATCH v3] " Dale Farnsworth
2008-04-08 16:04 ` Johannes Berg
2008-04-08 21:36 ` Johannes Berg
2008-04-18 13:19 ` Kumar Gala
2008-04-23 4:40 ` Benjamin Herrenschmidt
2008-04-23 6:33 ` Johannes Berg
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=1207349184.10388.438.camel@pasglop \
--to=benh@kernel.crashing.org \
--cc=dale@farnsworth.org \
--cc=johannes@sipsolutions.net \
--cc=linuxppc-dev@ozlabs.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 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.