Fix some PS3 build warnings reported by `make C=1'. You need to
install sparse:
git://git.kernel.org/pub/scm/devel/sparse/sparse.git
Signed-off-by: Geert Uytterhoeven <redacted>
Signed-off-by: Geoff Levand <redacted>
---
arch/powerpc/platforms/ps3/os-area.c | 4 ++--
arch/powerpc/platforms/ps3/spu.c | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
--- a/arch/powerpc/platforms/ps3/os-area.c
+++ b/arch/powerpc/platforms/ps3/os-area.c
@@ -133,7 +133,7 @@ struct saved_params {
} static saved_params;
#define dump_header(_a) _dump_header(_a, __func__, __LINE__)
-static void _dump_header(const struct os_area_header __iomem *h, const char* func,
+static void _dump_header(const struct os_area_header *h, const char* func,
int line)
{
pr_debug("%s:%d: h.magic_num: '%s'\n", func, line,@@ -151,7 +151,7 @@ static void _dump_header(const struct os
}
#define dump_params(_a) _dump_params(_a, __func__, __LINE__)
-static void _dump_params(const struct os_area_params __iomem *p, const char* func,
+static void _dump_params(const struct os_area_params *p, const char* func,
int line)
{
pr_debug("%s:%d: p.boot_flag: %u\n", func, line, p->boot_flag);
--- a/arch/powerpc/platforms/ps3/spu.c
+++ b/arch/powerpc/platforms/ps3/spu.c@@ -190,7 +190,8 @@ static int __init setup_areas(struct spu
goto fail_ioremap;
}
- spu->local_store = ioremap(spu->local_store_phys, LS_SIZE);
+ spu->local_store = (__force void *)ioremap(spu->local_store_phys,
+ LS_SIZE);
if (!spu->local_store) {
pr_debug("%s:%d: ioremap local_store failed\n",
__func__, __LINE__);
--
On Wednesday 06 June 2007, Geoff Levand wrote:
-=A0=A0=A0=A0=A0=A0=A0spu->local_store =3D ioremap(spu->local_store_phys,=
LS_SIZE);
+=A0=A0=A0=A0=A0=A0=A0spu->local_store =3D (__force void *)ioremap(spu->l=
ocal_store_phys,
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 L=
S_SIZE);
I haven't noticed this before, but it seems to be a preexisting bug:
You map the local_store as with the guarded page table bit set, which
causes a performance degradation when accessing the memory from kernel
space.
If you're lucky, your hypervisor knows this and will fix it up for
you, but I would replace the ioremap call with an
ioremap_flags(..., _PAGE_NO_CACHE); to be on the safe side.
If you want to measure the impact, I'd suggest timing a user space
read() on the mem file of a running SPU context.
Arnd <><
Arnd Bergmann wrote:
On Wednesday 06 June 2007, Geoff Levand wrote:
quoted
-=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BDspu->l=
ocal_store =3D ioremap(spu->local_store_phys, LS_SIZE);
quoted
+=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BDspu->l=
ocal_store =3D (__force void *)ioremap(spu->local_store_phys,
quoted
+=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=
=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=
=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=
=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=
=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=
=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD =EF=BF=BD LS_SIZE)=
;
=20
I haven't noticed this before, but it seems to be a preexisting bug:
You map the local_store as with the guarded page table bit set, which
causes a performance degradation when accessing the memory from kernel
space.
=20
If you're lucky, your hypervisor knows this and will fix it up for
you, but I would replace the ioremap call with an
ioremap_flags(..., _PAGE_NO_CACHE); to be on the safe side.
=20
If you want to measure the impact, I'd suggest timing a user space
read() on the mem file of a running SPU context.
Hi Arnd,
I asked Noguchi-san to check the performance and below is his
report and test program. I'll add the change into my patch set.
-Geoff
-------- Original Message --------
Subject: RE: [patch 05/18] PS3: Fix sparse warnings
Date: Thu, 7 Jun 2007 05:39:43 -0700
From: Noguchi, Masato <redacted>
To: Levand, Geoff <redacted>
<< A time to read a whole of LS by read system call >>
not patched: avg. 21053.7800 tick ( 263.831830 microseconds )
patched: avg. 20809.2412 tick ( 260.767434 microseconds )
about 1% faster.=20
I think it's a valid difference. (not a measurement error.)
FYI,=20
The attached file is source code to measure it.
I run it 10000 times and calc an average.
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/mman.h>
#include <sys/wait.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdio.h>
#include <signal.h>
#include <string.h>
#include <stdint.h>
#include <stdlib.h>
#include <pthread.h>
#define __NR_spe_run 278
#define __NR_spe_create 279
#define LS_SIZE 0x40000
#define SPENODE "/spu/stoplooptest"
#define MFTB(RA) __asm__ volatile("mftb %0":"=3Dr"(RA))
long long do_test(void)
{
int spefd =3D -1, lsfd =3D -1;
int npc, status;
long long ret =3D -1;
char buf[LS_SIZE];
int n;
uint32_t t1, t2;
/* create context */
spefd =3D syscall(__NR_spe_create, SPENODE, 0,
S_IRUSR | S_IWUSR | S_IXUSR);
if (spefd < 0) goto out;
/* run once to assign physical spe */
npc =3D 0;
syscall(__NR_spe_run, spefd, &npc, &status);
/* get /mem file descriptor */
lsfd =3D open(SPENODE "/mem", O_RDWR,
S_IRUSR | S_IWUSR);
if (lsfd < 0) goto out;
/* read mem */
MFTB(t1);
if (read(lsfd, buf, LS_SIZE) !=3D LS_SIZE) {
goto out;
}
MFTB(t2);
ret =3D t2 - t1;
out:
if ( lsfd >=3D 0 ) close(lsfd);
if ( spefd >=3D 0 ) close(spefd);
return ret;
}
int main(int argc, char *argv[])
{
long long r;
r =3D do_test();
printf("%lld\n", r);
return 0;
}
On 2007/06/07, at 23:34, Geoff Levand wrote:
Arnd Bergmann wrote:
quoted
On Wednesday 06 June 2007, Geoff Levand wrote:
quoted
-╴╴╴╴╴╴╴spu->local_store = ioremap(spu->local_store_phys, LS_SIZE);
+╴╴╴╴╴╴╴spu->local_store = (__force void
*)ioremap(spu->local_store_phys,
+╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴ ╴ LS_SIZE);
I haven't noticed this before, but it seems to be a preexisting bug:
You map the local_store as with the guarded page table bit set, which
causes a performance degradation when accessing the memory from kernel
space.
If you're lucky, your hypervisor knows this and will fix it up for
you, but I would replace the ioremap call with an
ioremap_flags(..., _PAGE_NO_CACHE); to be on the safe side.
If you want to measure the impact, I'd suggest timing a user space
read() on the mem file of a running SPU context.
Hi Arnd,
I asked Noguchi-san to check the performance and below is his
report and test program. I'll add the change into my patch set.
-Geoff
-------- Original Message --------
Subject: RE: [patch 05/18] PS3: Fix sparse warnings
Date: Thu, 7 Jun 2007 05:39:43 -0700
From: Noguchi, Masato <redacted>
To: Levand, Geoff <redacted>
<< A time to read a whole of LS by read system call >>
not patched: avg. 21053.7800 tick ( 263.831830 microseconds )
patched: avg. 20809.2412 tick ( 260.767434 microseconds )
about 1% faster.
I think it's a valid difference. (not a measurement error.)
Let me correct above measurements and analysis.
My understanding is:
1) caching-inhibited loads are usually implemented as guarded. Thus,
guarded property will not affect load performance.
2) caching-inhibited and non-guarded sequential stores are usually
gathered (merged) in store buffer. Thus, guarded property will
affect store performance much.
Therefor, we should measure write(2) performance to evaluate the impact
of the patch.
Attached is a little modified test program based on Noguchi-san's one.
It measures write(2) performance. Buffer area is 'pre-touched' to avoid
VM overhead.
***** test results *****
before: 103903 [TB cycles] (best value out of 100 tries.)
after: 14277 [TB cycles] (best value out of 100 tries.)
-- Takao Shinohara