From 59203c51cc6a9676ef1ab42aebe070a55f55ead2 Mon Sep 17 00:00:00 2001 From: Sean Bright Date: Mon, 17 Apr 2017 20:06:10 -0400 Subject: core: Use eventfd for alert pipes on Linux when possible The primary win of switching to eventfd when possible is that it only uses a single file descriptor while pipe() will use two. This means for each bridge channel we're reducing the number of required file descriptors by 1, and - if you're using timerfd - we also now have 1 less file descriptor per Asterisk channel. The API is not ideal (passing int arrays), but this is the cleanest approach I could come up with to maintain API/ABI. I've also removed what I believe to be an erroneous code block that checked the non-blocking flag on the pipe ends for each read. If the file descriptor is 'losing' its non-blocking mode, it is because of a bug somewhere else in our code. In my testing I haven't seen any measurable difference in performance. Change-Id: Iff0fb1573e7f7a187d5211ddc60aa8f3da3edb1d --- main/bridge_channel.c | 87 ++++----------------------------------------------- 1 file changed, 6 insertions(+), 81 deletions(-) (limited to 'main/bridge_channel.c') diff --git a/main/bridge_channel.c b/main/bridge_channel.c index 7f3c8fe31..89222d365 100644 --- a/main/bridge_channel.c +++ b/main/bridge_channel.c @@ -35,6 +35,7 @@ #include #include "asterisk/heap.h" +#include "asterisk/alertpipe.h" #include "asterisk/astobj2.h" #include "asterisk/stringfields.h" #include "asterisk/app.h" @@ -954,7 +955,6 @@ static void bridge_frame_free(struct ast_frame *frame) int ast_bridge_channel_queue_frame(struct ast_bridge_channel *bridge_channel, struct ast_frame *fr) { struct ast_frame *dup; - char nudge = 0; if (bridge_channel->suspended /* Also defer DTMF frames. */ @@ -983,7 +983,7 @@ int ast_bridge_channel_queue_frame(struct ast_bridge_channel *bridge_channel, st } AST_LIST_INSERT_TAIL(&bridge_channel->wr_queue, dup, frame_list); - if (write(bridge_channel->alert_pipe[1], &nudge, sizeof(nudge)) != sizeof(nudge)) { + if (ast_alertpipe_write(bridge_channel->alert_pipe)) { ast_log(LOG_ERROR, "We couldn't write alert pipe for %p(%s)... something is VERY wrong\n", bridge_channel, ast_channel_name(bridge_channel->chan)); } @@ -2255,25 +2255,6 @@ static void bridge_channel_handle_control(struct ast_bridge_channel *bridge_chan } } -/*! - * \internal - * \param bridge_channel Channel to read wr_queue alert pipe. - * - * \return Nothing - */ -static void bridge_channel_read_wr_queue_alert(struct ast_bridge_channel *bridge_channel) -{ - char nudge; - - if (read(bridge_channel->alert_pipe[0], &nudge, sizeof(nudge)) < 0) { - if (errno != EINTR && errno != EAGAIN) { - ast_log(LOG_WARNING, "read() failed for alert pipe on %p(%s): %s\n", - bridge_channel, ast_channel_name(bridge_channel->chan), - strerror(errno)); - } - } -} - /*! * \internal * \brief Handle bridge channel write frame to channel. @@ -2296,7 +2277,7 @@ static void bridge_channel_handle_write(struct ast_bridge_channel *bridge_channe /* No frame, flush the alert pipe of excess alerts. */ ast_log(LOG_WARNING, "Weird. No frame from bridge for %s to process?\n", ast_channel_name(bridge_channel->chan)); - bridge_channel_read_wr_queue_alert(bridge_channel); + ast_alertpipe_read(bridge_channel->alert_pipe); ast_bridge_channel_unlock(bridge_channel); return; } @@ -2312,7 +2293,7 @@ static void bridge_channel_handle_write(struct ast_bridge_channel *bridge_channe break; } } - bridge_channel_read_wr_queue_alert(bridge_channel); + ast_alertpipe_read(bridge_channel->alert_pipe); AST_LIST_REMOVE_CURRENT(frame_list); break; } @@ -2879,62 +2860,6 @@ int bridge_channel_internal_allows_optimization(struct ast_bridge_channel *bridg && AST_LIST_EMPTY(&bridge_channel->wr_queue); } -/*! - * \internal - * \brief Close a pipe. - * \since 12.0.0 - * - * \param my_pipe What to close. - * - * \return Nothing - */ -static void pipe_close(int *my_pipe) -{ - if (my_pipe[0] > -1) { - close(my_pipe[0]); - my_pipe[0] = -1; - } - if (my_pipe[1] > -1) { - close(my_pipe[1]); - my_pipe[1] = -1; - } -} - -/*! - * \internal - * \brief Initialize a pipe as non-blocking. - * \since 12.0.0 - * - * \param my_pipe What to initialize. - * - * \retval 0 on success. - * \retval -1 on error. - */ -static int pipe_init_nonblock(int *my_pipe) -{ - int flags; - - my_pipe[0] = -1; - my_pipe[1] = -1; - if (pipe(my_pipe)) { - ast_log(LOG_WARNING, "Can't create pipe! Try increasing max file descriptors with ulimit -n\n"); - return -1; - } - flags = fcntl(my_pipe[0], F_GETFL); - if (fcntl(my_pipe[0], F_SETFL, flags | O_NONBLOCK) < 0) { - ast_log(LOG_WARNING, "Unable to set read pipe nonblocking! (%d: %s)\n", - errno, strerror(errno)); - return -1; - } - flags = fcntl(my_pipe[1], F_GETFL); - if (fcntl(my_pipe[1], F_SETFL, flags | O_NONBLOCK) < 0) { - ast_log(LOG_WARNING, "Unable to set write pipe nonblocking! (%d: %s)\n", - errno, strerror(errno)); - return -1; - } - return 0; -} - /* Destroy elements of the bridge channel structure and the bridge channel structure itself */ static void bridge_channel_destroy(void *obj) { @@ -2954,7 +2879,7 @@ static void bridge_channel_destroy(void *obj) while ((fr = AST_LIST_REMOVE_HEAD(&bridge_channel->wr_queue, frame_list))) { bridge_frame_free(fr); } - pipe_close(bridge_channel->alert_pipe); + ast_alertpipe_close(bridge_channel->alert_pipe); ast_cond_destroy(&bridge_channel->cond); @@ -2971,7 +2896,7 @@ struct ast_bridge_channel *bridge_channel_internal_alloc(struct ast_bridge *brid return NULL; } ast_cond_init(&bridge_channel->cond, NULL); - if (pipe_init_nonblock(bridge_channel->alert_pipe)) { + if (ast_alertpipe_init(bridge_channel->alert_pipe)) { ao2_ref(bridge_channel, -1); return NULL; } -- cgit v1.2.3