Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Jan 6, 2014 at 2:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> -1 for the any_spinlock_held business (useless overhead IMO, as it doesn't
>> have anything whatsoever to do with enforcing the actual coding rule).
> Hmm. I thought that was a pretty well-aimed bullet myself; why do you
> think that it isn't? I don't particularly mind ripping it out, but it
> seemed like a good automated test to me.
The coding rule is "only short straight-line code while holding a
spinlock". That could be violated in any number of nasty ways without
trying to take another spinlock. And since the whole point of the rule is
that spinlock-holding code segments should be quick, adding more overhead
to them doesn't seem very nice, even if it is only done in Assert builds.
I agree it'd be nicer if we had some better way than mere manual
inspection to enforce proper use of spinlocks; but this change doesn't
seem to me to move the ball downfield by any meaningful distance.
>> And I'd suggest defining NUM_SPINLOCK_SEMAPHORES in pg_config_manual.h,
>> and maybe dropping SpinlockSemas() altogether in favor of just referencing
>> the constant. Otherwise this seems reasonable.
> As far as pg_config_manual.h is concerned, is this the sort of thing
> you have in mind?
> #ifndef HAVE_SPINLOCKS
> #define NUM_SPINLOCK_SEMAPHORES 1024
> #endif
I think we can just define it unconditionally, don't you? It shouldn't
get referenced in HAVE_SPINLOCK builds. Or is the point that you want
to enforce that?
regards, tom lane