All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Gortmaker <paul.gortmaker@windriver.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Stephen Warren <swarren@wwwdotorg.org>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Will Deacon <will.deacon@arm.com>,
	linux-kernel@vger.kernel.org, Joseph Lo <josephl@nvidia.com>,
	linux-tegra@vger.kernel.org, Stephen Warren <swarren@nvidia.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] ARM: move body of head-common.S back to text section
Date: Wed, 3 Jul 2013 20:22:35 -0400	[thread overview]
Message-ID: <20130704002235.GL22702@windriver.com> (raw)
In-Reply-To: <20130703172001.GH24642@n2100.arm.linux.org.uk>

[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.

WARNING: multiple messages have this Message-ID (diff)
From: paul.gortmaker@windriver.com (Paul Gortmaker)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: move body of head-common.S back to text section
Date: Wed, 3 Jul 2013 20:22:35 -0400	[thread overview]
Message-ID: <20130704002235.GL22702@windriver.com> (raw)
In-Reply-To: <20130703172001.GH24642@n2100.arm.linux.org.uk>

[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.

WARNING: multiple messages have this Message-ID (diff)
From: Paul Gortmaker <paul.gortmaker@windriver.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Stephen Warren <swarren@wwwdotorg.org>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Will Deacon <will.deacon@arm.com>, <linux-kernel@vger.kernel.org>,
	Joseph Lo <josephl@nvidia.com>, <linux-tegra@vger.kernel.org>,
	Stephen Warren <swarren@nvidia.com>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] ARM: move body of head-common.S back to text section
Date: Wed, 3 Jul 2013 20:22:35 -0400	[thread overview]
Message-ID: <20130704002235.GL22702@windriver.com> (raw)
In-Reply-To: <20130703172001.GH24642@n2100.arm.linux.org.uk>

[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.

  reply	other threads:[~2013-07-04  0:22 UTC|newest]

Thread overview: 25+ 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 22:53 ` Stephen Warren
     [not found] ` <1372805629-18382-1-git-send-email-swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-07-02 23:22   ` Stephen Boyd
2013-07-02 23:22     ` Stephen Boyd
2013-07-02 23:22     ` Stephen Boyd
     [not found]     ` <20130702232259.GH11625-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2013-07-03  2:44       ` Stephen Warren
2013-07-03  2:44         ` Stephen Warren
2013-07-03  2:44         ` Stephen Warren
     [not found]         ` <51D39004.9000907-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-07-03  5:19           ` Paul Gortmaker
2013-07-03  5:19             ` Paul Gortmaker
2013-07-03  5:19             ` Paul Gortmaker
2013-07-03 10:00             ` Russell King - ARM Linux
2013-07-03 10:00               ` Russell King - ARM Linux
     [not found]               ` <20130703100044.GG24642-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2013-07-03 15:30                 ` Paul Gortmaker
2013-07-03 15:30                   ` Paul Gortmaker
2013-07-03 15:30                   ` Paul Gortmaker
     [not found]                   ` <20130703153012.GK22702-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
2013-07-03 17:20                     ` Russell King - ARM Linux
2013-07-03 17:20                       ` Russell King - ARM Linux
2013-07-03 17:20                       ` Russell King - ARM Linux
2013-07-04  0:22                       ` Paul Gortmaker [this message]
2013-07-04  0:22                         ` Paul Gortmaker
2013-07-04  0:22                         ` Paul Gortmaker
     [not found]                         ` <20130704002235.GL22702-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
2013-07-05 15:10                           ` Dave P Martin
2013-07-05 15:10                             ` Dave P Martin
2013-07-05 15:10                             ` Dave P Martin

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=20130704002235.GL22702@windriver.com \
    --to=paul.gortmaker@windriver.com \
    --cc=josephl@nvidia.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=sboyd@codeaurora.org \
    --cc=swarren@nvidia.com \
    --cc=swarren@wwwdotorg.org \
    --cc=will.deacon@arm.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 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.