From: Sergei Trofimovich <slyich@gmail.com>
To: Rob Landley <rob@landley.net>
Cc: Aboriginal Linux <aboriginal@lists.landley.net>,
linux-kernel@vger.kernel.org, davem@davemloft.net, tytso@mit.edu,
sparclinux@vger.kernel.org, Jakub Jelinek <jakub@redhat.com>
Subject: Re: Sparc-32 doesn't work in 3.1.
Date: Wed, 14 Dec 2011 17:42:25 +0000 [thread overview]
Message-ID: <20111214204225.7c8ec86c@sf.home> (raw)
In-Reply-To: <4EE80891.70604@landley.net>
[-- Attachment #1.1: Type: text/plain, Size: 3040 bytes --]
[ CCed Jakub ]
>>> Boot time fixup v1.6. 4/Mar/98 Jakub Jelinek (jj@ultra.linux.cz).
>>> Patching kernel for srmmu[Fujitsu TurboSparc]/iommu
>>> Fixup i f029ddfc doesn't refer to a valid instruction at
>>> f00de648[95eea000]
>>> halt, power off
> I put the broken image up at http://landley.net/sparc-image for the
> moment, but if you build 3.1 with the attached .config and the toolchain
> mentioned last time, it should reproduce for you. It's 100% reliable
> for me...
Nice! With this config it breaks for me on your and mine toolchains.
The offending function is ext4_kvmalloc (and similar ext4_kvzalloc).
The usual relocation in sparc looks like a pair of instructions loading
two pats of address in 2 instructions:
Like that:
> sethi %hi(ext4_fill_super), %o4 !, tmp113
> or %o4, %lo(ext4_fill_super), %o4 ! tmp113,, tmp28
In our case relocatable symbol sits in tail call, so %lo part is in "unusual"
RESTORE instruction:
> ext4_kvmalloc:
...
> sethi %hi(___i_page_kernel), %i2 !, tmp112
> call __vmalloc, 0 !
> restore %i2, %lo(___i_page_kernel), %o2 ! tmp112,,
...
David: is this code correct? Or it's a compiler bug? I am sparc32 newbie.
(C source and asm sources of function are in [1])
I think this kind of code is generated only in -Os.
So to workaround it I tried this hack:
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> ret = kmalloc(size, flags);
> if (!ret)
> ret = __vmalloc(size, flags, PAGE_KERNEL);
> +
> + asm __volatile__("nop":::"memory");
> +
> return ret;
> }
(for both ext4_kvmalloc / ext4_kvzalloc. Attached workaround as a patch.)
It forces compiler to geterate "usual" pattern for relocation.
I think of 2 solutions:
1. trying to fix sparc/boot/btfixupprep.c and arch/sparc/mm/btfixup.c
to distinct HI22 and LO10 relocations as different ones.
Right now they are merged into one 'i' type and rely on instruction heuristics to fix it.
2. Add a hack to arch/sparc/mm/btfixup.c to recognize restore instruction as well
Any others?
Hope that helps.
[1]:
void *ext4_kvmalloc(size_t size, gfp_t flags)
{
void *ret;
ret = kmalloc(size, flags);
if (!ret)
ret = __vmalloc(size, flags, PAGE_KERNEL);
return ret;
}
.global ext4_kvmalloc
.type ext4_kvmalloc, #function
.proc 0120
ext4_kvmalloc:
save %sp, -96, %sp !
mov %i0, %o0 ! size,
call __kmalloc, 0 !,
mov %i1, %o1 ! flags,
cmp %o0, 0 ! ret,
bne .LL274 !
sethi %hi(___i_page_kernel), %i2 !, tmp112
call __vmalloc, 0 !
restore %i2, %lo(___i_page_kernel), %o2 ! tmp112,,
.LL274:
jmp %i7+8
restore %g0, %o0, %o0 ! ret,
.size ext4_kvmalloc, .-ext4_kvmalloc
--
Sergei
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: pessimizing-hack.patch --]
[-- Type: text/x-patch, Size: 574 bytes --]
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 44d0c8d..570d45e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -117,6 +117,7 @@ void *ext4_kvmalloc(size_t size, gfp_t flags)
ret = kmalloc(size, flags);
if (!ret)
ret = __vmalloc(size, flags, PAGE_KERNEL);
+ mb(); /* hack to pessimize code */
return ret;
}
@@ -127,6 +128,7 @@ void *ext4_kvzalloc(size_t size, gfp_t flags)
ret = kzalloc(size, flags);
if (!ret)
ret = __vmalloc(size, flags | __GFP_ZERO, PAGE_KERNEL);
+ mb(); /* hack to pessimize code */
return ret;
}
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Sergei Trofimovich <slyich@gmail.com>
To: Rob Landley <rob@landley.net>
Cc: Aboriginal Linux <aboriginal@lists.landley.net>,
linux-kernel@vger.kernel.org, davem@davemloft.net, tytso@mit.edu,
sparclinux@vger.kernel.org, Jakub Jelinek <jakub@redhat.com>
Subject: Re: Sparc-32 doesn't work in 3.1.
Date: Wed, 14 Dec 2011 20:42:25 +0300 [thread overview]
Message-ID: <20111214204225.7c8ec86c@sf.home> (raw)
In-Reply-To: <4EE80891.70604@landley.net>
[-- Attachment #1.1: Type: text/plain, Size: 3040 bytes --]
[ CCed Jakub ]
>>> Boot time fixup v1.6. 4/Mar/98 Jakub Jelinek (jj@ultra.linux.cz).
>>> Patching kernel for srmmu[Fujitsu TurboSparc]/iommu
>>> Fixup i f029ddfc doesn't refer to a valid instruction at
>>> f00de648[95eea000]
>>> halt, power off
> I put the broken image up at http://landley.net/sparc-image for the
> moment, but if you build 3.1 with the attached .config and the toolchain
> mentioned last time, it should reproduce for you. It's 100% reliable
> for me...
Nice! With this config it breaks for me on your and mine toolchains.
The offending function is ext4_kvmalloc (and similar ext4_kvzalloc).
The usual relocation in sparc looks like a pair of instructions loading
two pats of address in 2 instructions:
Like that:
> sethi %hi(ext4_fill_super), %o4 !, tmp113
> or %o4, %lo(ext4_fill_super), %o4 ! tmp113,, tmp28
In our case relocatable symbol sits in tail call, so %lo part is in "unusual"
RESTORE instruction:
> ext4_kvmalloc:
...
> sethi %hi(___i_page_kernel), %i2 !, tmp112
> call __vmalloc, 0 !
> restore %i2, %lo(___i_page_kernel), %o2 ! tmp112,,
...
David: is this code correct? Or it's a compiler bug? I am sparc32 newbie.
(C source and asm sources of function are in [1])
I think this kind of code is generated only in -Os.
So to workaround it I tried this hack:
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> ret = kmalloc(size, flags);
> if (!ret)
> ret = __vmalloc(size, flags, PAGE_KERNEL);
> +
> + asm __volatile__("nop":::"memory");
> +
> return ret;
> }
(for both ext4_kvmalloc / ext4_kvzalloc. Attached workaround as a patch.)
It forces compiler to geterate "usual" pattern for relocation.
I think of 2 solutions:
1. trying to fix sparc/boot/btfixupprep.c and arch/sparc/mm/btfixup.c
to distinct HI22 and LO10 relocations as different ones.
Right now they are merged into one 'i' type and rely on instruction heuristics to fix it.
2. Add a hack to arch/sparc/mm/btfixup.c to recognize restore instruction as well
Any others?
Hope that helps.
[1]:
void *ext4_kvmalloc(size_t size, gfp_t flags)
{
void *ret;
ret = kmalloc(size, flags);
if (!ret)
ret = __vmalloc(size, flags, PAGE_KERNEL);
return ret;
}
.global ext4_kvmalloc
.type ext4_kvmalloc, #function
.proc 0120
ext4_kvmalloc:
save %sp, -96, %sp !
mov %i0, %o0 ! size,
call __kmalloc, 0 !,
mov %i1, %o1 ! flags,
cmp %o0, 0 ! ret,
bne .LL274 !
sethi %hi(___i_page_kernel), %i2 !, tmp112
call __vmalloc, 0 !
restore %i2, %lo(___i_page_kernel), %o2 ! tmp112,,
.LL274:
jmp %i7+8
restore %g0, %o0, %o0 ! ret,
.size ext4_kvmalloc, .-ext4_kvmalloc
--
Sergei
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: pessimizing-hack.patch --]
[-- Type: text/x-patch, Size: 574 bytes --]
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 44d0c8d..570d45e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -117,6 +117,7 @@ void *ext4_kvmalloc(size_t size, gfp_t flags)
ret = kmalloc(size, flags);
if (!ret)
ret = __vmalloc(size, flags, PAGE_KERNEL);
+ mb(); /* hack to pessimize code */
return ret;
}
@@ -127,6 +128,7 @@ void *ext4_kvzalloc(size_t size, gfp_t flags)
ret = kzalloc(size, flags);
if (!ret)
ret = __vmalloc(size, flags | __GFP_ZERO, PAGE_KERNEL);
+ mb(); /* hack to pessimize code */
return ret;
}
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
next prev parent reply other threads:[~2011-12-14 17:42 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-12 17:22 Sparc-32 doesn't work in 3.1 Rob Landley
2011-11-12 17:22 ` Rob Landley
2011-11-13 0:07 ` Rob Landley
2011-11-13 0:07 ` Rob Landley
2011-12-11 8:17 ` Sergei Trofimovich
2011-12-11 8:17 ` Sergei Trofimovich
2011-12-14 2:23 ` Rob Landley
2011-12-14 2:23 ` Rob Landley
2011-12-14 17:42 ` Sergei Trofimovich [this message]
2011-12-14 17:42 ` Sergei Trofimovich
2011-12-14 17:49 ` Eric Dumazet
2011-12-14 17:49 ` Eric Dumazet
2011-12-14 17:53 ` David Miller
2011-12-14 17:53 ` David Miller
2011-12-14 17:54 ` David Miller
2011-12-14 17:54 ` David Miller
2011-12-14 18:18 ` David Miller
2011-12-14 18:18 ` David Miller
2011-12-14 18:55 ` Sergei Trofimovich
2011-12-14 18:55 ` Sergei Trofimovich
2011-12-14 19:13 ` David Miller
2011-12-14 19:13 ` David Miller
2011-12-14 19:27 ` Rob Landley
2011-12-14 19:27 ` Rob Landley
2011-12-14 19:26 ` Rob Landley
2011-12-14 19:26 ` Rob Landley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111214204225.7c8ec86c@sf.home \
--to=slyich@gmail.com \
--cc=aboriginal@lists.landley.net \
--cc=davem@davemloft.net \
--cc=jakub@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rob@landley.net \
--cc=sparclinux@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.