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