From: Dave.Martin@arm.com (Dave P Martin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: move body of head-common.S back to text section
Date: Fri, 5 Jul 2013 16:10:02 +0100 [thread overview]
Message-ID: <20130705151001.GF2932@localhost.localdomain> (raw)
In-Reply-To: <20130704002235.GL22702@windriver.com>
On Wed, Jul 03, 2013 at 08:22:35PM -0400, Paul Gortmaker wrote:
> [Re: [PATCH] ARM: move body of head-common.S back to text section] On 03/07/2013 (Wed 18:20) Russell King - ARM Linux wrote:
>
> > On Wed, Jul 03, 2013 at 11:30:12AM -0400, Paul Gortmaker wrote:
> > > [Re: [PATCH] ARM: move body of head-common.S back to text section] On 03/07/2013 (Wed 11:00) Russell King - ARM Linux wrote:
> > >
> > > > On Wed, Jul 03, 2013 at 01:19:07AM -0400, Paul Gortmaker wrote:
> > > > > As an aside, I'm now thinking any __INIT that implicitly rely on EOF for
> > > > > closure are nasty traps waiting to happen and it might be worthwhile to
> > > > > audit and explicitly __FINIT them before someone appends to the file...
> > > >
> > > > That hides a different kind of bug though - I hate __FINIT for exactly
> > > > that reason. Consider this:
> > >
> > > Agreed - perhaps masking that it is a ".previous" just hides the fact
> > > that it is more like a pop operation vs. an on/off operation, or per
> > > function as we have in C.
> >
> > I read the info pages, because I thought it was a pop operation too.
> > I was concerned that .section didn't push the previous section onto the
> > stack.
> >
> > However, .popsection is the pseudio-op which pops. .previous just toggles
> > the current section with the section immediately before it.
> >
> > So:
> >
> > .text
> > .data
> > .previous
> > /* this is .text */
> > .previous
> > /* this is .data */
> > .previous
> > /* this is .text */
> > .previous
> > /* this is .data */
>
> Cool -- I bet we weren't the only ones thinking it was a pop. Thanks.
>
> Does that make __FINIT less evil than we previously assumed? I think
> your example was the following pseudo-patch:
>
>
> .text
> <some text>
> + .data
> + <some data>
> __INIT
> <big hunk of init>
> __FINIT
> /* this below used to be text */
> <more stuff that was originally meant for text>
>
> Even if it is a toggle (vs. pop), the end text will now become data,
> so the no-op __FINIT with an explicit section called out just below
> it may still be the most unambiguous way to indicate what is going on.
>
> >
> > > That seems reasonable to me. I can't think of any self auditing that is
> > > reasonably simple to implement. One downside of __FINIT as a no-op vs.
> > > what it is today, is that a dangling __FINIT in a file with no other
> > > previous sections will emit a warning. But that is a small low value
> > > corner case I think.
> >
> > That warning from __FINIT will only happen if there has been no section
> > or .text or .data statement in the file at all. As soon as you have any
> > statement setting any kind of section, .previous doesn't warn.
> >
> > So:
> >
> > .text
> > ...
> > __FINIT
> >
> > produces no warning.o
>
> Yep -- we are both saying the same thing here - hence why I called it a
> small low value corner case.
Note that .previous has another important gotcha. Consider:
__INIT
/* now in .text.init */
ALT_UP(...)
/* now in .text.init */
__FINIT
/* now in .alt.smp.text! */
.previous (or macros containing a dangling .previous) shouldn't be used
unless you're absolutely certain what the previous section was.
In general:
label:
<stuff>
.previous
restores to the section which was current at label, only if there are
no section directives in <stuff>, nor anything which could contain a
section directive after macro expansion.
The same goes for the hidden, dangling .previous embedded in __FINIT
and friends.
Cheers
---Dave
prev parent reply other threads:[~2013-07-05 15:10 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-02 22:53 [PATCH] ARM: move body of head-common.S back to text section Stephen Warren
2013-07-02 23:22 ` Stephen Boyd
2013-07-03 2:44 ` Stephen Warren
2013-07-03 5:19 ` Paul Gortmaker
2013-07-03 10:00 ` Russell King - ARM Linux
2013-07-03 15:30 ` Paul Gortmaker
2013-07-03 17:20 ` Russell King - ARM Linux
2013-07-04 0:22 ` Paul Gortmaker
2013-07-05 15:10 ` Dave P Martin [this message]
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=20130705151001.GF2932@localhost.localdomain \
--to=dave.martin@arm.com \
--cc=linux-arm-kernel@lists.infradead.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).