Scan any text section whose name begins with ".text." so
we will find all the functions in a kernel built with
-ffunction-sections.
Signed-off-by: Will Newton <[email protected]>
---
scripts/recordmcount.pl | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index b33446c..89461c4 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -135,9 +135,13 @@ my %text_sections = (
".spinlock.text" => 1,
".irqentry.text" => 1,
".kprobes.text" => 1,
- ".text.unlikely" => 1,
);
+sub is_valid_section
+{
+ return defined($text_sections{$1}) || $1 =~ m/^\.text\./;
+}
+
# Note: we are nice to C-programmers here, thus we skip the '||='-idiom.
$objdump = 'objdump' if (!$objdump);
$objcopy = 'objcopy' if (!$objcopy);
@@ -502,7 +506,7 @@ while (<IN>) {
$read_headers = 0;
# Only record text sections that we know are safe
- $read_function = defined($text_sections{$1});
+ $read_function = is_valid_section($1);
# print out any recorded offsets
update_funcs();
--
1.7.1
On Tue, 2012-10-30 at 14:51 +0000, Will Newton wrote:
> Scan any text section whose name begins with ".text." so
> we will find all the functions in a kernel built with
> -ffunction-sections.
A couple of things.
First, I'm very paranoid about a blanket "ok" on sections. We must
guarantee that all sections that starts with ".text" never is freed. And
if it is freed, that it must inform ftrace that it's about to free it
before it does. If we can not guarantee this, then we can't do it.
Second, most archs today do not use recordmcount.pl. They use the new
recordmcount.c file.
-- Steve
>
> Signed-off-by: Will Newton <[email protected]>
> ---
> scripts/recordmcount.pl | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index b33446c..89461c4 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -135,9 +135,13 @@ my %text_sections = (
> ".spinlock.text" => 1,
> ".irqentry.text" => 1,
> ".kprobes.text" => 1,
> - ".text.unlikely" => 1,
> );
>
> +sub is_valid_section
> +{
> + return defined($text_sections{$1}) || $1 =~ m/^\.text\./;
> +}
> +
> # Note: we are nice to C-programmers here, thus we skip the '||='-idiom.
> $objdump = 'objdump' if (!$objdump);
> $objcopy = 'objcopy' if (!$objcopy);
> @@ -502,7 +506,7 @@ while (<IN>) {
> $read_headers = 0;
>
> # Only record text sections that we know are safe
> - $read_function = defined($text_sections{$1});
> + $read_function = is_valid_section($1);
> # print out any recorded offsets
> update_funcs();
>
On Tue, Oct 30, 2012 at 3:05 PM, Steven Rostedt <[email protected]> wrote:
> On Tue, 2012-10-30 at 14:51 +0000, Will Newton wrote:
>> Scan any text section whose name begins with ".text." so
>> we will find all the functions in a kernel built with
>> -ffunction-sections.
>
> A couple of things.
>
> First, I'm very paranoid about a blanket "ok" on sections. We must
> guarantee that all sections that starts with ".text" never is freed. And
> if it is freed, that it must inform ftrace that it's about to free it
> before it does. If we can not guarantee this, then we can't do it.
I share that concern, however it looks like it should be safe. The two
architectures in tree that enable -ffunction-sections by default are
parisc and score and they both use a .text.* wildcard in their linker
script. If the support for building with --gc-sections is ever
completed then that would require -ffunction-sections and a similar
wildcard to be applied to section names.
> Second, most archs today do not use recordmcount.pl. They use the new
> recordmcount.c file.
Ok, thanks for letting me know. Is the plan to switch over to
recordmcount.c exclusively at some point?
The following architectures have some support in recordmcount.c but do
not enable it:
sh, powerpc, ia64
And the following have no support in recordmcount.c as yet:
blackfin, microblaze
On Tue, 2012-10-30 at 15:35 +0000, Will Newton wrote:
> On Tue, Oct 30, 2012 at 3:05 PM, Steven Rostedt <[email protected]> wrote:
> > On Tue, 2012-10-30 at 14:51 +0000, Will Newton wrote:
> >> Scan any text section whose name begins with ".text." so
> >> we will find all the functions in a kernel built with
> >> -ffunction-sections.
> >
> > A couple of things.
> >
> > First, I'm very paranoid about a blanket "ok" on sections. We must
> > guarantee that all sections that starts with ".text" never is freed. And
> > if it is freed, that it must inform ftrace that it's about to free it
> > before it does. If we can not guarantee this, then we can't do it.
>
> I share that concern, however it looks like it should be safe.
Can we confirm that it is safe. I'm not too confident with the words
"looks like" ;-)
> The two
> architectures in tree that enable -ffunction-sections by default are
> parisc and score and they both use a .text.* wildcard in their linker
> script. If the support for building with --gc-sections is ever
> completed then that would require -ffunction-sections and a similar
> wildcard to be applied to section names.
We need to make a way to guarantee any section that is added as ".text"
is never removed (freed), except for modules, which already have a way
to notify ftrace that its removing its text.
>
> > Second, most archs today do not use recordmcount.pl. They use the new
> > recordmcount.c file.
>
> Ok, thanks for letting me know. Is the plan to switch over to
> recordmcount.c exclusively at some point?
When all the archs convert.
>
> The following architectures have some support in recordmcount.c but do
> not enable it:
>
> sh, powerpc, ia64
Hmm, I have a powerpc, I should get that fixed.
>
> And the following have no support in recordmcount.c as yet:
>
> blackfin, microblaze
It's up to the arch maintainers to do the work. If the perl script is
good enough for them then I have no problem maintaining both. But I
don't have the machines to test these archs.
-- Steve
On Tue, Oct 30, 2012 at 4:05 PM, Steven Rostedt <[email protected]> wrote:
> On Tue, 2012-10-30 at 15:35 +0000, Will Newton wrote:
>> On Tue, Oct 30, 2012 at 3:05 PM, Steven Rostedt <[email protected]> wrote:
>> > On Tue, 2012-10-30 at 14:51 +0000, Will Newton wrote:
>> >> Scan any text section whose name begins with ".text." so
>> >> we will find all the functions in a kernel built with
>> >> -ffunction-sections.
>> >
>> > A couple of things.
>> >
>> > First, I'm very paranoid about a blanket "ok" on sections. We must
>> > guarantee that all sections that starts with ".text" never is freed. And
>> > if it is freed, that it must inform ftrace that it's about to free it
>> > before it does. If we can not guarantee this, then we can't do it.
>>
>> I share that concern, however it looks like it should be safe.
>
> Can we confirm that it is safe. I'm not too confident with the words
> "looks like" ;-)
Ok, well we are only concerned with C compiled code, otherwise there
would be no calls to mcount? The only way I can think of to cause code
to be emitted in a section of this type is to use the section
attribute. A quick grep for __attribute__(__section__ or __section
does not turn up any sections that are named .text.* on any
architecture (freed or not).
Is there any other case I have missed? I also did a grep for .text.
more generally and didn't see anything that looked problematic
(although it is mainly .S files so not relevant).
>>
>> > Second, most archs today do not use recordmcount.pl. They use the new
>> > recordmcount.c file.
>>
>> Ok, thanks for letting me know. Is the plan to switch over to
>> recordmcount.c exclusively at some point?
>
> When all the archs convert.
I'll rework my patch against recordmcount.c once I have it working.
On Tue, 2012-10-30 at 17:43 +0000, Will Newton wrote:
> Ok, well we are only concerned with C compiled code, otherwise there
> would be no calls to mcount? The only way I can think of to cause code
> to be emitted in a section of this type is to use the section
> attribute. A quick grep for __attribute__(__section__ or __section
> does not turn up any sections that are named .text.* on any
> architecture (freed or not).
>
> Is there any other case I have missed? I also did a grep for .text.
> more generally and didn't see anything that looked problematic
> (although it is mainly .S files so not relevant).
If you feel confident enough, then I'm fine with it. But if any more
network cards get bricked, I'm blaming you.
;-)
>
> >>
> >> > Second, most archs today do not use recordmcount.pl. They use the new
> >> > recordmcount.c file.
> >>
> >> Ok, thanks for letting me know. Is the plan to switch over to
> >> recordmcount.c exclusively at some point?
> >
> > When all the archs convert.
>
> I'll rework my patch against recordmcount.c once I have it working.
OK, thanks.
-- Steve