Re: "movb" for spin-unlock (was Re: namei() query)

From: Oliver Xymoron (oxymoron@waste.org)
Date: Sun Apr 23 2000 - 11:12:14 EST


On Sat, 22 Apr 2000, Oliver Xymoron wrote:

> On Sat, 22 Apr 2000, Jamie Lokier wrote:
>
> > Linus Torvalds wrote:
> > > I have conflicting reports about the safety of "movb" from Intel.
> > > According to some people in there, "movb" is always safe, and there should
> > > not be any need for any config option at all.
> > >
> > > However, at the same time my original contact at intel was Andy Glew, who
> > > probably knows more about the ia32 core than anybody else I know. And Andy
> > > says that yes "movb" is legal, but that some very early P6 steppings may
> > > be buggy. And Andy is God.
> >
> > That comment in <asm-i386/spinlock.h> is rather tantalising. It says
> > don't use "movb" because it doesn't work but gives no clues why.
> >
> > I still have the thread where this was hashed out. And it seemed very
> > few people ended up understanding the precise reason for not using
> > "movb". Not me :-(
>
> There are very few things that could cause the movb to be a problem. For
> instance, it can't be in the cache coherency protocol as the unlock can
> be lazy at it likes and still be safe. My only guess is that somehow the
> movb can get scheduled ahead of reads or writes inside the critical
> section. If that's the case, then the whole coherency scheme is broken,
> no? We'd need to rethink quite a number of things we've presumed safe.
> My guess is the whole thing is apocryphal.

Ok, I read the Pentium Pro errata, available at

 http://developer.intel.com/design/pro/specupdt/242689.htm

and the issues relevant to MP locking appear to be 1, 39, 41, 51, 66, and
92. All but 1 and 92 were fixed in the PPro. Some of them are about
processors having inconsistent MTRR settings (which should be a non-issue
for us) while the others are about cache coherency and snooping. From my
reading the latter allow may allow violations of write-ordering but none
that would allow a second processor to acquire a lock before the first had
released it. This is not surprising as the spinlock *would have to see
someone unlock before the unlock actually happened*. That'd be spooky. So
again, the unlock is just inherently safer than the lock side. Everyone
feel free to double-check this, but I still see no reason we can't use the
faster movb.

Below is Manfred's lock test code if people with Pentium Pros want to
bang on it. If it locks up, we have a problem. Set USE_MB to one to use
the mov-based unlock. My dual PPro is a later stepping 9 and seems to work
just fine. If you have an SMP system with steppings 1-8 (only 1, 2, 6, and
7 should be out there) and can confirm this works for you, that'd be
great.

/*
 * movopt: test for Intel memory ordering.
 * Copyright (C) 1999 by Manfred Spraul.
 *
 * Redistribution of this file is permitted under the terms of the GNU
 * Public License (GPL)
 * $Header: /pub/cvs/ms/movopt/movopt.cpp,v 1.3 1999/11/25 23:38:44 manfreds Ex
p $
 */

/*
   NOTE: this code will run _extremely_ slowly on UP systems

   func1 and func2 are the functions that may race with each other

   cpu1 and cpu2 are thread functions that synchronize func1 and func2
   while adjusting timings
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <pthread.h>
#include <assert.h>

#define USE_MB 0
#define USE_ASM 1

volatile int start = 0;
volatile int current_state = 0;
volatile int lock = 1;
volatile int ready = 0;
volatile int go = 0;

#if USE_ASM == 0 /* doesn't compile, here for explanation */

static inline void func1()
{
          set_current_state(1);
          if(lock!=1)
                while (current_state!=0)
                        ;
}

static inline void func2()
{
          lock=0;
          current_state=0;
          mb();
}

#else
static inline void func1()
{
        __asm__ __volatile(
#if USE_MB == 0
                "movl $1,%1\n\t" /* set current_state = 1 */
#else
                "lock;bts $0,%1\n\t"
#endif
                "movl %0,%%eax\n\t" /* lock into %%eax */
                "cmpl $1,%%eax\n\t"
                "jne dont_sleep\n"
                "retry:\n\t"
                "movl %1, %%eax\n\t"
                "cmpl $0,%%eax\n\t"
                "jne retry\n"
                "dont_sleep:\n"
                : /* no output */
                : "m" (lock), "m" (current_state)
                : "eax", "cc", "memory");
}

static inline void func2()
{
        __asm__ __volatile(
                "movl $0,%0\n\t"
                "movl $0,%1\n\t"
                "lock; addl $0,(%%esp)\n\t" /* flush our write buffers*/
                : /* no output */
                : "m" (lock), "m" (current_state)
                : "memory");
}
#endif

void* cpu1(void* param)
{
        /* 1: always sleep
           10 000: never sleep
           PII/350: lock-up at delay=217
           */
        volatile int delay = 1;

        int i=0,j;

        while(!start)
                ;

        for(;i<50000000;i++) {
                lock = 1;
                current_state = 0;
                go = 0;
                while(!ready)
                        ;
                go = 1;

                for(j=0;j<delay;j++)
                        ;

                func1();

                if((i%5000)==0) {
                        printf("delay %d: ok\n",delay);
                        delay++;
                }
        }

        printf("thread %d finished.\n",(int)param);
        exit(0);
}

void* cpu2(void* param)
{
        /* increase this value if no lock-up occurs,
           eg: 2000
           */
        volatile int delay2 = 300;

        int i=0,j;

        while(!start)
                ;

        for(;;) {
                ready = 1;
                while(!go)
                        ;
                ready = 0;
                go = 0;

                for(j=0;j<delay2;j++)
                        ;
                
                func2();
        }
}

typedef void *threadfunc(void *);

void start_thread(threadfunc *f)
{
        pthread_t thread;
        int res;

        res = pthread_create(&thread,NULL,f,NULL);
        if(res != 0)
                assert(0);
}

int main()
{
        printf("movopt:\n");
        start_thread(cpu1);
        start_thread(cpu2);
        printf(" starting, please wait.\n");
        fflush(stdout);
        start = 1;
        for(;;) sleep(1000);
}

--
 "Love the dolphins," she advised him. "Write by W.A.S.T.E.." 

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sun Apr 23 2000 - 21:00:22 EST