From 1673c4a8837b7e90a221c248669942f6f3988211 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 7 Apr 2021 17:36:57 +0200 Subject: [PATCH] MINOR: fd: implement an exclusive syscall bit to remove the ugly "log" lock There is a function called fd_write_frag_line() that's essentially used by loggers and that is used to write an atomic message line over a file descriptor using writev(). However a lock is required around the writev() call to prevent messages from multiple threads from being interleaved. Till now a SPIN_TRYLOCK was used on a dedicated lock that was common to all FDs. This is quite not pretty as if there are multiple output pipes to collect logs, there will be quite some contention. Now that there are empty flags left in the FD state and that we can finally use atomic ops on them, let's add a flag to indicate the FD is locked for exclusive access by a syscall. At least the locking will now be on an FD basis and not the whole process, so we can remove the log_lock. --- include/haproxy/fd-t.h | 2 ++ src/fd.c | 5 ++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/haproxy/fd-t.h b/include/haproxy/fd-t.h index 9108b4ea4..4759ef2c6 100644 --- a/include/haproxy/fd-t.h +++ b/include/haproxy/fd-t.h @@ -68,6 +68,7 @@ enum { #define FD_INITIALIZED_BIT 18 /* init phase was done (e.g. output pipe set non-blocking) */ #define FD_ET_POSSIBLE_BIT 19 /* edge-triggered is possible on this FD */ #define FD_EXPORTED_BIT 20 /* FD is exported and must not be closed */ +#define FD_EXCL_SYSCALL_BIT 21 /* a syscall claims exclusivity on this FD */ /* and flag values */ @@ -107,6 +108,7 @@ enum { #define FD_INITIALIZED (1U << FD_INITIALIZED_BIT) #define FD_ET_POSSIBLE (1U << FD_ET_POSSIBLE_BIT) #define FD_EXPORTED (1U << FD_EXPORTED_BIT) +#define FD_EXCL_SYSCALL (1U << FD_EXCL_SYSCALL_BIT) /* This is the value used to mark a file descriptor as dead. This value is * negative, this is important so that tests on fd < 0 properly match. It diff --git a/src/fd.c b/src/fd.c index d63abbf54..8d671bdc3 100644 --- a/src/fd.c +++ b/src/fd.c @@ -444,7 +444,6 @@ void updt_fd_polling(const int fd) } } -__decl_spinlock(log_lock); /* Tries to send parts from followed by parts from * optionally followed by a newline if is non-null, to file descriptor * . The message is sent atomically using writev(). It may be truncated to @@ -503,7 +502,7 @@ ssize_t fd_write_frag_line(int fd, size_t maxlen, const struct ist pfx[], size_t * up. */ - while (HA_SPIN_TRYLOCK(OTHER_LOCK, &log_lock) != 0) { + while (HA_ATOMIC_BTS(&fdtab[fd].state, FD_EXCL_SYSCALL_BIT)) { if (++attempts >= 200) { /* so that the caller knows the message couldn't be delivered */ sent = -1; @@ -519,7 +518,7 @@ ssize_t fd_write_frag_line(int fd, size_t maxlen, const struct ist pfx[], size_t fcntl(fd, F_SETFL, O_NONBLOCK); } sent = writev(fd, iovec, vec); - HA_SPIN_UNLOCK(OTHER_LOCK, &log_lock); + HA_ATOMIC_BTR(&fdtab[fd].state, FD_EXCL_SYSCALL_BIT); leave: /* sent > 0 if the message was delivered */