sparc64: Optimized immediate value implementation build error

sparc64: Optimized immediate value implementation build error

[Mathieu Desnoyers]

Hi David,

I ran a cross-compiler on the -lttng tree
git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-lttng.git

to build the sparc64 arch, and ran across this build error :

/home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h: In function `do_fork':
/home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: asm operand 1 probably doesn't match constraints
/home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: error: impossible constraint in `asm'
/home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: 'value' might be used uninitialized in this function
make[2]: *** [kernel/fork.o] Error 1

I think it's caused by :
http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=commit;h=c76bfa90a6eb02651368a152b3646aa672d9e625

Any idea what's wrong with the inline asm ?

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: sparc64: Optimized immediate value implementation build error

[David Miller]

From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Date: Fri, 3 Oct 2008 16:10:07 -0400

> I ran a cross-compiler on the -lttng tree
> git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-lttng.git
> 
> to build the sparc64 arch, and ran across this build error :
> 
> /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h: In function `do_fork':
> /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: asm operand 1 probably doesn't match constraints
> /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: error: impossible constraint in `asm'
> /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: 'value' might be used uninitialized in this function
> make[2]: *** [kernel/fork.o] Error 1
> 
> I think it's caused by :
> http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=commit;h=c76bfa90a6eb02651368a152b3646aa672d9e625

It can't be "caused by" that commit because that's the initial
immediate value sparc64 support I wrote and the tree very much compiled
properly when I did test builds way back then.

I don't have time to look further into this right now, sorry.

Re: sparc64: Optimized immediate value implementation build error

[Mathieu Desnoyers]

* David Miller (davem@davemloft.net) wrote:
> From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Date: Fri, 3 Oct 2008 16:10:07 -0400
> 
> > I ran a cross-compiler on the -lttng tree
> > git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-lttng.git
> > 
> > to build the sparc64 arch, and ran across this build error :
> > 
> > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h: In function `do_fork':
> > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: asm operand 1 probably doesn't match constraints
> > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: error: impossible constraint in `asm'
> > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: 'value' might be used uninitialized in this function
> > make[2]: *** [kernel/fork.o] Error 1
> > 
> > I think it's caused by :
> > http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=commit;h=c76bfa90a6eb02651368a152b3646aa672d9e625
> 
> It can't be "caused by" that commit because that's the initial
> immediate value sparc64 support I wrote and the tree very much compiled
> properly when I did test builds way back then.
> 
> I don't have time to look further into this right now, sorry.

Ok, I'll dig into this. I use gcc 3.4.5, that could be my problem; it's
broken on x86_64. I'll do some testing in this direction. Also, I see
that you have used .uaword and declared "int" in your sparc64
implementation to encode pointers. Am I missing something or should it
be "long" and either .word or .xword (32 or 64 bits) ?

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: sparc64: Optimized immediate value implementation build error

[David Miller]

From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Date: Mon, 13 Oct 2008 19:37:53 -0400

> * David Miller (davem@davemloft.net) wrote:
> > From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > Date: Fri, 3 Oct 2008 16:10:07 -0400
> > 
> > > I ran a cross-compiler on the -lttng tree
> > > git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-lttng.git
> > > 
> > > to build the sparc64 arch, and ran across this build error :
> > > 
> > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h: In function `do_fork':
> > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: asm operand 1 probably doesn't match constraints
> > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: error: impossible constraint in `asm'
> > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: 'value' might be used uninitialized in this function
> > > make[2]: *** [kernel/fork.o] Error 1
> > > 
> > > I think it's caused by :
> > > http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=commit;h=c76bfa90a6eb02651368a152b3646aa672d9e625
> > 
> > It can't be "caused by" that commit because that's the initial
> > immediate value sparc64 support I wrote and the tree very much compiled
> > properly when I did test builds way back then.
> > 
> > I don't have time to look further into this right now, sorry.
> 
> Ok, I'll dig into this. I use gcc 3.4.5, that could be my problem; it's
> broken on x86_64. I'll do some testing in this direction. Also, I see
> that you have used .uaword and declared "int" in your sparc64
> implementation to encode pointers. Am I missing something or should it
> be "long" and either .word or .xword (32 or 64 bits) ?

long would be for ".uaxword", ".uaword" is 32-bit.  It means
"UnAligned WORD"

Re: sparc64: Optimized immediate value implementation build error

[Mathieu Desnoyers]

* David Miller (davem@davemloft.net) wrote:
> From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Date: Mon, 13 Oct 2008 19:37:53 -0400
> 
> > * David Miller (davem@davemloft.net) wrote:
> > > From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > > Date: Fri, 3 Oct 2008 16:10:07 -0400
> > > 
> > > > I ran a cross-compiler on the -lttng tree
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-lttng.git
> > > > 
> > > > to build the sparc64 arch, and ran across this build error :
> > > > 
> > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h: In function `do_fork':
> > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: asm operand 1 probably doesn't match constraints
> > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: error: impossible constraint in `asm'
> > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: 'value' might be used uninitialized in this function
> > > > make[2]: *** [kernel/fork.o] Error 1
> > > > 
> > > > I think it's caused by :
> > > > http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=commit;h=c76bfa90a6eb02651368a152b3646aa672d9e625
> > > 
> > > It can't be "caused by" that commit because that's the initial
> > > immediate value sparc64 support I wrote and the tree very much compiled
> > > properly when I did test builds way back then.
> > > 
> > > I don't have time to look further into this right now, sorry.
> > 
> > Ok, I'll dig into this. I use gcc 3.4.5, that could be my problem; it's
> > broken on x86_64. I'll do some testing in this direction. Also, I see
> > that you have used .uaword and declared "int" in your sparc64
> > implementation to encode pointers. Am I missing something or should it
> > be "long" and either .word or .xword (32 or 64 bits) ?
> 
> long would be for ".uaxword", ".uaword" is 32-bit.  It means
> "UnAligned WORD"
> 

Ok, it's fixed.

If someone has access to a sparc64 gcc >= 4.0.0 compiler (native or
cross) and some spare cycles, if would be interesting to build test this
with CONFIG_IMMEDIATE=y and with CONFIG_TRACEPOINTS=y and
CONFIG_MARKERS=y :

git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-lttng.git
branch : 2.6.27-lttng-0.40

Regards,

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: sparc64: Optimized immediate value implementation build error

[David Miller]

From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Date: Mon, 13 Oct 2008 22:17:04 -0400

> * David Miller (davem@davemloft.net) wrote:
> > From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > Date: Mon, 13 Oct 2008 19:37:53 -0400
> > 
> > > * David Miller (davem@davemloft.net) wrote:
> > > > From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > > > Date: Fri, 3 Oct 2008 16:10:07 -0400
> > > > 
> > > > > I ran a cross-compiler on the -lttng tree
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-lttng.git
> > > > > 
> > > > > to build the sparc64 arch, and ran across this build error :
> > > > > 
> > > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h: In function `do_fork':
> > > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: asm operand 1 probably doesn't match constraints
> > > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: error: impossible constraint in `asm'
> > > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: 'value' might be used uninitialized in this function
> > > > > make[2]: *** [kernel/fork.o] Error 1
> > > > > 
> > > > > I think it's caused by :
> > > > > http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=commit;h=c76bfa90a6eb02651368a152b3646aa672d9e625
> > > > 
> > > > It can't be "caused by" that commit because that's the initial
> > > > immediate value sparc64 support I wrote and the tree very much compiled
> > > > properly when I did test builds way back then.
> > > > 
> > > > I don't have time to look further into this right now, sorry.
> > > 
> > > Ok, I'll dig into this. I use gcc 3.4.5, that could be my problem; it's
> > > broken on x86_64. I'll do some testing in this direction. Also, I see
> > > that you have used .uaword and declared "int" in your sparc64
> > > implementation to encode pointers. Am I missing something or should it
> > > be "long" and either .word or .xword (32 or 64 bits) ?
> > 
> > long would be for ".uaxword", ".uaword" is 32-bit.  It means
> > "UnAligned WORD"
> > 
> 
> Ok, it's fixed.

How? :-)

> If someone has access to a sparc64 gcc >= 4.0.0 compiler (native or
> cross) and some spare cycles, if would be interesting to build test this
> with CONFIG_IMMEDIATE=y and with CONFIG_TRACEPOINTS=y and
> CONFIG_MARKERS=y :
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-lttng.git
> branch : 2.6.27-lttng-0.40

I just checked out 0.39 and was building it to try and reproduce your failure.
I'll pull in 0.40 next.

Re: sparc64: Optimized immediate value implementation build error

[Mathieu Desnoyers]

* David Miller (davem@davemloft.net) wrote:
> From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Date: Mon, 13 Oct 2008 22:17:04 -0400
> 
> > * David Miller (davem@davemloft.net) wrote:
> > > From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > > Date: Mon, 13 Oct 2008 19:37:53 -0400
> > > 
> > > > * David Miller (davem@davemloft.net) wrote:
> > > > > From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > > > > Date: Fri, 3 Oct 2008 16:10:07 -0400
> > > > > 
> > > > > > I ran a cross-compiler on the -lttng tree
> > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-lttng.git
> > > > > > 
> > > > > > to build the sparc64 arch, and ran across this build error :
> > > > > > 
> > > > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h: In function `do_fork':
> > > > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: asm operand 1 probably doesn't match constraints
> > > > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: error: impossible constraint in `asm'
> > > > > > /home/compudj/git/morestable/linux-2.6-lttng/include/trace/sched.h:38: warning: 'value' might be used uninitialized in this function
> > > > > > make[2]: *** [kernel/fork.o] Error 1
> > > > > > 
> > > > > > I think it's caused by :
> > > > > > http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=commit;h=c76bfa90a6eb02651368a152b3646aa672d9e625
> > > > > 
> > > > > It can't be "caused by" that commit because that's the initial
> > > > > immediate value sparc64 support I wrote and the tree very much compiled
> > > > > properly when I did test builds way back then.
> > > > > 
> > > > > I don't have time to look further into this right now, sorry.
> > > > 
> > > > Ok, I'll dig into this. I use gcc 3.4.5, that could be my problem; it's
> > > > broken on x86_64. I'll do some testing in this direction. Also, I see
> > > > that you have used .uaword and declared "int" in your sparc64
> > > > implementation to encode pointers. Am I missing something or should it
> > > > be "long" and either .word or .xword (32 or 64 bits) ?
> > > 
> > > long would be for ".uaxword", ".uaword" is 32-bit.  It means
> > > "UnAligned WORD"
> > > 
> > 
> > Ok, it's fixed.
> 
> How? :-)
> 

The int -> long uses commit 
http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=commitdiff;h=3940305f7248f269ae330982bcd6d51d5c55c2f4

Which adds a sparc asm.h to abstract the 32/64 bits "pointer" type at
the assembler level. It's inspired from x86 32/64 :

diff --git a/arch/sparc/include/asm/asm.h b/arch/sparc/include/asm/asm.h
new file mode 100644 (file)
index 0000000..2be3d10
--- /dev/null
+++ b/arch/sparc/include/asm/asm.h
@@ -0,0 +1,19 @@
+#ifndef _ASM_SPARC_ASM_H
+#define _ASM_SPARC_ASM_H
+
+#ifdef __ASSEMBLY__
+# define __ASM_FORM(x) x
+#else
+# define __ASM_FORM(x) " " #x " "
+#endif
+
+#ifdef CONFIG_SPARC32
+# define __ASM_SEL(a,b)        __ASM_FORM(a)
+#else
+# define __ASM_SEL(a,b)        __ASM_FORM(b)
+#endif
+
+#define _ASM_PTR       __ASM_SEL(.word, .xword)
+#define _ASM_UAPTR     __ASM_SEL(.uaword, .uaxword)
+
+#endif /* _ASM_SPARC_ASM_H */

I also check for the compiler version to only enable immediate values if
gcc is >= 4.0.0. I suspect that sparc64 gcc has similar issues x86_64
gcc 3.x has with base+offset addresses and "i" inline asm constraint.

This is done in commit :
http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=commit;h=3c1106cf88f849a381c0bc01ccb185a3911e6a51

But my cross-compiler for sparc64 is only gcc 3.4.5 and I did not
succeed to find the right versions of gcc/glibc and binutils working
together to cross-build with a gcc >= 4.0.0 on x86_64. So some testing
is welcome.

> > If someone has access to a sparc64 gcc >= 4.0.0 compiler (native or
> > cross) and some spare cycles, if would be interesting to build test this
> > with CONFIG_IMMEDIATE=y and with CONFIG_TRACEPOINTS=y and
> > CONFIG_MARKERS=y :
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-lttng.git
> > branch : 2.6.27-lttng-0.40
> 
> I just checked out 0.39 and was building it to try and reproduce your failure.
> I'll pull in 0.40 next.

I'm looking forward to hear about the results. Thanks !

Mathieu


-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: sparc64: Optimized immediate value implementation build error

[David Miller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=utf-8, Size: 4034 bytes --]

From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Date: Tue, 14 Oct 2008 00:47:58 -0400

> * David Miller (davem@davemloft.net) wrote:
> > I'll pull in 0.40 next.
> 
> I'm looking forward to hear about the results. Thanks !

It builds, and I made sure that USE_IMMEDIATE does get defined
by the makefile changes you made.

On the other hand, CONFIG_PSRWLOCK_LATENCY_TEST fails to build:

  CC      lib/psrwlock-latency-trace.o
lib/psrwlock-latency-trace.c: In function ‘calibrate_get_cycles’:
lib/psrwlock-latency-trace.c:60: error: implicit declaration of function ‘rdtsc_barrier’

You could use sched_clock() or similar, we do have portable
interfaces by which to do these things.  And if we don't
have something fitting exactly what is needed here, add it :-)

Also:

<stdin>:1421:2: warning: #warning syscall marker not implemented
<stdin>:1425:2: warning: #warning syscall trace not implemented

which should be fixed by the following patch:

sparc: Add sys_trace() and sys_marker() syscall table entries.

Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/arch/sparc/include/asm/unistd_32.h b/arch/sparc/include/asm/unistd_32.h
index 648643a..ef924f4 100644
--- a/arch/sparc/include/asm/unistd_32.h
+++ b/arch/sparc/include/asm/unistd_32.h
@@ -338,8 +338,10 @@
 #define __NR_dup3		320
 #define __NR_pipe2		321
 #define __NR_inotify_init1	322
+#define __NR_marker		323
+#define __NR_trace		324
 
-#define NR_SYSCALLS		323
+#define NR_SYSCALLS		325
 
 /* Sparc 32-bit only has the "setresuid32", "getresuid32" variants,
  * it never had the plain ones and there is no value to adding those
diff --git a/arch/sparc/include/asm/unistd_64.h b/arch/sparc/include/asm/unistd_64.h
index c5cc0e0..bd830d8 100644
--- a/arch/sparc/include/asm/unistd_64.h
+++ b/arch/sparc/include/asm/unistd_64.h
@@ -340,8 +340,10 @@
 #define __NR_dup3		320
 #define __NR_pipe2		321
 #define __NR_inotify_init1	322
+#define __NR_marker		323
+#define __NR_trace		324
 
-#define NR_SYSCALLS		323
+#define NR_SYSCALLS		325
 
 #ifdef __KERNEL__
 #define __ARCH_WANT_IPC_PARSE_VERSION
diff --git a/arch/sparc/kernel/systbls.S b/arch/sparc/kernel/systbls.S
index e1b9233..0a1dd3d 100644
--- a/arch/sparc/kernel/systbls.S
+++ b/arch/sparc/kernel/systbls.S
@@ -81,4 +81,4 @@ sys_call_table:
 /*305*/	.long sys_set_mempolicy, sys_kexec_load, sys_move_pages, sys_getcpu, sys_epoll_pwait
 /*310*/	.long sys_utimensat, sys_signalfd, sys_timerfd_create, sys_eventfd, sys_fallocate
 /*315*/	.long sys_timerfd_settime, sys_timerfd_gettime, sys_signalfd4, sys_eventfd2, sys_epoll_create1
-/*320*/	.long sys_dup3, sys_pipe2, sys_inotify_init1
+/*320*/	.long sys_dup3, sys_pipe2, sys_inotify_init1, sys_marker, sys_trace
diff --git a/arch/sparc64/kernel/systbls.S b/arch/sparc64/kernel/systbls.S
index 0fdbf3b..0257912 100644
--- a/arch/sparc64/kernel/systbls.S
+++ b/arch/sparc64/kernel/systbls.S
@@ -82,7 +82,7 @@ sys_call_table32:
 	.word compat_sys_set_mempolicy, compat_sys_kexec_load, compat_sys_move_pages, sys_getcpu, compat_sys_epoll_pwait
 /*310*/	.word compat_sys_utimensat, compat_sys_signalfd, sys_timerfd_create, sys_eventfd, compat_sys_fallocate
 	.word compat_sys_timerfd_settime, compat_sys_timerfd_gettime, compat_sys_signalfd4, sys_eventfd2, sys_epoll_create1
-/*320*/	.word sys_dup3, sys_pipe2, sys_inotify_init1
+/*320*/	.word sys_dup3, sys_pipe2, sys_inotify_init1, sys_marker, sys_trace
 
 #endif /* CONFIG_COMPAT */
 
@@ -156,4 +156,4 @@ sys_call_table:
 	.word sys_set_mempolicy, sys_kexec_load, sys_move_pages, sys_getcpu, sys_epoll_pwait
 /*310*/	.word sys_utimensat, sys_signalfd, sys_timerfd_create, sys_eventfd, sys_fallocate
 	.word sys_timerfd_settime, sys_timerfd_gettime, sys_signalfd4, sys_eventfd2, sys_epoll_create1
-/*320*/	.word sys_dup3, sys_pipe2, sys_inotify_init1
+/*320*/	.word sys_dup3, sys_pipe2, sys_inotify_init1, sys_marker, sys_trace
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: sparc64: Optimized immediate value implementation build error

[Mathieu Desnoyers]

* David Miller (davem@davemloft.net) wrote:
> From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Date: Tue, 14 Oct 2008 00:47:58 -0400
> 
> > * David Miller (davem@davemloft.net) wrote:
> > > I'll pull in 0.40 next.
> > 
> > I'm looking forward to hear about the results. Thanks !
> 
> It builds, and I made sure that USE_IMMEDIATE does get defined
> by the makefile changes you made.
> 
> On the other hand, CONFIG_PSRWLOCK_LATENCY_TEST fails to build:
> 
>   CC      lib/psrwlock-latency-trace.o
> lib/psrwlock-latency-trace.c: In function ‘calibrate_get_cycles’:
> lib/psrwlock-latency-trace.c:60: error: implicit declaration of function ‘rdtsc_barrier’
> 
> You could use sched_clock() or similar, we do have portable
> interfaces by which to do these things.  And if we don't
> have something fitting exactly what is needed here, add it :-)
> 

I think the %tick register we get with get_cycles() on sparc64 is what
is needed. Hopefully it's synchronized across CPUs on SMP systems ?

sched_clock() is meant to provide a clock good enough for scheduler
needs, which implies some rough edges in the way precise counting can be
capped to a max value so it does not go over the predicted cycle count
within the current jiffies period and stuff like that. Given this
latency_test code mainly aims at checking the worse-case latencies
generated by psrwlock, I prefer to use rock-solid time bases (e.g.
synchronized cycle counter) and do just disable the whole code if the
architecture does not provide a precise enough counter or a counter
which requires any kind of locking (I don't want to change the
measurements of timings of this new locking mechanism because of
seq_lock timings).

On x86_64, rdtsc_barrier() issues a synchronizing instruction (cpuid)
which serializes the instructions executed on the CPU so we do not
execute rdtsc speculatively. Is reading %tick synchronized on sparc64 or
not ? If not, just defining an empty get_cycles_barrier() macro should
be good enough. As a comparison, get_cycles() on x86_32 issues rdtsc
which is guaranteed to be a synchronizing instruction, so
rdtsc_barrier() is defined as empty.

Is there a similar %tick register on sparc32 ? I've read somewhere it's
new to sparc v8. (http://cr.yp.to/hardware/sparc.html) So I guess we
should simply disable this psrwlock latency tracer on SPARC32 ?

Probably that the best way to deal with this is to create a 

(generic code)
HAVE_GET_CYCLES
  def_bool n

(sparc, x86, powerpc... Kconfig)
config SPARC64/X86/POWERPC
  select HAVE_GET_CYCLES

And we can make CONFIG_PSRWLOCK_LATENCY_TEST depend on HAVE_GET_CYCLES.


> Also:
> 
> <stdin>:1421:2: warning: #warning syscall marker not implemented
> <stdin>:1425:2: warning: #warning syscall trace not implemented
> 
> which should be fixed by the following patch:
> 
> sparc: Add sys_trace() and sys_marker() syscall table entries.
> 

Thanks, I'll merge it :) I don't expect the userspace tracing to be in
its final form, but it's good to add such support.

Mathieu

> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> diff --git a/arch/sparc/include/asm/unistd_32.h b/arch/sparc/include/asm/unistd_32.h
> index 648643a..ef924f4 100644
> --- a/arch/sparc/include/asm/unistd_32.h
> +++ b/arch/sparc/include/asm/unistd_32.h
> @@ -338,8 +338,10 @@
>  #define __NR_dup3		320
>  #define __NR_pipe2		321
>  #define __NR_inotify_init1	322
> +#define __NR_marker		323
> +#define __NR_trace		324
>  
> -#define NR_SYSCALLS		323
> +#define NR_SYSCALLS		325
>  
>  /* Sparc 32-bit only has the "setresuid32", "getresuid32" variants,
>   * it never had the plain ones and there is no value to adding those
> diff --git a/arch/sparc/include/asm/unistd_64.h b/arch/sparc/include/asm/unistd_64.h
> index c5cc0e0..bd830d8 100644
> --- a/arch/sparc/include/asm/unistd_64.h
> +++ b/arch/sparc/include/asm/unistd_64.h
> @@ -340,8 +340,10 @@
>  #define __NR_dup3		320
>  #define __NR_pipe2		321
>  #define __NR_inotify_init1	322
> +#define __NR_marker		323
> +#define __NR_trace		324
>  
> -#define NR_SYSCALLS		323
> +#define NR_SYSCALLS		325
>  
>  #ifdef __KERNEL__
>  #define __ARCH_WANT_IPC_PARSE_VERSION
> diff --git a/arch/sparc/kernel/systbls.S b/arch/sparc/kernel/systbls.S
> index e1b9233..0a1dd3d 100644
> --- a/arch/sparc/kernel/systbls.S
> +++ b/arch/sparc/kernel/systbls.S
> @@ -81,4 +81,4 @@ sys_call_table:
>  /*305*/	.long sys_set_mempolicy, sys_kexec_load, sys_move_pages, sys_getcpu, sys_epoll_pwait
>  /*310*/	.long sys_utimensat, sys_signalfd, sys_timerfd_create, sys_eventfd, sys_fallocate
>  /*315*/	.long sys_timerfd_settime, sys_timerfd_gettime, sys_signalfd4, sys_eventfd2, sys_epoll_create1
> -/*320*/	.long sys_dup3, sys_pipe2, sys_inotify_init1
> +/*320*/	.long sys_dup3, sys_pipe2, sys_inotify_init1, sys_marker, sys_trace
> diff --git a/arch/sparc64/kernel/systbls.S b/arch/sparc64/kernel/systbls.S
> index 0fdbf3b..0257912 100644
> --- a/arch/sparc64/kernel/systbls.S
> +++ b/arch/sparc64/kernel/systbls.S
> @@ -82,7 +82,7 @@ sys_call_table32:
>  	.word compat_sys_set_mempolicy, compat_sys_kexec_load, compat_sys_move_pages, sys_getcpu, compat_sys_epoll_pwait
>  /*310*/	.word compat_sys_utimensat, compat_sys_signalfd, sys_timerfd_create, sys_eventfd, compat_sys_fallocate
>  	.word compat_sys_timerfd_settime, compat_sys_timerfd_gettime, compat_sys_signalfd4, sys_eventfd2, sys_epoll_create1
> -/*320*/	.word sys_dup3, sys_pipe2, sys_inotify_init1
> +/*320*/	.word sys_dup3, sys_pipe2, sys_inotify_init1, sys_marker, sys_trace
>  
>  #endif /* CONFIG_COMPAT */
>  
> @@ -156,4 +156,4 @@ sys_call_table:
>  	.word sys_set_mempolicy, sys_kexec_load, sys_move_pages, sys_getcpu, sys_epoll_pwait
>  /*310*/	.word sys_utimensat, sys_signalfd, sys_timerfd_create, sys_eventfd, sys_fallocate
>  	.word sys_timerfd_settime, sys_timerfd_gettime, sys_signalfd4, sys_eventfd2, sys_epoll_create1
> -/*320*/	.word sys_dup3, sys_pipe2, sys_inotify_init1
> +/*320*/	.word sys_dup3, sys_pipe2, sys_inotify_init1, sys_marker, sys_trace
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: sparc64: Optimized immediate value implementation build error

[David Miller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=utf-8, Size: 2352 bytes --]

From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Date: Tue, 14 Oct 2008 12:08:48 -0400

> * David Miller (davem@davemloft.net) wrote:
> > On the other hand, CONFIG_PSRWLOCK_LATENCY_TEST fails to build:
> > 
> >   CC      lib/psrwlock-latency-trace.o
> > lib/psrwlock-latency-trace.c: In function ‘calibrate_get_cycles’:
> > lib/psrwlock-latency-trace.c:60: error: implicit declaration of function ‘rdtsc_barrier’
> > 
> > You could use sched_clock() or similar, we do have portable
> > interfaces by which to do these things.  And if we don't
> > have something fitting exactly what is needed here, add it :-)
> > 
> 
> I think the %tick register we get with get_cycles() on sparc64 is what
> is needed. Hopefully it's synchronized across CPUs on SMP systems ?

Yes, it is synchronized.

> On x86_64, rdtsc_barrier() issues a synchronizing instruction (cpuid)
> which serializes the instructions executed on the CPU so we do not
> execute rdtsc speculatively. Is reading %tick synchronized on sparc64 or
> not ?

It is synchronized on sparc64.

> Is there a similar %tick register on sparc32 ? I've read somewhere it's
> new to sparc v8. (http://cr.yp.to/hardware/sparc.html) So I guess we
> should simply disable this psrwlock latency tracer on SPARC32 ?

Not really.  There is only the time keeping device out in I/O space
which is very expensive to access.

This is why we don't have a sched_clock() implementation on sparc32.

> Probably that the best way to deal with this is to create a 
> 
> (generic code)
> HAVE_GET_CYCLES
>   def_bool n
> 
> (sparc, x86, powerpc... Kconfig)
> config SPARC64/X86/POWERPC
>   select HAVE_GET_CYCLES
> 
> And we can make CONFIG_PSRWLOCK_LATENCY_TEST depend on HAVE_GET_CYCLES.

Yes, something like that.

> > Also:
> > 
> > <stdin>:1421:2: warning: #warning syscall marker not implemented
> > <stdin>:1425:2: warning: #warning syscall trace not implemented
> > 
> > which should be fixed by the following patch:
> > 
> > sparc: Add sys_trace() and sys_marker() syscall table entries.
> > 
> 
> Thanks, I'll merge it :) I don't expect the userspace tracing to be in
> its final form, but it's good to add such support.

I think so too :)
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥