On Tue, Jul 28, 2015 at 3:28 PM, Heikki Linnakangas <hlinn...@iki.fi> wrote:
> * The patch requires that the LWLOCK_INDIVIDUAL_NAMES array is kept in sync
> with the list of individual locks in lwlock.h. Sooner or later someone will
> add an LWLock and forget to update the names-array. That needs to be made
> less error-prone, so that the names are maintained in the same place as the
> #defines. Perhaps something like rmgrlist.h.
This is a good idea, but it's not easy to do in the style of
rmgrlist.h, because I don't believe there's any way to define a macro
that expands to a preprocessor directive. Attached is a patch that
instead generates the list of macros from a text file, and also
generates an array inside lwlock.c with the lock names that gets used
by the Trace_lwlocks stuff where applicable.
Any objections to this solution to the problem? If not, I'd like to
go ahead and push this much. I can't test the Windows changes
locally, though, so it would be helpful if someone could check that
out.
> * Instead of having LWLOCK_INDIVIDUAL_NAMES to name "individual" locks, how
> about just giving each one of them a separate tranche?
I don't think it's good to split things up to that degree;
standardizing on one name per fixed lwlock and one per tranche
otherwise seems like a good compromise to me.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 98b978f..d16ab10 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -106,7 +106,7 @@ endif
endif # aix
# Update the commonly used headers before building the subdirectories
-$(SUBDIRS:%=%-recursive): $(top_builddir)/src/include/parser/gram.h $(top_builddir)/src/include/catalog/schemapg.h $(top_builddir)/src/include/utils/fmgroids.h $(top_builddir)/src/include/utils/errcodes.h $(top_builddir)/src/include/utils/probes.h
+$(SUBDIRS:%=%-recursive): $(top_builddir)/src/include/parser/gram.h $(top_builddir)/src/include/catalog/schemapg.h $(top_builddir)/src/include/storage/lwlocknames.h $(top_builddir)/src/include/utils/fmgroids.h $(top_builddir)/src/include/utils/errcodes.h $(top_builddir)/src/include/utils/probes.h
# run this unconditionally to avoid needing to know its dependencies here:
submake-schemapg:
@@ -135,6 +135,9 @@ postgres.o: $(OBJS)
parser/gram.h: parser/gram.y
$(MAKE) -C parser gram.h
+storage/lmgr/lwlocknames.h: storage/lmgr/generate-lwlocknames.pl storage/lmgr/lwlocknames.txt
+ $(MAKE) -C storage/lmgr lwlocknames.h
+
utils/fmgroids.h: utils/Gen_fmgrtab.pl catalog/Catalog.pm $(top_srcdir)/src/include/catalog/pg_proc.h
$(MAKE) -C utils fmgroids.h
@@ -165,6 +168,11 @@ $(top_builddir)/src/include/catalog/schemapg.h: catalog/schemapg.h
cd '$(dir $@)' && rm -f $(notdir $@) && \
$(LN_S) "$$prereqdir/$(notdir $<)" .
+$(top_builddir)/src/include/storage/lwlocknames.h: storage/lmgr/lwlocknames.h
+ prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
+ cd '$(dir $@)' && rm -f $(notdir $@) && \
+ $(LN_S) "$$prereqdir/$(notdir $<)" .
+
$(top_builddir)/src/include/utils/errcodes.h: utils/errcodes.h
prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
cd '$(dir $@)' && rm -f $(notdir $@) && \
@@ -192,6 +200,7 @@ distprep:
$(MAKE) -C bootstrap bootparse.c bootscanner.c
$(MAKE) -C catalog schemapg.h postgres.bki postgres.description postgres.shdescription
$(MAKE) -C replication repl_gram.c repl_scanner.c
+ $(MAKE) -C storage lwlocknames.h
$(MAKE) -C utils fmgrtab.c fmgroids.h errcodes.h
$(MAKE) -C utils/misc guc-file.c
$(MAKE) -C utils/sort qsort_tuple.c
@@ -307,6 +316,7 @@ maintainer-clean: distclean
catalog/postgres.shdescription \
replication/repl_gram.c \
replication/repl_scanner.c \
+ storage/lmgr/lwlocknames.h \
utils/fmgroids.h \
utils/fmgrtab.c \
utils/errcodes.h \
diff --git a/src/backend/storage/lmgr/.gitignore b/src/backend/storage/lmgr/.gitignore
new file mode 100644
index 0000000..9355cae
--- /dev/null
+++ b/src/backend/storage/lmgr/.gitignore
@@ -0,0 +1,2 @@
+/lwlocknames.c
+/lwlocknames.h
diff --git a/src/backend/storage/lmgr/Makefile b/src/backend/storage/lmgr/Makefile
index e12a854..e941f2d 100644
--- a/src/backend/storage/lmgr/Makefile
+++ b/src/backend/storage/lmgr/Makefile
@@ -24,8 +24,17 @@ s_lock_test: s_lock.c $(top_builddir)/src/port/libpgport.a
$(CC) $(CPPFLAGS) $(CFLAGS) -DS_LOCK_TEST=1 $(srcdir)/s_lock.c \
$(TASPATH) -L $(top_builddir)/src/port -lpgport -o s_lock_test
+# see explanation in ../../parser/Makefile
+lwlocknames.c: lwlocknames.h ;
+
+lwlocknames.h: $(top_srcdir)/src/backend/storage/lmgr/lwlocknames.txt generate-lwlocknames.pl
+ $(PERL) $(srcdir)/generate-lwlocknames.pl $<
+
check: s_lock_test
./s_lock_test
-clean distclean maintainer-clean:
+clean distclean:
rm -f s_lock_test
+
+maintainer-clean: clean
+ rm -f lwlocknames.h
diff --git a/src/backend/storage/lmgr/generate-lwlocknames.pl b/src/backend/storage/lmgr/generate-lwlocknames.pl
new file mode 100644
index 0000000..7ce4e2a
--- /dev/null
+++ b/src/backend/storage/lmgr/generate-lwlocknames.pl
@@ -0,0 +1,62 @@
+#!/usr/bin/perl
+#
+# Generate lwlocknames.h and lwlocknames.c from lwlocknames.txt
+# Copyright (c) 2000-2015, PostgreSQL Global Development Group
+
+use warnings;
+use strict;
+
+print
+ "/* autogenerated from src/backend/storage/lmgr/lwlocknames.txt, do not edit */\n";
+print "/* there is deliberately not an #ifndef LWLOCKNAMES_H here */\n";
+
+my $lastlockidx = -1;
+my $continue = "\n";
+
+open my $lwlocknames, $ARGV[0] or die;
+
+# Include PID in suffix in case parallel make runs this multiple times.
+my $htmp = "lwlocknames.h.tmp$$";
+my $ctmp = "lwlocknames.c.tmp$$";
+open H, '>', $htmp or die "Could not open $htmp: $!";
+open C, '>', $ctmp or die "Could not open $ctmp: $!";
+
+print C "static char *MainLWLockNames[] = {";
+
+while (<$lwlocknames>)
+{
+ chomp;
+
+ # Skip comments
+ next if /^#/;
+ next if /^\s*$/;
+
+ die "unable to parse lwlocknames.txt"
+ unless /^(\w+)\s+(\d+)$/;
+
+ (my $lockname, my $lockidx) = ($1, $2);
+
+ die "lwlocknames.txt not in order" if $lockidx < $lastlockidx;
+ die "lwlocknames.txt has duplicates" if $lockidx == $lastlockidx;
+
+ while ($lastlockidx < $lockidx - 1)
+ {
+ ++$lastlockidx;
+ printf C "%s \"<unassigned:%d>\"", $continue, $lastlockidx;
+ $continue = ",\n";
+ }
+ printf C "%s \"%s\"", $continue, $lockname;
+ $lastlockidx = $lockidx;
+ $continue = ",\n";
+
+ print H "#define $lockname (&MainLWLockArray[$lockidx].lock)\n";
+}
+
+printf C "\n};\n";
+print H "\n";
+printf H "#define NUM_INDIVIDUAL_LWLOCKS %s\n", $lastlockidx + 1;
+
+rename($htmp, 'lwlocknames.h') || die "rename: $htmp: $!";
+rename($ctmp, 'lwlocknames.c') || die "rename: $ctmp: $!";
+
+close $lwlocknames;
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 687ed63..db10a96 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -95,6 +95,9 @@
#include "utils/hsearch.h"
#endif
+/* Constants for lwlock names */
+#include "lwlocknames.c"
+
/* We use the ShmemLock spinlock to protect LWLockAssign */
extern slock_t *ShmemLock;
@@ -183,18 +186,32 @@ PRINT_LWDEBUG(const char *where, LWLock *lock, LWLockMode mode)
if (Trace_lwlocks)
{
uint32 state = pg_atomic_read_u32(&lock->state);
-
- ereport(LOG,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("%d: %s(%s %d): excl %u shared %u haswaiters %u waiters %u rOK %d",
- MyProcPid,
- where, T_NAME(lock), T_ID(lock),
- !!(state & LW_VAL_EXCLUSIVE),
- state & LW_SHARED_MASK,
- !!(state & LW_FLAG_HAS_WAITERS),
- pg_atomic_read_u32(&lock->nwaiters),
- !!(state & LW_FLAG_RELEASE_OK))));
+ int id = T_ID(lock);
+
+ if (lock->tranche == 0 && id < NUM_INDIVIDUAL_LWLOCKS)
+ ereport(LOG,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("%d: %s(%s): excl %u shared %u haswaiters %u waiters %u rOK %d",
+ MyProcPid,
+ where, MainLWLockNames[id],
+ !!(state & LW_VAL_EXCLUSIVE),
+ state & LW_SHARED_MASK,
+ !!(state & LW_FLAG_HAS_WAITERS),
+ pg_atomic_read_u32(&lock->nwaiters),
+ !!(state & LW_FLAG_RELEASE_OK))));
+ else
+ ereport(LOG,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("%d: %s(%s %d): excl %u shared %u haswaiters %u waiters %u rOK %d",
+ MyProcPid,
+ where, T_NAME(lock), id,
+ !!(state & LW_VAL_EXCLUSIVE),
+ state & LW_SHARED_MASK,
+ !!(state & LW_FLAG_HAS_WAITERS),
+ pg_atomic_read_u32(&lock->nwaiters),
+ !!(state & LW_FLAG_RELEASE_OK))));
}
}
@@ -204,11 +221,20 @@ LOG_LWDEBUG(const char *where, LWLock *lock, const char *msg)
/* hide statement & context here, otherwise the log is just too verbose */
if (Trace_lwlocks)
{
- ereport(LOG,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("%s(%s %d): %s", where,
- T_NAME(lock), T_ID(lock), msg)));
+ int id = T_ID(lock);
+
+ if (lock->tranche == 0 && id < NUM_INDIVIDUAL_LWLOCKS)
+ ereport(LOG,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("%s(%s): %s", where,
+ MainLWLockNames[id], msg)));
+ else
+ ereport(LOG,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("%s(%s %d): %s", where,
+ T_NAME(lock), id, msg)));
}
}
diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txt
new file mode 100644
index 0000000..96bbfe8
--- /dev/null
+++ b/src/backend/storage/lmgr/lwlocknames.txt
@@ -0,0 +1,47 @@
+# Some commonly-used locks have predefined positions within MainLWLockArray;
+# these are defined here. If you add a lock, add it to the end to avoid
+# renumbering the existing locks; if you remove a lock, consider leaving a gap
+# in the numbering sequence for the benefit of DTrace and other external
+# debugging scripts.
+
+# 0 is available; was formerly BufFreelistLock
+ShmemIndexLock 1
+OidGenLock 2
+XidGenLock 3
+ProcArrayLock 4
+SInvalReadLock 5
+SInvalWriteLock 6
+WALBufMappingLock 7
+WALWriteLock 8
+ControlFileLock 9
+CheckpointLock 10
+CLogControlLock 11
+SubtransControlLock 12
+MultiXactGenLock 13
+MultiXactOffsetControlLock 14
+MultiXactMemberControlLock 15
+RelCacheInitLock 16
+CheckpointerCommLock 17
+TwoPhaseStateLock 18
+TablespaceCreateLock 19
+BtreeVacuumLock 20
+AddinShmemInitLock 21
+AutovacuumLock 22
+AutovacuumScheduleLock 23
+SyncScanLock 24
+RelationMappingLock 25
+AsyncCtlLock 26
+AsyncQueueLock 27
+SerializableXactHashLock 28
+SerializableFinishedListLock 29
+SerializablePredicateLockListLock 30
+OldSerXidLock 31
+SyncRepLock 32
+BackgroundWorkerLock 33
+DynamicSharedMemoryControlLock 34
+AutoFileLock 35
+ReplicationSlotAllocationLock 36
+ReplicationSlotControlLock 37
+CommitTsControlLock 38
+CommitTsLock 39
+ReplicationOriginLock 40
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index e278fa0..0c9b545 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -281,7 +281,7 @@
/*
* Enable debugging print statements for lock-related operations.
*/
-/* #define LOCK_DEBUG */
+#define LOCK_DEBUG 1
/*
* Enable debugging print statements for WAL-related operations; see
diff --git a/src/include/storage/.gitignore b/src/include/storage/.gitignore
new file mode 100644
index 0000000..209c8be
--- /dev/null
+++ b/src/include/storage/.gitignore
@@ -0,0 +1 @@
+/lwlocknames.h
diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h
index cbd6318..ec3f350 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -87,56 +87,8 @@ typedef union LWLockPadded
} LWLockPadded;
extern PGDLLIMPORT LWLockPadded *MainLWLockArray;
-/*
- * Some commonly-used locks have predefined positions within MainLWLockArray;
- * defining macros here makes it much easier to keep track of these. If you
- * add a lock, add it to the end to avoid renumbering the existing locks;
- * if you remove a lock, consider leaving a gap in the numbering sequence for
- * the benefit of DTrace and other external debugging scripts.
- */
-/* 0 is available; was formerly BufFreelistLock */
-#define ShmemIndexLock (&MainLWLockArray[1].lock)
-#define OidGenLock (&MainLWLockArray[2].lock)
-#define XidGenLock (&MainLWLockArray[3].lock)
-#define ProcArrayLock (&MainLWLockArray[4].lock)
-#define SInvalReadLock (&MainLWLockArray[5].lock)
-#define SInvalWriteLock (&MainLWLockArray[6].lock)
-#define WALBufMappingLock (&MainLWLockArray[7].lock)
-#define WALWriteLock (&MainLWLockArray[8].lock)
-#define ControlFileLock (&MainLWLockArray[9].lock)
-#define CheckpointLock (&MainLWLockArray[10].lock)
-#define CLogControlLock (&MainLWLockArray[11].lock)
-#define SubtransControlLock (&MainLWLockArray[12].lock)
-#define MultiXactGenLock (&MainLWLockArray[13].lock)
-#define MultiXactOffsetControlLock (&MainLWLockArray[14].lock)
-#define MultiXactMemberControlLock (&MainLWLockArray[15].lock)
-#define RelCacheInitLock (&MainLWLockArray[16].lock)
-#define CheckpointerCommLock (&MainLWLockArray[17].lock)
-#define TwoPhaseStateLock (&MainLWLockArray[18].lock)
-#define TablespaceCreateLock (&MainLWLockArray[19].lock)
-#define BtreeVacuumLock (&MainLWLockArray[20].lock)
-#define AddinShmemInitLock (&MainLWLockArray[21].lock)
-#define AutovacuumLock (&MainLWLockArray[22].lock)
-#define AutovacuumScheduleLock (&MainLWLockArray[23].lock)
-#define SyncScanLock (&MainLWLockArray[24].lock)
-#define RelationMappingLock (&MainLWLockArray[25].lock)
-#define AsyncCtlLock (&MainLWLockArray[26].lock)
-#define AsyncQueueLock (&MainLWLockArray[27].lock)
-#define SerializableXactHashLock (&MainLWLockArray[28].lock)
-#define SerializableFinishedListLock (&MainLWLockArray[29].lock)
-#define SerializablePredicateLockListLock (&MainLWLockArray[30].lock)
-#define OldSerXidLock (&MainLWLockArray[31].lock)
-#define SyncRepLock (&MainLWLockArray[32].lock)
-#define BackgroundWorkerLock (&MainLWLockArray[33].lock)
-#define DynamicSharedMemoryControlLock (&MainLWLockArray[34].lock)
-#define AutoFileLock (&MainLWLockArray[35].lock)
-#define ReplicationSlotAllocationLock (&MainLWLockArray[36].lock)
-#define ReplicationSlotControlLock (&MainLWLockArray[37].lock)
-#define CommitTsControlLock (&MainLWLockArray[38].lock)
-#define CommitTsLock (&MainLWLockArray[39].lock)
-#define ReplicationOriginLock (&MainLWLockArray[40].lock)
-
-#define NUM_INDIVIDUAL_LWLOCKS 41
+/* Names for fixed lwlocks */
+#include "lwlocknames.h"
/*
* It's a bit odd to declare NUM_BUFFER_PARTITIONS and NUM_LOCK_PARTITIONS
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 6b16e69..fe1f08d 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -285,6 +285,22 @@ s{PG_VERSION_STR "[^"]+"}{__STRINGIFY(x) #x\n#define __STRINGIFY2(z) __STRINGIFY
'src/include/utils/fmgroids.h');
}
+ if (IsNewer(
+ 'src/backend/storage/lmgr/lwlocknames.txt', 'src/include/storage/lwlocknames.h'))
+ {
+ print "Generating lwlocknames.c and fmgroids.h...\n";
+ chdir('src/backend/storage/lmgr');
+ system('perl generate-lwlocknames.pl lwlocknames.txt');
+ chdir('../../../..');
+ }
+ if (IsNewer(
+ 'src/include/storage/lwlocknames.h',
+ 'src/backend/storage/lmgr/fmgroids.h'))
+ {
+ copyFile('src/backend/storage/lmgr/lwlocknames.h',
+ 'src/include/storage/lwlocknames.h');
+ }
+
if (IsNewer('src/include/utils/probes.h', 'src/backend/utils/probes.d'))
{
print "Generating probes.h...\n";
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
