LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] ftrace: Allow section alignment
@ 2008-11-07 13:12 Matt Fleming
  2008-11-07 13:18 ` Peter Zijlstra
  0 siblings, 1 reply; 7+ messages in thread
From: Matt Fleming @ 2008-11-07 13:12 UTC (permalink / raw)
  To: LKML, mingo, rostedt

[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]

Provide a means of aligning the __mcount_loc section.

Signed-off-by: Matt Fleming <matthew.fleming@imgtec.com>
---
 scripts/recordmcount.pl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 6b9fe3e..2d0bfa1 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -134,6 +134,7 @@ my $section_regex;  # Find the start of a section
 my $function_regex;    # Find the name of a function
                        #    (return offset and func name)
 my $mcount_regex;      # Find the call site to mcount (return offset)
+my $alignment;         # The .align value to use for $mcount_section

 if ($arch eq "x86") {
     if ($bits == 64) {
@@ -148,6 +149,7 @@ if ($arch eq "x86_64") {
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
     $type = ".quad";
+    $alignment = 8;

     # force flags for this arch
     $ld .= " -m elf_x86_64";
@@ -160,6 +162,7 @@ if ($arch eq "x86_64") {
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
     $type = ".long";
+    $alignment = 4;

     # force flags for this arch
     $ld .= " -m elf_i386";
@@ -288,6 +291,7 @@ sub update_funcs
            open(FILE, ">$mcount_s") || die "can't create $mcount_s\n";
            $opened = 1;
            print FILE "\t.section $mcount_section,\"a\",\@progbits\n";
+           print FILE "\t.align $alignment\n";
        }
        printf FILE "\t%s %s + %d\n", $type, $ref_func, $offsets[$i] - $offset;
     }
--
1.5.6.4

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ftrace-Allow-section-alignment.patch --]
[-- Type: text/x-diff; name=0001-ftrace-Allow-section-alignment.patch, Size: 1777 bytes --]

From 2a405e754457a676155fb86e62dac43c60ee4be0 Mon Sep 17 00:00:00 2001
From: Matt Fleming <matthew.fleming@imgtec.com>
Date: Fri, 7 Nov 2008 12:55:22 +0000
Subject: [PATCH] ftrace: Allow section alignment

Provide a means of aligning the __mcount_loc section.

Signed-off-by: Matt Fleming <matthew.fleming@imgtec.com>
---
 scripts/recordmcount.pl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 6b9fe3e..2d0bfa1 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -134,6 +134,7 @@ my $section_regex;	# Find the start of a section
 my $function_regex;	# Find the name of a function
 			#    (return offset and func name)
 my $mcount_regex;	# Find the call site to mcount (return offset)
+my $alignment;		# The .align value to use for $mcount_section
 
 if ($arch eq "x86") {
     if ($bits == 64) {
@@ -148,6 +149,7 @@ if ($arch eq "x86_64") {
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
     $type = ".quad";
+    $alignment = 8;
 
     # force flags for this arch
     $ld .= " -m elf_x86_64";
@@ -160,6 +162,7 @@ if ($arch eq "x86_64") {
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
     $type = ".long";
+    $alignment = 4;
 
     # force flags for this arch
     $ld .= " -m elf_i386";
@@ -288,6 +291,7 @@ sub update_funcs
 	    open(FILE, ">$mcount_s") || die "can't create $mcount_s\n";
 	    $opened = 1;
 	    print FILE "\t.section $mcount_section,\"a\",\@progbits\n";
+	    print FILE "\t.align $alignment\n";
 	}
 	printf FILE "\t%s %s + %d\n", $type, $ref_func, $offsets[$i] - $offset;
     }
-- 
1.5.6.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ftrace: Allow section alignment
  2008-11-07 13:12 [PATCH] ftrace: Allow section alignment Matt Fleming
@ 2008-11-07 13:18 ` Peter Zijlstra
  2008-11-07 13:25   ` Matt Fleming
  2008-11-07 13:29   ` Steven Rostedt
  0 siblings, 2 replies; 7+ messages in thread
From: Peter Zijlstra @ 2008-11-07 13:18 UTC (permalink / raw)
  To: Matt Fleming; +Cc: LKML, mingo, rostedt

On Fri, 2008-11-07 at 13:12 +0000, Matt Fleming wrote:
> Provide a means of aligning the __mcount_loc section.

Not that I object to the patch, but this changelog needs work.

Its wrong, it doesn't provide means, it plain does.
It doesn't tell us why.

> Signed-off-by: Matt Fleming <matthew.fleming@imgtec.com>
> ---
>  scripts/recordmcount.pl |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 6b9fe3e..2d0bfa1 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -134,6 +134,7 @@ my $section_regex;  # Find the start of a section
>  my $function_regex;    # Find the name of a function
>                         #    (return offset and func name)
>  my $mcount_regex;      # Find the call site to mcount (return offset)
> +my $alignment;         # The .align value to use for $mcount_section
> 
>  if ($arch eq "x86") {
>      if ($bits == 64) {
> @@ -148,6 +149,7 @@ if ($arch eq "x86_64") {
>      $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
>      $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
>      $type = ".quad";
> +    $alignment = 8;
> 
>      # force flags for this arch
>      $ld .= " -m elf_x86_64";
> @@ -160,6 +162,7 @@ if ($arch eq "x86_64") {
>      $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
>      $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
>      $type = ".long";
> +    $alignment = 4;
> 
>      # force flags for this arch
>      $ld .= " -m elf_i386";
> @@ -288,6 +291,7 @@ sub update_funcs
>             open(FILE, ">$mcount_s") || die "can't create $mcount_s\n";
>             $opened = 1;
>             print FILE "\t.section $mcount_section,\"a\",\@progbits\n";
> +           print FILE "\t.align $alignment\n";
>         }
>         printf FILE "\t%s %s + %d\n", $type, $ref_func, $offsets[$i] - $offset;
>      }
> --
> 1.5.6.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ftrace: Allow section alignment
  2008-11-07 13:18 ` Peter Zijlstra
@ 2008-11-07 13:25   ` Matt Fleming
  2008-11-07 13:50     ` Steven Rostedt
  2008-11-07 13:29   ` Steven Rostedt
  1 sibling, 1 reply; 7+ messages in thread
From: Matt Fleming @ 2008-11-07 13:25 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: LKML, mingo, rostedt

2008/11/7 Peter Zijlstra <peterz@infradead.org>:
> On Fri, 2008-11-07 at 13:12 +0000, Matt Fleming wrote:
>> Provide a means of aligning the __mcount_loc section.
>
> Not that I object to the patch, but this changelog needs work.
>
> Its wrong, it doesn't provide means, it plain does.
> It doesn't tell us why.
>

Would you like me to resubmit the patch with a better changelog?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ftrace: Allow section alignment
  2008-11-07 13:18 ` Peter Zijlstra
  2008-11-07 13:25   ` Matt Fleming
@ 2008-11-07 13:29   ` Steven Rostedt
  2008-11-07 13:59     ` Matt Fleming
  1 sibling, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2008-11-07 13:29 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Matt Fleming, LKML, mingo


On Fri, 7 Nov 2008, Peter Zijlstra wrote:

> On Fri, 2008-11-07 at 13:12 +0000, Matt Fleming wrote:
> > Provide a means of aligning the __mcount_loc section.
> 
> Not that I object to the patch, but this changelog needs work.
> 
> Its wrong, it doesn't provide means, it plain does.
> It doesn't tell us why.

Also, please tell us what broke. Did you have a compiler that did
not align it properly?  As long as the alignment is less than or equal to 
the word size, it is fine. If the alignment is larger than the word size, 
then there will be holes in the mcount_loc section and that will break 
ftrace. Heck, an alignment of 1 should work for everyone ;-) All the 
sections are packed together in an aligned space when it is pulled in by 
the linker. If that is not true, then the linker script needs to be fixed.

-- Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ftrace: Allow section alignment
  2008-11-07 13:25   ` Matt Fleming
@ 2008-11-07 13:50     ` Steven Rostedt
  0 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2008-11-07 13:50 UTC (permalink / raw)
  To: Matt Fleming; +Cc: Peter Zijlstra, LKML, mingo


On Fri, 7 Nov 2008, Matt Fleming wrote:

> 2008/11/7 Peter Zijlstra <peterz@infradead.org>:
> > On Fri, 2008-11-07 at 13:12 +0000, Matt Fleming wrote:
> >> Provide a means of aligning the __mcount_loc section.
> >
> > Not that I object to the patch, but this changelog needs work.
> >
> > Its wrong, it doesn't provide means, it plain does.
> > It doesn't tell us why.
> >
> 
> Would you like me to resubmit the patch with a better changelog?

Yes, because I'm not convinced that the change is needed.

-- Steve


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ftrace: Allow section alignment
  2008-11-07 13:29   ` Steven Rostedt
@ 2008-11-07 13:59     ` Matt Fleming
  2008-11-07 14:04       ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Matt Fleming @ 2008-11-07 13:59 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Peter Zijlstra, LKML, mingo

[-- Attachment #1: Type: text/plain, Size: 3356 bytes --]

2008/11/7 Steven Rostedt <rostedt@goodmis.org>:
>
> On Fri, 7 Nov 2008, Peter Zijlstra wrote:
>
>> On Fri, 2008-11-07 at 13:12 +0000, Matt Fleming wrote:
>> > Provide a means of aligning the __mcount_loc section.
>>
>> Not that I object to the patch, but this changelog needs work.
>>
>> Its wrong, it doesn't provide means, it plain does.
>> It doesn't tell us why.
>
> Also, please tell us what broke. Did you have a compiler that did
> not align it properly?  As long as the alignment is less than or equal to
> the word size, it is fine. If the alignment is larger than the word size,
> then there will be holes in the mcount_loc section and that will break
> ftrace. Heck, an alignment of 1 should work for everyone ;-) All the
> sections are packed together in an aligned space when it is pulled in by
> the linker. If that is not true, then the linker script needs to be fixed.
>

I've tried to explain the bug better in the attached patch. The issue
is not that there were gaps in __mcount_loc, rather that the addresses
in __mcount_loc were not aligned on a 4-byte boundary by the
assembler. Unaligned accesses are not supported by our architecture
and an alignment of 1-byte will likely lead to performance loss on
architectures that do support them.



>From 4161c8b2a1e5c93146fc6e9a638c018a14648c6b Mon Sep 17 00:00:00 2001
From: Matt Fleming <matthew.fleming@imgtec.com>
Date: Fri, 7 Nov 2008 13:26:25 +0000
Subject: [PATCH] ftrace: Align __mcount_loc sections

Align the __mcount_loc sections so that architectures with strict
alignment requirements need not worry about performing unaligned
accesses.

This fixes an issue where I was seeing unaligned accesses, which are not
supported on our architecture (the results of an unaligned access are
undefined).

Signed-off-by: Matt Fleming <matthew.fleming@imgtec.com>
---
 scripts/recordmcount.pl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 6b9fe3e..2d0bfa1 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -134,6 +134,7 @@ my $section_regex;  # Find the start of a section
 my $function_regex;    # Find the name of a function
                        #    (return offset and func name)
 my $mcount_regex;      # Find the call site to mcount (return offset)
+my $alignment;         # The .align value to use for $mcount_section

 if ($arch eq "x86") {
     if ($bits == 64) {
@@ -148,6 +149,7 @@ if ($arch eq "x86_64") {
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
     $type = ".quad";
+    $alignment = 8;

     # force flags for this arch
     $ld .= " -m elf_x86_64";
@@ -160,6 +162,7 @@ if ($arch eq "x86_64") {
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
     $type = ".long";
+    $alignment = 4;

     # force flags for this arch
     $ld .= " -m elf_i386";
@@ -288,6 +291,7 @@ sub update_funcs
            open(FILE, ">$mcount_s") || die "can't create $mcount_s\n";
            $opened = 1;
            print FILE "\t.section $mcount_section,\"a\",\@progbits\n";
+           print FILE "\t.align $alignment\n";
        }
        printf FILE "\t%s %s + %d\n", $type, $ref_func, $offsets[$i] - $offset;
     }
--
1.5.6.4

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ftrace-Align-__mcount_loc-sections.patch --]
[-- Type: text/x-diff; name=0001-ftrace-Align-__mcount_loc-sections.patch, Size: 2024 bytes --]

From 4161c8b2a1e5c93146fc6e9a638c018a14648c6b Mon Sep 17 00:00:00 2001
From: Matt Fleming <matthew.fleming@imgtec.com>
Date: Fri, 7 Nov 2008 13:26:25 +0000
Subject: [PATCH] ftrace: Align __mcount_loc sections

Align the __mcount_loc sections so that architectures with strict
alignment requirements need not worry about performing unaligned
accesses.

This fixes an issue where I was seeing unaligned accesses, which are not
supported on our architecture (the results of an unaligned access are
undefined).

Signed-off-by: Matt Fleming <matthew.fleming@imgtec.com>
---
 scripts/recordmcount.pl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 6b9fe3e..2d0bfa1 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -134,6 +134,7 @@ my $section_regex;	# Find the start of a section
 my $function_regex;	# Find the name of a function
 			#    (return offset and func name)
 my $mcount_regex;	# Find the call site to mcount (return offset)
+my $alignment;		# The .align value to use for $mcount_section
 
 if ($arch eq "x86") {
     if ($bits == 64) {
@@ -148,6 +149,7 @@ if ($arch eq "x86_64") {
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
     $type = ".quad";
+    $alignment = 8;
 
     # force flags for this arch
     $ld .= " -m elf_x86_64";
@@ -160,6 +162,7 @@ if ($arch eq "x86_64") {
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
     $type = ".long";
+    $alignment = 4;
 
     # force flags for this arch
     $ld .= " -m elf_i386";
@@ -288,6 +291,7 @@ sub update_funcs
 	    open(FILE, ">$mcount_s") || die "can't create $mcount_s\n";
 	    $opened = 1;
 	    print FILE "\t.section $mcount_section,\"a\",\@progbits\n";
+	    print FILE "\t.align $alignment\n";
 	}
 	printf FILE "\t%s %s + %d\n", $type, $ref_func, $offsets[$i] - $offset;
     }
-- 
1.5.6.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ftrace: Allow section alignment
  2008-11-07 13:59     ` Matt Fleming
@ 2008-11-07 14:04       ` Steven Rostedt
  0 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2008-11-07 14:04 UTC (permalink / raw)
  To: Matt Fleming; +Cc: Peter Zijlstra, LKML, mingo


On Fri, 7 Nov 2008, Matt Fleming wrote:
> 
> I've tried to explain the bug better in the attached patch. The issue
> is not that there were gaps in __mcount_loc, rather that the addresses
> in __mcount_loc were not aligned on a 4-byte boundary by the
> assembler. Unaligned accesses are not supported by our architecture
> and an alignment of 1-byte will likely lead to performance loss on
> architectures that do support them.

This is what I was looking for ;-)

Yes, I was just telling Peter on IRC, that this might just be an 
architecture request. Which I find as a legitimate reason. But because 
there was no mention of that in the change log, I would have to NACK it.


Ingo,

I'll pull this in my tree and test it first. Then I'll pass it off to you.

-- Steve


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-11-07 14:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-07 13:12 [PATCH] ftrace: Allow section alignment Matt Fleming
2008-11-07 13:18 ` Peter Zijlstra
2008-11-07 13:25   ` Matt Fleming
2008-11-07 13:50     ` Steven Rostedt
2008-11-07 13:29   ` Steven Rostedt
2008-11-07 13:59     ` Matt Fleming
2008-11-07 14:04       ` Steven Rostedt

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