From owner-freebsd-hackers  Mon Feb  3 08:36:51 1997
Return-Path: <owner-hackers>
Received: (from root@localhost)
          by freefall.freebsd.org (8.8.5/8.8.5) id IAA06721
          for hackers-outgoing; Mon, 3 Feb 1997 08:36:51 -0800 (PST)
Received: from terra.Sarnoff.COM (terra.sarnoff.com [130.33.11.203])
          by freefall.freebsd.org (8.8.5/8.8.5) with SMTP id IAA06716
          for <freebsd-hackers@FreeBSD.ORG>; Mon, 3 Feb 1997 08:36:48 -0800 (PST)
Received: (from rminnich@localhost) by terra.Sarnoff.COM (8.6.12/8.6.12) id LAA04463; Mon, 3 Feb 1997 11:35:21 -0500
Date: Mon, 3 Feb 1997 11:35:20 -0500 (EST)
From: "Ron G. Minnich" <rminnich@Sarnoff.COM>
X-Sender: rminnich@terra
To: Andrew.Gordon@net-tel.co.uk
cc: freebsd-hackers@FreeBSD.ORG
Subject: Re: OK, here goes fastlock: (fwd)
In-Reply-To: <"3a14-970203154823-6178*/G=Andrew/S=Gordon/O=NET-TEL Computer Systems Ltd/PRMD=NET-TEL/ADMD=Gold 400/C=GB/"@MHS>
Message-ID: <Pine.SUN.3.91.970203111656.3325P-100000@terra>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: owner-hackers@FreeBSD.ORG
X-Loop: FreeBSD.org
Precedence: bulk

> > fastunlock(int *address, int unlockval)
> > {
> >   int dosyscall = 0;
> >   static int syscallnum = -1; /* this is really in the file */
> >   if (*address & 0x80000000)  /***/
> >     dosyscall = 1;
> >   *address = unlockval;
> >   if (dosyscall)
> >     syscall(syscallnum, 0, address, unlockval);
> > }
> 
> Isn't there a race here?  If the process holding the lock happens to
> get descheduled at the point marked /***/ and another process blocks
> waiting for the lock, the syscall doesn't get called and the second
> process continues to block until the 10 second timeout in the syscall.

You're right there is a race. It's not wide, and I have never hit it, 
but it's certainly there. What I should have done, but mistakenly did 
not do, was use an indivisible sequence of instructions for unlock as 
well. Mea culpa. 

What it seems to me I should have done: 
	let's assume for example I successfully fastlocked()'ed 
            and we set the lockword to 0x1. 

	We come to the unlock code: 
	If there is contention, the value will be 0x80000001
	For the first unlock attempt, 
	assume no contention: test and set with the test value 
	of 1 (our lock value) and the set value of 0. 

	IF the tset succeeds, the we have indivisibly unlocked it; done.

	If the tset fails, then there is contention. So test and set
	with the value 0x80000001. At this point this should have 
	succeeded. Then call the system call. 

Both tset sequences on the unlock are indivisible, eliminating the race. 

Does this look right now? 

> > 2) The 'address' argument does NOT NEED TO BE AN ADDRESS. it's a number
> >    that all the procs have to agree on, that is all.
> You seem to be assuming that it _is_ an address in this implementation
> however:
> >                         int curval = fuword((void *) uap->address);

Ouch, foolish me. Obviously I've never used it as 'not an address'. 
Sorry about that!

Thanks for the comments! But now i've got something to fix!
ron