On Mon, Mar 17, 2014 at 11:32 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Robert Haas <robertmh...@gmail.com> writes:
>> One option is to just change that function to also unmap the control
>> segment, and maybe rename it to dsm_detach_all(), and then use that
>> everywhere. The problem is that I'm not sure we really want to incur
>> the overhead of an extra munmap() during every backend exit, just to
>> get rid of a control segment which was going to be unmapped anyway by
>> process termination. For that matter, I'm not sure why we bother
>> arranging that for the main shared memory segment, either: surely
>> whatever function the shmdt() and munmap() calls in IpcMemoryDetach
>> may have will be equally well-served by the forthcoming exit()?
>
> Before you lobotomize that code too much, consider the postmaster
> crash-recovery case. That path does need to detach from the old
> shmem segment.
>
> Also, I might be wrong, but I think IpcMemoryDetach is a *postmaster*
> on_shmem_exit routine; it isn't called during backend exit.
Ah, right. I verified using strace that, at least on Linux
2.6.32-279.el6.x86_64, the only system call made on disconnecting a
psql session is exit_group(0). I also tried setting breakpoints on
PGSharedMemoryDetach and IpcMemoryDetach and they do not fire.
After mulling over a few possible approaches, I came up with the
attached, which seems short and to the point.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index cf2ce46..8153160 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -40,6 +40,7 @@
#include "postmaster/fork_process.h"
#include "postmaster/pgarch.h"
#include "postmaster/postmaster.h"
+#include "storage/dsm.h"
#include "storage/fd.h"
#include "storage/ipc.h"
#include "storage/latch.h"
@@ -163,6 +164,7 @@ pgarch_start(void)
on_exit_reset();
/* Drop our connection to postmaster's shared memory, as well */
+ dsm_detach_all();
PGSharedMemoryDetach();
PgArchiverMain(0, NULL);
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 1ca5d13..3dc280a 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -50,6 +50,7 @@
#include "postmaster/postmaster.h"
#include "storage/proc.h"
#include "storage/backendid.h"
+#include "storage/dsm.h"
#include "storage/fd.h"
#include "storage/ipc.h"
#include "storage/latch.h"
@@ -709,6 +710,7 @@ pgstat_start(void)
on_exit_reset();
/* Drop our connection to postmaster's shared memory, as well */
+ dsm_detach_all();
PGSharedMemoryDetach();
PgstatCollectorMain(0, NULL);
diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index e277a9a..4731ab7 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -39,6 +39,7 @@
#include "postmaster/fork_process.h"
#include "postmaster/postmaster.h"
#include "postmaster/syslogger.h"
+#include "storage/dsm.h"
#include "storage/ipc.h"
#include "storage/latch.h"
#include "storage/pg_shmem.h"
@@ -626,6 +627,7 @@ SysLogger_Start(void)
on_exit_reset();
/* Drop our connection to postmaster's shared memory, as well */
+ dsm_detach_all();
PGSharedMemoryDetach();
/* do the work */
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 232c099..c967177 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -722,6 +722,8 @@ dsm_attach(dsm_handle h)
/*
* At backend shutdown time, detach any segments that are still attached.
+ * (This is similar to dsm_detach_all, except that there's no reason to
+ * unmap the control segment before exiting, so we don't bother.)
*/
void
dsm_backend_shutdown(void)
@@ -736,6 +738,31 @@ dsm_backend_shutdown(void)
}
/*
+ * Detach all shared memory segments, including the control segments. This
+ * should be called, along with PGSharedMemoryDetach, in processes that
+ * might inherit mappings but are not intended to be connected to dynamic
+ * shared memory.
+ */
+void
+dsm_detach_all(void)
+{
+ void *control_address = dsm_control;
+
+ while (!dlist_is_empty(&dsm_segment_list))
+ {
+ dsm_segment *seg;
+
+ seg = dlist_head_element(dsm_segment, node, &dsm_segment_list);
+ dsm_detach(seg);
+ }
+
+ if (control_address != NULL)
+ dsm_impl_op(DSM_OP_DETACH, dsm_control_handle, 0,
+ &dsm_control_impl_private, &control_address,
+ &dsm_control_mapped_size, ERROR);
+}
+
+/*
* Resize an existing shared memory segment.
*
* This may cause the shared memory segment to be remapped at a different
diff --git a/src/include/storage/dsm.h b/src/include/storage/dsm.h
index 03dd767..e4669a1 100644
--- a/src/include/storage/dsm.h
+++ b/src/include/storage/dsm.h
@@ -20,6 +20,7 @@ typedef struct dsm_segment dsm_segment;
/* Startup and shutdown functions. */
extern void dsm_postmaster_startup(void);
extern void dsm_backend_shutdown(void);
+extern void dsm_detach_all(void);
/* Functions that create, update, or remove mappings. */
extern dsm_segment *dsm_create(Size size);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
