Re: sparc64: Optimized immediate value implementation build error

[Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>]

* 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