Re: [PATCH] recordmcount.pl: look for jgnop instruction as well as bcrl on s390

From: Jerome Marchand <jmarchan@redhat.com>
To: Heiko Carstens <hca@linux.ibm.com>, Miroslav Benes <mbenes@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] recordmcount.pl: look for jgnop instruction as well as bcrl on s390
Date: Mon, 13 Dec 2021 09:13:12 +0100	[thread overview]
Message-ID: <5c02587d-86fd-8851-dc2c-104fb2c52ee9@redhat.com> (raw)
In-Reply-To: <YbMyvJ6gz7LvoXgE@osiris>

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 <jmarchan@redhat.com>
>>
>> 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 <mbenes@suse.cz>
> 
> 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!
> 