Re: [XenPPC] [xenppc-unstable] [XEN][POWERPC] SMP/IPI/MB combined

[Jimi Xenidis <jimix@xxxxxxxxxxxxxx>]

On Nov 28, 2006, at 1:37 AM, Amos Waterland wrote:

> This will have to be reworked and broken up into individual parts for
> submission, but here is what is necessary to make 'C-a C-a C-a t' work
> properly.
>
> Jimi, when you disassemble xen-syms compiled without this patch, do  
> you
> see a bogus infinite loop in read_clocks?
looked briefly but did not notice it.

>   The compiler is not told that
> read_clocks_cpumask is volatile, so it is free to turn that loop  
> into an
> infinite loop, as my gcc 4.1.1 cross-compiler does.  I am surprised  
> that
> other Xen architectures have not seen the same problem.
Found it, cpu_relax() is supposed to contain barrier() call.
My fault but it coulda been hollis :)

diff -r cc45282daf3d xen/include/asm-powerpc/processor.h
--- a/xen/include/asm-powerpc/processor.h       Mon Nov 27 17:17:07 2006 -0500
+++ b/xen/include/asm-powerpc/processor.h       Tue Nov 28 10:19:09 2006 -0500
@@ -152,7 +152,7 @@ static inline void nop(void) {
static inline void nop(void) {
     __asm__ __volatile__ ("nop");
}
-#define cpu_relax() nop()
+#define cpu_relax() barrier()
static inline unsigned int mfpir(void)
{

this will solve the volatile issue and am pushing this now.

more below..



>  arch/powerpc/smp.c    |   18 ++++++++++--------
>  common/keyhandler.c   |    2 +-
>  include/xen/cpumask.h |    2 +-
>  3 files changed, 12 insertions(+), 10 deletions(-)
>
> diff -r 305751a5281e xen/arch/powerpc/smp.c
> --- a/xen/arch/powerpc/smp.c    Wed Nov 22 16:29:25 2006 -0500
> +++ b/xen/arch/powerpc/smp.c    Tue Nov 28 00:45:24 2006 -0500
> @@ -91,31 +91,33 @@ int on_selected_cpus(
>      int wait)
>  {
>      int t, retval = 0, nr_cpus = cpus_weight(selected);
> +    int stall = timebase_freq * 10;
>
>      spin_lock(&call_lock);
>
>      call_data.func = func;
>      call_data.info = info;
>      call_data.wait = wait;
> -    call_data.wait = 1;  /* Until we get RCU around call_data.  */
>      atomic_set(&call_data.started, 0);
>      atomic_set(&call_data.finished, 0);
>      mb();
>
>      send_IPI_mask(selected, CALL_FUNCTION_VECTOR);
>
> -    /* We always wait for an initiation ACK from remote CPU.  */
> -    for (t = 0; atomic_read(&call_data.started) != nr_cpus; t++) {
> -        if (t && t % timebase_freq == 0) {
> -            printk("IPI start stall: %d ACKS to %d SYNS\n",
> -                   atomic_read(&call_data.started), nr_cpus);
> -        }
> +    /* If told to, we wait for an initiation ACK from remote CPU.  */
> +    if (wait) {
> +       for (t = 0; atomic_read(&call_data.started) != nr_cpus; t++) {
> +           if (t > 0 && t % stall == 0) {
> +               printk("IPI start stall: %d ACKS to %d SYNS\n",
> +                      atomic_read(&call_data.started), nr_cpus);
> +           }
> +       }
you _always_ have to wait for call_data.started it means that its  
safe to reuse call_data.
>      }
>
>      /* If told to, we wait for a completion ACK from remote CPU.  */
>      if (wait) {
>          for (t = 0; atomic_read(&call_data.finished) != nr_cpus; t+ 
> +) {
> -            if (t > timebase_freq && t % timebase_freq == 0) {
> +            if (t > 0 && t % stall == 0) {
>                  printk("IPI finish stall: %d ACKS to %d SYNS\n",
>                         atomic_read(&call_data.finished), nr_cpus);
>              }
> diff -r 305751a5281e xen/common/keyhandler.c
> --- a/xen/common/keyhandler.c   Wed Nov 22 16:29:25 2006 -0500
> +++ b/xen/common/keyhandler.c   Tue Nov 28 00:12:14 2006 -0500
> @@ -193,7 +193,7 @@ static void dump_domains(unsigned char k
>      read_unlock(&domlist_lock);
>  }

_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel