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:38 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-12 17:22 Rob Landley
2011-11-13 0:07 ` Rob Landley
2011-12-11 8:17 ` Sergei Trofimovich
2011-12-14 2:23 ` Rob Landley
2011-12-14 17:42 ` Sergei Trofimovich [this message]
2011-12-14 17:49 ` Eric Dumazet
2011-12-14 17:53 ` David Miller
2011-12-14 17:54 ` David Miller
2011-12-14 18:18 ` David Miller
2011-12-14 18:55 ` Sergei Trofimovich
2011-12-14 19:13 ` David Miller
2011-12-14 19:27 ` 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome