diff mbox

Regression with 4.7.2 on sun4u

Message ID 20161021.132645.2214056203503527673.davem@davemloft.net
State RFC
Delegated to: David Miller
Headers show

Commit Message

David Miller Oct. 21, 2016, 5:26 p.m. UTC
From: Rob Gardner <rob.gardner@oracle.com>
Date: Fri, 21 Oct 2016 09:49:30 -0600

> That could be either a stray memory write or a boot time patch gone
> wrong.

It could be that we need to use non-predicting branches in the jump
label implementation.  We could be overflowing the branch displacement
range if the kernel being built is really huge.

Something like the following would fix it if true:

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jessica Clarke Oct. 21, 2016, 5:47 p.m. UTC | #1
(On phone, sorry for top-posting)
Yes, I found that. I don't think its overflowing, more negative (hence the 3ffffff2, which would be fffff88 or something like that for off). Trying with that masked appropriately. If it works I'll send a patch with appropriate BUG_ONs.

James

> On 21 Oct 2016, at 18:26, David Miller <davem@davemloft.net> wrote:
> 
> From: Rob Gardner <rob.gardner@oracle.com>
> Date: Fri, 21 Oct 2016 09:49:30 -0600
> 
>> That could be either a stray memory write or a boot time patch gone
>> wrong.
> 
> It could be that we need to use non-predicting branches in the jump
> label implementation.  We could be overflowing the branch displacement
> range if the kernel being built is really huge.
> 
> Something like the following would fix it if true:
> 
> diff --git a/arch/sparc/kernel/jump_label.c b/arch/sparc/kernel/jump_label.c
> index 59bbeff..841d98e 100644
> --- a/arch/sparc/kernel/jump_label.c
> +++ b/arch/sparc/kernel/jump_label.c
> @@ -19,13 +19,8 @@ void arch_jump_label_transform(struct jump_entry *entry,
>    if (type == JUMP_LABEL_JMP) {
>        s32 off = (s32)entry->target - (s32)entry->code;
> 
> -#ifdef CONFIG_SPARC64
> -        /* ba,pt %xcc, . + (off << 2) */
> -        val = 0x10680000 | ((u32) off >> 2);
> -#else
>        /* ba . + (off << 2) */
>        val = 0x10800000 | ((u32) off >> 2);
> -#endif
>    } else {
>        val = 0x01000000;
>    }
> 
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jessica Clarke Oct. 21, 2016, 9:52 p.m. UTC | #2
> On 21 Oct 2016, at 18:47, James Clarke <jrtc27@jrtc27.com> wrote:
>> On 21 Oct 2016, at 18:26, David Miller <davem@davemloft.net> wrote:
>> 
>> From: Rob Gardner <rob.gardner@oracle.com>
>> Date: Fri, 21 Oct 2016 09:49:30 -0600
>> 
>>> That could be either a stray memory write or a boot time patch gone
>>> wrong.
>> 
>> It could be that we need to use non-predicting branches in the jump
>> label implementation.  We could be overflowing the branch displacement
>> range if the kernel being built is really huge.
>> 
>> Something like the following would fix it if true:
>> 
>> diff --git a/arch/sparc/kernel/jump_label.c b/arch/sparc/kernel/jump_label.c
>> index 59bbeff..841d98e 100644
>> --- a/arch/sparc/kernel/jump_label.c
>> +++ b/arch/sparc/kernel/jump_label.c
>> @@ -19,13 +19,8 @@ void arch_jump_label_transform(struct jump_entry *entry,
>>   if (type == JUMP_LABEL_JMP) {
>>       s32 off = (s32)entry->target - (s32)entry->code;
>> 
>> -#ifdef CONFIG_SPARC64
>> -        /* ba,pt %xcc, . + (off << 2) */
>> -        val = 0x10680000 | ((u32) off >> 2);
>> -#else
>>       /* ba . + (off << 2) */
>>       val = 0x10800000 | ((u32) off >> 2);
>> -#endif
>>   } else {
>>       val = 0x01000000;
>>   }
>> 
> 
> (Was top-post; moved here)
> 
> Yes, I found that. I don't think its overflowing, more negative (hence the
> 3ffffff2, which would be fffff88 or something like that for off). Trying with
> that masked appropriately. If it works I'll send a patch with appropriate
> BUG_ONs.

This indeed was the case. The attached patch fixes the problem for me,
generating 0x106ffff2, which gdb can verify is sensible (of course, the
addresses have shifted slightly):

(gdb) x/10xw 0x5c9880
0x5c9880:	0x400f10d0	0x01000000	0x106ffff2	0x01000000
0x5c9890:	0x106fffc8	0x01000000	0xc611a036	0x05002c36
0x5c98a0:	0x8410a038	0x8328f030
(gdb) x/i 0x5c9888
   0x5c9888:	b  %xcc, 0x5c9850
   0x5c988c:	nop 

I also took the opportunity to correct the misleading/incorrect comments.
Please let me know if you’d like this properly submitted git-send-email style.

Regards,
James
David Miller Oct. 22, 2016, 1:07 a.m. UTC | #3
From: James Clarke <jrtc27@jrtc27.com>
Date: Fri, 21 Oct 2016 22:52:45 +0100

> This indeed was the case. The attached patch fixes the problem for me,
> generating 0x106ffff2, which gdb can verify is sensible (of course, the
> addresses have shifted slightly):

Please don't use attachments for patch submissions.

Patches must be inline so that they can be commented upon properly
using simply email quoting mechanisms.

Thank you.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/sparc/kernel/jump_label.c b/arch/sparc/kernel/jump_label.c
index 59bbeff..841d98e 100644
--- a/arch/sparc/kernel/jump_label.c
+++ b/arch/sparc/kernel/jump_label.c
@@ -19,13 +19,8 @@  void arch_jump_label_transform(struct jump_entry *entry,
 	if (type == JUMP_LABEL_JMP) {
 		s32 off = (s32)entry->target - (s32)entry->code;
 
-#ifdef CONFIG_SPARC64
-		/* ba,pt %xcc, . + (off << 2) */
-		val = 0x10680000 | ((u32) off >> 2);
-#else
 		/* ba . + (off << 2) */
 		val = 0x10800000 | ((u32) off >> 2);
-#endif
 	} else {
 		val = 0x01000000;
 	}