On s390, recordmcount.pl is looking for "bcrl 0,<xxx>" instructions in
the objdump -d outpout. However since binutils 2.37, objdump -d
display "jgnop <xxx>" for the same instruction. Update the
mcount_regex so that it accepts both.
Signed-off-by: Jerome Marchand <[email protected]>
---
scripts/recordmcount.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 7d631aaa0ae1..52a000b057a5 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -219,7 +219,7 @@ if ($arch eq "x86_64") {
} elsif ($arch eq "s390" && $bits == 64) {
if ($cc =~ /-DCC_USING_HOTPATCH/) {
- $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*brcl\\s*0,[0-9a-f]+ <([^\+]*)>\$";
+ $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*(bcrl\\s*0,|jgnop\\s*)[0-9a-f]+ <([^\+]*)>\$";
$mcount_adjust = 0;
}
$alignment = 8;
--
2.31.1
On Fri, 10 Dec 2021, Jerome Marchand wrote:
> On s390, recordmcount.pl is looking for "bcrl 0,<xxx>" instructions in
> the objdump -d outpout. However since binutils 2.37, objdump -d
> display "jgnop <xxx>" for the same instruction. Update the
> mcount_regex so that it accepts both.
>
> Signed-off-by: Jerome Marchand <[email protected]>
Yes, we ran into exactly this issue too...
> ---
> scripts/recordmcount.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 7d631aaa0ae1..52a000b057a5 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -219,7 +219,7 @@ if ($arch eq "x86_64") {
>
> } elsif ($arch eq "s390" && $bits == 64) {
> if ($cc =~ /-DCC_USING_HOTPATCH/) {
> - $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*brcl\\s*0,[0-9a-f]+ <([^\+]*)>\$";
> + $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*(bcrl\\s*0,|jgnop\\s*)[0-9a-f]+ <([^\+]*)>\$";
> $mcount_adjust = 0;
> }
> $alignment = 8;
...and we have exactly the same fix in SLES. I haven't got to submit it
to upstream yet :(, many thanks for doing it.
So at least
Reviewed-by: Miroslav Benes <[email protected]>
M
On Fri, Dec 10, 2021 at 10:57:00AM +0100, Miroslav Benes wrote:
> On Fri, 10 Dec 2021, Jerome Marchand wrote:
>
> > On s390, recordmcount.pl is looking for "bcrl 0,<xxx>" instructions in
> > the objdump -d outpout. However since binutils 2.37, objdump -d
> > display "jgnop <xxx>" for the same instruction. Update the
> > mcount_regex so that it accepts both.
> >
> > Signed-off-by: Jerome Marchand <[email protected]>
>
> Yes, we ran into exactly this issue too...
>
> > ---
> > scripts/recordmcount.pl | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> > index 7d631aaa0ae1..52a000b057a5 100755
> > --- a/scripts/recordmcount.pl
> > +++ b/scripts/recordmcount.pl
> > @@ -219,7 +219,7 @@ if ($arch eq "x86_64") {
> >
> > } elsif ($arch eq "s390" && $bits == 64) {
> > if ($cc =~ /-DCC_USING_HOTPATCH/) {
> > - $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*brcl\\s*0,[0-9a-f]+ <([^\+]*)>\$";
> > + $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*(bcrl\\s*0,|jgnop\\s*)[0-9a-f]+ <([^\+]*)>\$";
> > $mcount_adjust = 0;
> > }
> > $alignment = 8;
>
> ...and we have exactly the same fix in SLES. I haven't got to submit it
> to upstream yet :(, many thanks for doing it.
>
> So at least
>
> Reviewed-by: Miroslav Benes <[email protected]>
Just out of curiosity: am I right if I assume that both of you have
kernel sources without upstream commit
d983c89cc96a ("s390/ftrace: Add -mfentry and -mnop-mcount support")
and the commits directly preceding that one?
Otherwise I would be surprised that this would make any difference.
Applied to s390 tree + added a stable tag.
Thank you!
On Fri, 10 Dec 2021, Heiko Carstens wrote:
> On Fri, Dec 10, 2021 at 10:57:00AM +0100, Miroslav Benes wrote:
> > On Fri, 10 Dec 2021, Jerome Marchand wrote:
> >
> > > On s390, recordmcount.pl is looking for "bcrl 0,<xxx>" instructions in
> > > the objdump -d outpout. However since binutils 2.37, objdump -d
> > > display "jgnop <xxx>" for the same instruction. Update the
> > > mcount_regex so that it accepts both.
> > >
> > > Signed-off-by: Jerome Marchand <[email protected]>
> >
> > Yes, we ran into exactly this issue too...
> >
> > > ---
> > > scripts/recordmcount.pl | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> > > index 7d631aaa0ae1..52a000b057a5 100755
> > > --- a/scripts/recordmcount.pl
> > > +++ b/scripts/recordmcount.pl
> > > @@ -219,7 +219,7 @@ if ($arch eq "x86_64") {
> > >
> > > } elsif ($arch eq "s390" && $bits == 64) {
> > > if ($cc =~ /-DCC_USING_HOTPATCH/) {
> > > - $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*brcl\\s*0,[0-9a-f]+ <([^\+]*)>\$";
> > > + $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*(bcrl\\s*0,|jgnop\\s*)[0-9a-f]+ <([^\+]*)>\$";
> > > $mcount_adjust = 0;
> > > }
> > > $alignment = 8;
> >
> > ...and we have exactly the same fix in SLES. I haven't got to submit it
> > to upstream yet :(, many thanks for doing it.
> >
> > So at least
> >
> > Reviewed-by: Miroslav Benes <[email protected]>
>
> Just out of curiosity: am I right if I assume that both of you have
> kernel sources without upstream commit
> d983c89cc96a ("s390/ftrace: Add -mfentry and -mnop-mcount support")
> and the commits directly preceding that one?
Unfortunately, it was reported also on 5.3-based kernels which have the
commits.
> Otherwise I would be surprised that this would make any difference.
How come? I mean, s390 does not define HAVE_C_RECORDMCOUNT which implies
that recordmcount.pl is used (see scripts/Makefile.build and
BUILD_C_RECORDMCOUNT definition in Makefile).
> Applied to s390 tree + added a stable tag.
Thanks.
Miroslav
> > Otherwise I would be surprised that this would make any difference.
>
> How come? I mean, s390 does not define HAVE_C_RECORDMCOUNT which implies
> that recordmcount.pl is used (see scripts/Makefile.build and
> BUILD_C_RECORDMCOUNT definition in Makefile).
Ah, sorry, that was too quick.
I guess you want to say that recordmcount is not used at all and GCC
should do it. Because CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT is not set,
since FTRACE_MCOUNT_USE_CC should be set.
What a maze.
But isn't -mrecord-mcount x86_64-only option?
Miroslav
On Fri, Dec 10, 2021 at 01:31:23PM +0100, Miroslav Benes wrote:
> > > Otherwise I would be surprised that this would make any difference.
> >
> > How come? I mean, s390 does not define HAVE_C_RECORDMCOUNT which implies
> > that recordmcount.pl is used (see scripts/Makefile.build and
> > BUILD_C_RECORDMCOUNT definition in Makefile).
>
> Ah, sorry, that was too quick.
>
> I guess you want to say that recordmcount is not used at all and GCC
> should do it. Because CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT is not set,
> since FTRACE_MCOUNT_USE_CC should be set.
>
> What a maze.
>
> But isn't -mrecord-mcount x86_64-only option?
No, it is (at least) also supported on s390. IIRC that was added with
gcc9. Anyway, this is not too important since the fix makes sense
anyway.
On Fri, 10 Dec 2021 16:51:35 +0100
Heiko Carstens <[email protected]> wrote:
> On Fri, Dec 10, 2021 at 01:31:23PM +0100, Miroslav Benes wrote:
> > > > Otherwise I would be surprised that this would make any difference.
> > >
> > > How come? I mean, s390 does not define HAVE_C_RECORDMCOUNT which implies
> > > that recordmcount.pl is used (see scripts/Makefile.build and
> > > BUILD_C_RECORDMCOUNT definition in Makefile).
> >
> > Ah, sorry, that was too quick.
> >
> > I guess you want to say that recordmcount is not used at all and GCC
> > should do it. Because CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT is not set,
> > since FTRACE_MCOUNT_USE_CC should be set.
> >
> > What a maze.
> >
> > But isn't -mrecord-mcount x86_64-only option?
>
> No, it is (at least) also supported on s390. IIRC that was added with
> gcc9. Anyway, this is not too important since the fix makes sense
> anyway.
Thanks everyone for fixing this.
Acked-by: Steven Rostedt (VMware) <[email protected]>
-- Steve
On 10/12/2021 11:58, Heiko Carstens wrote:
> On Fri, Dec 10, 2021 at 10:57:00AM +0100, Miroslav Benes wrote:
>> On Fri, 10 Dec 2021, Jerome Marchand wrote:
>>
>>> On s390, recordmcount.pl is looking for "bcrl 0,<xxx>" instructions in
>>> the objdump -d outpout. However since binutils 2.37, objdump -d
>>> display "jgnop <xxx>" for the same instruction. Update the
>>> mcount_regex so that it accepts both.
>>>
>>> Signed-off-by: Jerome Marchand <[email protected]>
>>
>> Yes, we ran into exactly this issue too...
>>
>>> ---
>>> scripts/recordmcount.pl | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
>>> index 7d631aaa0ae1..52a000b057a5 100755
>>> --- a/scripts/recordmcount.pl
>>> +++ b/scripts/recordmcount.pl
>>> @@ -219,7 +219,7 @@ if ($arch eq "x86_64") {
>>>
>>> } elsif ($arch eq "s390" && $bits == 64) {
>>> if ($cc =~ /-DCC_USING_HOTPATCH/) {
>>> - $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*brcl\\s*0,[0-9a-f]+ <([^\+]*)>\$";
>>> + $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*(bcrl\\s*0,|jgnop\\s*)[0-9a-f]+ <([^\+]*)>\$";
>>> $mcount_adjust = 0;
>>> }
>>> $alignment = 8;
>>
>> ...and we have exactly the same fix in SLES. I haven't got to submit it
>> to upstream yet :(, many thanks for doing it.
>>
>> So at least
>>
>> Reviewed-by: Miroslav Benes <[email protected]>
>
> Just out of curiosity: am I right if I assume that both of you have
> kernel sources without upstream commit
> d983c89cc96a ("s390/ftrace: Add -mfentry and -mnop-mcount support")
> and the commits directly preceding that one?
That is correct. As a matter of fact, the first thing I did to fix the
issue was to backport it, but that didn't work since it requires gcc 9
and that particular kernel is build with gcc 8.5.
Jerome
>
> Otherwise I would be surprised that this would make any difference.
>
> Applied to s390 tree + added a stable tag.
>
> Thank you!
>
Hi,
On Fri, 10 Dec 2021, Jerome Marchand wrote:
> On s390, recordmcount.pl is looking for "bcrl 0,<xxx>" instructions in
> the objdump -d outpout. However since binutils 2.37, objdump -d
> display "jgnop <xxx>" for the same instruction. Update the
> mcount_regex so that it accepts both.
>
> Signed-off-by: Jerome Marchand <[email protected]>
> ---
> scripts/recordmcount.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 7d631aaa0ae1..52a000b057a5 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -219,7 +219,7 @@ if ($arch eq "x86_64") {
>
> } elsif ($arch eq "s390" && $bits == 64) {
> if ($cc =~ /-DCC_USING_HOTPATCH/) {
> - $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*brcl\\s*0,[0-9a-f]+ <([^\+]*)>\$";
> + $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*(bcrl\\s*0,|jgnop\\s*)[0-9a-f]+ <([^\+]*)>\$";
there is a typo I did not notice before. Sorry about that *sigh*.
s/bcrl/brcl/ on the whole patch.
Miroslav
> > On s390, recordmcount.pl is looking for "bcrl 0,<xxx>" instructions in
> > the objdump -d outpout. However since binutils 2.37, objdump -d
> > display "jgnop <xxx>" for the same instruction. Update the
> > mcount_regex so that it accepts both.
> >
> > Signed-off-by: Jerome Marchand <[email protected]>
> > ---
> > scripts/recordmcount.pl | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> > index 7d631aaa0ae1..52a000b057a5 100755
> > --- a/scripts/recordmcount.pl
> > +++ b/scripts/recordmcount.pl
> > @@ -219,7 +219,7 @@ if ($arch eq "x86_64") {
> >
> > } elsif ($arch eq "s390" && $bits == 64) {
> > if ($cc =~ /-DCC_USING_HOTPATCH/) {
> > - $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*brcl\\s*0,[0-9a-f]+ <([^\+]*)>\$";
> > + $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*(bcrl\\s*0,|jgnop\\s*)[0-9a-f]+ <([^\+]*)>\$";
>
> there is a typo I did not notice before. Sorry about that *sigh*.
>
> s/bcrl/brcl/ on the whole patch.
Hm.. nice. So I will schedule the patch below for next week. Actually
tested with the variants I mentioned in the commit message.
From 30b3302ae3e9fb30c4cef5c17a2a63ba6ba34195 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <[email protected]>
Date: Thu, 23 Dec 2021 17:43:14 +0100
Subject: [PATCH] recordmcount.pl: fix typo in s390 mcount regex
Commit 85bf17b28f97 ("recordmcount.pl: look for jgnop instruction as well
as bcrl on s390") added a new alternative mnemonic for the existing brcl
instruction. This is required for the combination old gcc version (pre 9.0)
and binutils since version 2.37.
However at the same time this commit introduced a typo, replacing brcl with
bcrl. As a result no mcount locations are detected anymore with old gcc
versions (pre 9.0) and binutils before version 2.37.
Fix this by using the correct mnemonic again.
Reported-by: Miroslav Benes <[email protected]>
Cc: Jerome Marchand <[email protected]>
Cc: <[email protected]>
Fixes: 85bf17b28f97 ("recordmcount.pl: look for jgnop instruction as well as bcrl on s390")
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Heiko Carstens <[email protected]>
---
scripts/recordmcount.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 52a000b057a5..3ccb2c70add4 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -219,7 +219,7 @@ if ($arch eq "x86_64") {
} elsif ($arch eq "s390" && $bits == 64) {
if ($cc =~ /-DCC_USING_HOTPATCH/) {
- $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*(bcrl\\s*0,|jgnop\\s*)[0-9a-f]+ <([^\+]*)>\$";
+ $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*c0 04 00 00 00 00\\s*(brcl\\s*0,|jgnop\\s*)[0-9a-f]+ <([^\+]*)>\$";
$mcount_adjust = 0;
}
$alignment = 8;
--
2.32.0