Re: fast softint support for sparc64

[Eduardo Horvath <eeh%NetBSD.org@localhost>]

On Sat, 18 Jun 2011, Takeshi Nakayama wrote:

> > + * an interrupt handler blocks.
> > + *
> > + * Arguments:
> > + * o0      old lwp from cpu_switchto()
> > + *
> > + * from softint_fastintr():
> > + * l0      CPUINFO_VA
> > + * l6      saved ci_eintstack
> > + * l7      saved ipl
> > + */
> > +softint_fastintr_ret:
> > +   /* re-adjust after mi_switch() */
> > +   ld      [%l0 + CI_MTX_COUNT], %o1
> > +   inc     %o1                             ! ci_mtx_count++
> > +   st      %o1, [%l0 + CI_MTX_COUNT]
> > +   membar  #LoadLoad                       ! membar_exit()
> > 
> > LoadLoad?  All loads are completed before the next load?  This doesn't 
> > sound right.  What are you trying to sync here?
> >
> > +   st      %g0, [%o0 + L_CTXSWTCH]         ! prev->l_ctxswtch = 0
> 
> These code is borrowed from mi_switch() in kern_synch.c:
> 
>   http://nxr.netbsd.org/xref/src/sys/kern/kern_synch.c#797
> 
> and membar_exit() for sparc64 is "membar #LoadLoad":
> 
>   
> http://nxr.netbsd.org/xref/src/common/lib/libc/arch/sparc64/atomic/membar_ops.S#42
> 
> Should it be "membar #StoreStore" ?

If you're trying to order the store to the CI_MTX_COUNT with the store to 
L_CTXSWTCH then I think it should be #StoreStore.  

Note the comment in membar_ops.S:

/* These assume Total Store Order (TSO) */

Um, if you're running TSO then you don't need any membars at all.  We 
should really be coding this for RMO to get some performance improvements.  
Otherwise leaving them out will reduce instruction count and CPU cycles.

Hm.  Looking more at mi_switch() it doesn't seem all that mi to me.  It 
appears to be using l->l_ctxswtch as a synchronization flag, but the code 
where it's set:

    733                 KASSERT(l->l_ctxswtch == 0);
    734                 l->l_ctxswtch = 1;
    735                 l->l_ncsw++;
    736                 KASSERT((l->l_pflag & LP_RUNNING) != 0);
    737                 l->l_pflag &= ~LP_RUNNING;
    738 

really needs a membar after setting it to 1 to make sure the store 
completes before continuing.  Well this explains why things didn't work 
when I fixed up the stuff in libatomic and tried running the kernel RMO.

OTOH I think you can just leave the membar out.  It should work fine and 
simplify code maintenance in the future.  As far as I'm concerned extra 
membars in the wrong places are worse than not having them at all.  The 
chance someone will go fix the membar correctly in future is pretty close 
to zero.

Eduardo