Re: macppc IPI counter

[Mark Kettenis]

 Date: Wed, 24 Jun 2015 16:11:08 +0200
 From: Martin Pieuchot m...@openbsd.org
 
 Use only one ipi counter just like other archs do.
 
 ok?

Problem is that the event counters aren't really MP safe.  By keeping
them per-CPU you circumvent any problems with that.

 Index: dev/openpic.c
 ===
 RCS file: /cvs/src/sys/arch/macppc/dev/openpic.c,v
 retrieving revision 1.81
 diff -u -p -r1.81 openpic.c
 --- dev/openpic.c 24 Jun 2015 11:58:06 -  1.81
 +++ dev/openpic.c 24 Jun 2015 13:47:34 -
 @@ -130,11 +130,9 @@ void openpic_ipi_ddb(void);
  #define IPI_VECTOR_NOP   64
  #define IPI_VECTOR_DDB   65
  
 -static struct evcount ipi_ddb[PPC_MAXPROCS];
 -static struct evcount ipi_nop[PPC_MAXPROCS];
 +static struct evcount ipi_count;
  
 -static int ipi_nopirq = IPI_VECTOR_NOP;
 -static int ipi_ddbirq = IPI_VECTOR_DDB;
 +static int ipi_irq = IPI_VECTOR_NOP;
  
  intr_send_ipi_t openpic_send_ipi;
  #endif /* MULTIPROCESSOR */
 @@ -288,11 +286,7 @@ openpic_attach(struct device *parent, st
   x |= 15  OPENPIC_PRIORITY_SHIFT;
   openpic_write(OPENPIC_IPI_VECTOR(1), x);
  
 - /* XXX - ncpus */
 - evcount_attach(ipi_nop[0], ipi_nop0, ipi_nopirq);
 - evcount_attach(ipi_nop[1], ipi_nop1, ipi_nopirq);
 - evcount_attach(ipi_ddb[0], ipi_ddb0, ipi_ddbirq);
 - evcount_attach(ipi_ddb[1], ipi_ddb1, ipi_ddbirq);
 + evcount_attach(ipi_count, ipi, ipi_irq);
  #endif
  
   /* clear all pending interrunts */
 @@ -638,16 +632,11 @@ openpic_ext_intr(void)
   return;
   }
  #ifdef MULTIPROCESSOR
 - if (irq == IPI_VECTOR_NOP) {
 - ipi_nop[ci-ci_cpuid].ec_count++;
 + if (irq == IPI_VECTOR_NOP || irq == IPI_VECTOR_DDB) {
 + ipi_count.ec_count++;
   openpic_eoi(ci-ci_cpuid);
 - irq = openpic_read_irq(ci-ci_cpuid);
 - continue;
 - }
 - if (irq == IPI_VECTOR_DDB) {
 - ipi_ddb[ci-ci_cpuid].ec_count++;
 - openpic_eoi(ci-ci_cpuid);
 - openpic_ipi_ddb();
 + if (irq == IPI_VECTOR_DDB)
 + openpic_ipi_ddb();
   irq = openpic_read_irq(ci-ci_cpuid);
   continue;
   }
 @@ -758,7 +747,6 @@ openpic_send_ipi(struct cpu_info *ci, in
  void
  openpic_ipi_ddb(void)
  {
 - DPRINTF(ipi_ddb() called\n);
  #ifdef DDB
   Debugger();
  #endif
 
 

Re: macppc IPI counter

[Martin Pieuchot]

On 24/06/15(Wed) 16:45, Mark Kettenis wrote:
  Date: Wed, 24 Jun 2015 16:11:08 +0200
  From: Martin Pieuchot m...@openbsd.org
  
  Use only one ipi counter just like other archs do.
  
  ok?
 
 Problem is that the event counters aren't really MP safe.  By keeping
 them per-CPU you circumvent any problems with that.

Do you suggest that I add another 4 counters for my CPUs #2 and #3?

Honestly the trashing is much more noticeable on the clock counter which
also not protected.  But as you know counters are 64bit long so we can't
easily use atomic operations like on sparc64.  Plus if I'm not mistaken
i386 and amd64 are already doing that...

If you have a suggestion for a correct fix, I'm interested, but I
believe this should be done anyway.

macppc IPI counter

[Martin Pieuchot]

Use only one ipi counter just like other archs do.

ok?

Index: dev/openpic.c
===
RCS file: /cvs/src/sys/arch/macppc/dev/openpic.c,v
retrieving revision 1.81
diff -u -p -r1.81 openpic.c
--- dev/openpic.c   24 Jun 2015 11:58:06 -  1.81
+++ dev/openpic.c   24 Jun 2015 13:47:34 -
@@ -130,11 +130,9 @@ void   openpic_ipi_ddb(void);
 #define IPI_VECTOR_NOP 64
 #define IPI_VECTOR_DDB 65
 
-static struct evcount ipi_ddb[PPC_MAXPROCS];
-static struct evcount ipi_nop[PPC_MAXPROCS];
+static struct evcount ipi_count;
 
-static int ipi_nopirq = IPI_VECTOR_NOP;
-static int ipi_ddbirq = IPI_VECTOR_DDB;
+static int ipi_irq = IPI_VECTOR_NOP;
 
 intr_send_ipi_t openpic_send_ipi;
 #endif /* MULTIPROCESSOR */
@@ -288,11 +286,7 @@ openpic_attach(struct device *parent, st
x |= 15  OPENPIC_PRIORITY_SHIFT;
openpic_write(OPENPIC_IPI_VECTOR(1), x);
 
-   /* XXX - ncpus */
-   evcount_attach(ipi_nop[0], ipi_nop0, ipi_nopirq);
-   evcount_attach(ipi_nop[1], ipi_nop1, ipi_nopirq);
-   evcount_attach(ipi_ddb[0], ipi_ddb0, ipi_ddbirq);
-   evcount_attach(ipi_ddb[1], ipi_ddb1, ipi_ddbirq);
+   evcount_attach(ipi_count, ipi, ipi_irq);
 #endif
 
/* clear all pending interrunts */
@@ -638,16 +632,11 @@ openpic_ext_intr(void)
return;
}
 #ifdef MULTIPROCESSOR
-   if (irq == IPI_VECTOR_NOP) {
-   ipi_nop[ci-ci_cpuid].ec_count++;
+   if (irq == IPI_VECTOR_NOP || irq == IPI_VECTOR_DDB) {
+   ipi_count.ec_count++;
openpic_eoi(ci-ci_cpuid);
-   irq = openpic_read_irq(ci-ci_cpuid);
-   continue;
-   }
-   if (irq == IPI_VECTOR_DDB) {
-   ipi_ddb[ci-ci_cpuid].ec_count++;
-   openpic_eoi(ci-ci_cpuid);
-   openpic_ipi_ddb();
+   if (irq == IPI_VECTOR_DDB)
+   openpic_ipi_ddb();
irq = openpic_read_irq(ci-ci_cpuid);
continue;
}
@@ -758,7 +747,6 @@ openpic_send_ipi(struct cpu_info *ci, in
 void
 openpic_ipi_ddb(void)
 {
-   DPRINTF(ipi_ddb() called\n);
 #ifdef DDB
Debugger();
 #endif