From 143be1b59f2f69ce09e69b116f8a02a16e446668 Mon Sep 17 00:00:00 2001 From: William Lallemand Date: Tue, 7 Jan 2025 16:22:17 +0100 Subject: [PATCH] MEDIUM: errors: get rid of shm_open() Since 5ee266b7 ("MINOR: error: simplify startup_logs_init_shm"), the FD of the startup logs is always closed and the HAPROXY_STARTUPLOGS_FD variable is not used anymore. Which means we only need a mmap. Indeed the shm_open() function was only needed to keep the shm between the exec() of the master so we can get the logs stored there after doing the final exec() in wait mode. Since the wait mode doesn't exist anymore and the parsing is done in a worker, we only need to share a memory zone between the master and the worker. This patch removes shm_open() and replace it with a simple mmap(), this way the shared startup-logs become more portable and USE_SHM_OPEN is not required anymore. --- Makefile | 9 +++-- src/errors.c | 96 +++++++-------------------------------------------- src/mworker.c | 5 --- 3 files changed, 17 insertions(+), 93 deletions(-) diff --git a/Makefile b/Makefile index 62d2588f75722..fb9fa9b234750 100644 --- a/Makefile +++ b/Makefile @@ -62,7 +62,6 @@ # USE_MEMORY_PROFILING : enable the memory profiler. Linux-glibc only. # USE_LIBATOMIC : force to link with/without libatomic. Automatic. # USE_PTHREAD_EMULATION : replace pthread's rwlocks with ours -# USE_SHM_OPEN : use shm_open() for the startup-logs # # Options can be forced by specifying "USE_xxx=1" or can be disabled by using # "USE_xxx=" (empty string). The list of enabled and disabled options for a @@ -343,7 +342,7 @@ use_opts = USE_EPOLL USE_KQUEUE USE_NETFILTER USE_POLL \ USE_MATH USE_DEVICEATLAS USE_51DEGREES \ USE_WURFL USE_OBSOLETE_LINKER USE_PRCTL USE_PROCCTL \ USE_THREAD_DUMP USE_EVPORTS USE_OT USE_QUIC USE_PROMEX \ - USE_MEMORY_PROFILING USE_SHM_OPEN \ + USE_MEMORY_PROFILING \ USE_STATIC_PCRE USE_STATIC_PCRE2 \ USE_PCRE USE_PCRE_JIT USE_PCRE2 USE_PCRE2_JIT USE_QUIC_OPENSSL_COMPAT @@ -382,7 +381,7 @@ ifeq ($(TARGET),linux-glibc) USE_POLL USE_TPROXY USE_LIBCRYPT USE_DL USE_RT USE_CRYPT_H USE_NETFILTER \ USE_CPU_AFFINITY USE_THREAD USE_EPOLL USE_LINUX_TPROXY USE_LINUX_CAP \ USE_ACCEPT4 USE_LINUX_SPLICE USE_PRCTL USE_THREAD_DUMP USE_NS USE_TFO \ - USE_GETADDRINFO USE_BACKTRACE USE_SHM_OPEN) + USE_GETADDRINFO USE_BACKTRACE) INSTALL = install -v endif @@ -401,7 +400,7 @@ ifeq ($(TARGET),linux-musl) USE_POLL USE_TPROXY USE_LIBCRYPT USE_DL USE_RT USE_CRYPT_H USE_NETFILTER \ USE_CPU_AFFINITY USE_THREAD USE_EPOLL USE_LINUX_TPROXY USE_LINUX_CAP \ USE_ACCEPT4 USE_LINUX_SPLICE USE_PRCTL USE_THREAD_DUMP USE_NS USE_TFO \ - USE_GETADDRINFO USE_SHM_OPEN) + USE_GETADDRINFO) INSTALL = install -v endif @@ -418,7 +417,7 @@ endif ifeq ($(TARGET),freebsd) set_target_defaults = $(call default_opts, \ USE_POLL USE_TPROXY USE_LIBCRYPT USE_THREAD USE_CPU_AFFINITY USE_KQUEUE \ - USE_ACCEPT4 USE_CLOSEFROM USE_GETADDRINFO USE_PROCCTL USE_SHM_OPEN) + USE_ACCEPT4 USE_CLOSEFROM USE_GETADDRINFO USE_PROCCTL) endif # kFreeBSD glibc diff --git a/src/errors.c b/src/errors.c index 8c508e7af2c5b..98e5126062a9c 100644 --- a/src/errors.c +++ b/src/errors.c @@ -21,9 +21,7 @@ * retrieve on the CLI. */ struct ring *startup_logs = NULL; uint tot_warnings = 0; -#ifdef USE_SHM_OPEN static struct ring *shm_startup_logs = NULL; -#endif /* A thread local buffer used to store all alerts/warnings. It can be used to * retrieve them for CLI commands after startup. @@ -36,71 +34,6 @@ static THREAD_LOCAL struct buffer usermsgs_buf = BUF_NULL; #define USERMSGS_CTX_BUFSIZE PATH_MAX static THREAD_LOCAL struct usermsgs_ctx usermsgs_ctx = { .str = BUF_NULL, }; -#ifdef USE_SHM_OPEN - -/* initialise an SHM for the startup logs and return its fd */ -static int startup_logs_new_shm() -{ - char *path = NULL; - int fd = -1; - int flags; - - /* create a unique path per PID so we don't collide with another - process */ - memprintf(&path, "/haproxy_startup_logs_%d", getpid()); - fd = shm_open(path, O_CREAT | O_RDWR, S_IRUSR | S_IWUSR); - if (fd == -1) - goto error; - shm_unlink(path); - ha_free(&path); - - if (ftruncate(fd, STARTUP_LOG_SIZE) == -1) - goto error; - - flags = fcntl(fd, F_GETFD); - if (flags == -1) - goto error; - flags &= ~FD_CLOEXEC; - flags = fcntl(fd, F_SETFD, flags); - if (flags == -1) - goto error; - - return fd; -error: - if (fd != -1) { - close(fd); - fd = -1; - } - return fd; -} - -/* mmap a startup-logs from a . - * if is set to one, initialize the buffer. - * Returns the ring. - */ -static struct ring *startup_logs_from_fd(int fd, int new) -{ - char *area; - struct ring *r = NULL; - - if (fd == -1) - goto error; - - area = mmap(NULL, STARTUP_LOG_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); - if (area == MAP_FAILED || area == NULL) - goto error; - - r = ring_make_from_area(area, STARTUP_LOG_SIZE, new); - if (r == NULL) - goto error; - - shm_startup_logs = r; /* save the ptr so we can unmap later */ - - return r; -error: - return NULL; -} - /* * At process start (step_init_1) opens shm and allocates the ring area for the * startup logs into it. In master-worker mode master and worker share the same @@ -112,30 +45,29 @@ static struct ring *startup_logs_from_fd(int fd, int new) static struct ring *startup_logs_init_shm() { struct ring *r = NULL; - int fd = -1; + char *area = NULL; - fd = startup_logs_new_shm(); - if (fd == -1) - return NULL; + area = mmap(NULL, STARTUP_LOG_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0); + if (area == MAP_FAILED || area == NULL) + goto error; + + r = ring_make_from_area(area, STARTUP_LOG_SIZE, 1); + if (r == NULL) + goto error; - r = startup_logs_from_fd(fd, 1); - close(fd); + shm_startup_logs = r; /* save the ptr so we can unmap later */ - if (!r) - return NULL; + return r; +error: + if (area != MAP_FAILED && area != NULL) + munmap(area, STARTUP_LOG_SIZE); return r; } -#endif /* ! USE_SHM_OPEN */ - void startup_logs_init() { -#ifdef USE_SHM_OPEN startup_logs = startup_logs_init_shm(); -#else /* ! USE_SHM_OPEN */ - startup_logs = ring_new(STARTUP_LOG_SIZE); -#endif if (startup_logs) vma_set_name(ring_allocated_area(startup_logs), ring_allocated_size(startup_logs), @@ -145,10 +77,8 @@ void startup_logs_init() /* free the startup logs, unmap if it was an shm */ void startup_logs_free(struct ring *r) { -#ifdef USE_SHM_OPEN if (r == shm_startup_logs) munmap(ring_allocated_area(r), STARTUP_LOG_SIZE); -#endif /* ! USE_SHM_OPEN */ ring_free(r); startup_logs = NULL; } diff --git a/src/mworker.c b/src/mworker.c index 2cf1884947c8c..68dd0f8762696 100644 --- a/src/mworker.c +++ b/src/mworker.c @@ -1008,7 +1008,6 @@ static int cli_io_handler_show_loadstatus(struct appctx *appctx) else chunk_printf(&trash, "Success=1\n"); -#ifdef USE_SHM_OPEN if (startup_logs && ring_data(startup_logs) > 1) chunk_appendf(&trash, "--\n"); @@ -1020,10 +1019,6 @@ static int cli_io_handler_show_loadstatus(struct appctx *appctx) ring_attach_cli(startup_logs, appctx, 0); return 0; } -#else - if (applet_putchk(appctx, &trash) == -1) - return 0; -#endif return 1; }