Skip to content

Commit

Permalink
MEDIUM: errors: get rid of shm_open()
Browse files Browse the repository at this point in the history
Since 5ee266b ("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.
  • Loading branch information
wlallemand committed Jan 7, 2025
1 parent d7fc90a commit 143be1b
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 93 deletions.
9 changes: 4 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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
Expand Down
96 changes: 13 additions & 83 deletions src/errors.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 <fd>.
* if <new> 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
Expand All @@ -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),
Expand All @@ -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;
}
Expand Down
5 changes: 0 additions & 5 deletions src/mworker.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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;
}

Expand Down

0 comments on commit 143be1b

Please sign in to comment.