>
> dg@informix.com (David Gould) writes:
>
> [ reversed, and possibly bad NS32K patch removed from quoted material. ]
>
> > I wish I had noticed this before Bruce applied it.
>
> Me too, I wrote a quick hack that worked, and I always like people
> who know what's really going on to double check things when possible.
>
> > The TAS function is needed so that stuck spinlocks can be recovered from.
> > Also, it enables the pseudo random back off which helps performance when
> > there are many backends.
>
> I suspected as much, which is why I wrote a tas function to fix
> the link errors, rather than try to remove the code that called
> tas.
>
> > In any case, this patch does not "follow the one true path" that I tried
> > to outline in s_lock.c and s_lock.h. In fact it is exactly backwards.
> >
> > Basically the preferred way is:
> >
> > - in s_lock.h do nothing, the defaults should take care of you.
> >
> > -in s_lock.c define a TAS function that sets the spinlock and returns the
> > previous state of the lock.
> >
> > I see from your asm()s that you are using gcc. In this case, your TAS function
> > should be called tas(), and should be defined inside the __GNUC__ section.
>
> Your quote removes the comment that says something to the effect
> of: I think I built a bogus patch, you might need patch -r to make
> it work because it's backwards. In fact you might need more than
> that to get the right pathnames in the right places. (My quoting
> removes the whole patch... 8-) I built the patch by hand, since
> cvs patch spewed out a whole bunch of stuff in files I know I never
> touched. I figured they were altered in the build process or
> something...
>
> However, what I did on my machine was to remove a #define S_LOCK ...
> from s_lock.h and add function tas in s_lock.c. It wouldn't link
> the way it came out of cvs checkout, but it would with the changes
> I just described.
>
> I suspect Bruce got it right, otherwise hed have a handful of
> garbled code. I think it *might* be like you describe it should
> be, but please double check it, I will. (Like I said above, the
> more the better, since I don't really know what I'm doing, just
> making some guesses.)
I completely missed the part about the patch being reversed... duh.
Ok, so if what you did is undefine the S_LOCK for your platform (to select
the default), and add a tas() function to s_lock.c then that is exactly
what was supposed to happen and I was just confused by the reversed patch.
Since Bruce un-applied your change, the simplest way to fix things from here
might be for you to submit a new (un-reversed this time) patch and after
Bruce applies it I will pull it down and verify it.
Sorry for the confusion.
-dg
David Gould dg@informix.com 510.628.3783 or 510.305.9468
Informix Software (No, really) 300 Lakeside Drive Oakland, CA 94612
- If simplicity worked, the world would be overrun with insects. -
