Re: Strange test failures

[Martin Husemann <martin%duskware.de@localhost>]

On Mon, Apr 18, 2011 at 06:58:38PM +0200, Martin Husemann wrote:

I had a closer look and it seems like two bugs happily playing together
to me:

> #5  0x0000000042774d04 in percpu_cpu_swap (p1=0x42909118, 
>     p2=0xffffffffffff9788)
>     at /usr/src/sys/rump/librump/rumpkern/../../../kern/subr_percpu.c:93
> 93              KASSERT(ci == curcpu());
> (gdb) list
> 88      {
> 89              struct cpu_info * const ci = p1;
> 90              percpu_cpu_t * const newpcc = p2;
> 91              percpu_cpu_t * const pcc = cpu_percpu(ci);
> 92      
> 93              KASSERT(ci == curcpu());

This KASSERT seems wrong to me, because the percpu
code explicitly calls this function for all cpus:

> (gdb) up
> #6  0x0000000042774e64 in percpu_cpu_enlarge (size=2064)
>     at /usr/src/sys/rump/librump/rumpkern/../../../kern/subr_percpu.c:147
> 147                             percpu_cpu_swap(ci, &pcc);
> (gdb) list
> 142                     percpu_cpu_t pcc;
> 143     
> 144                     pcc.pcc_data = kmem_alloc(size, KM_SLEEP); /* XXX 
> cacheline */
> 145                     pcc.pcc_size = size;
> 146                     if (!mp_online) {
> 147                             percpu_cpu_swap(ci, &pcc);

Now in rump (at least in my instance) the CPU_INFO_FOREACH() iterator already
iterates over all available (virtual) cpus, even if those are not attached
yet (and mp_online still is false).

This could be considered the second bug.

Easy fix:

Index: subr_percpu.c
===================================================================
RCS file: /cvsroot/src/sys/kern/subr_percpu.c,v
retrieving revision 1.11
diff -u -r1.11 subr_percpu.c
--- subr_percpu.c       14 Apr 2011 05:53:53 -0000      1.11
+++ subr_percpu.c       18 Apr 2011 21:24:00 -0000
@@ -90,7 +90,7 @@
        percpu_cpu_t * const newpcc = p2;
        percpu_cpu_t * const pcc = cpu_percpu(ci);
 
-       KASSERT(ci == curcpu());
+       KASSERT(ci == curcpu() || !mp_online);
 
        /*
         * swap *pcc and *newpcc unless anyone has beaten us.


And, not suprising, this makes the failing test cases pass.

Martin