<p>laforge has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/20732">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">select: Migrate over to poll()<br><br>select is an ancient interface with weird restrictions, such as<br>the fact that it cannot be used for file descriptor values > 1024.<br><br>This may have been sufficient 40 years ago, but certainly is not in<br>2020. I wanted to migrate to epoll(), but unfortunately it doesn't<br>work well with the fact that existing programs simply set osmo_fd.flags<br>without making any API calls at the time they change those flags.<br><br>So let's do the migration to poll() as a first step, and then consider<br>epoll() as a second step further down the road, after introducing new<br>APIs and porting applications over.<br><br>The poll() code introduced in this patch is not extremely efficient,<br>as it needs to do extensive linked list iterations after poll() returns<br>in order to find the osmo_fd from the fd. Optimization is possible,<br>but let's postpone that to a follow-up patch.<br><br>Change-Id: I9e80da68a144b36926066610d0d3df06abe09bca<br>---<br>M include/osmocom/core/timer.h<br>M src/select.c<br>M src/timer.c<br>3 files changed, 135 insertions(+), 10 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/32/20732/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/core/timer.h b/include/osmocom/core/timer.h</span><br><span>index 1979766..6ffc3b1 100644</span><br><span>--- a/include/osmocom/core/timer.h</span><br><span>+++ b/include/osmocom/core/timer.h</span><br><span>@@ -84,6 +84,7 @@</span><br><span> * internal timer list management</span><br><span> */</span><br><span> struct timeval *osmo_timers_nearest(void);</span><br><span style="color: hsl(120, 100%, 40%);">+int osmo_timers_nearest_ms(void);</span><br><span> void osmo_timers_prepare(void);</span><br><span> int osmo_timers_update(void);</span><br><span> int osmo_timers_check(void);</span><br><span>diff --git a/src/select.c b/src/select.c</span><br><span>index 496beea..870809f 100644</span><br><span>--- a/src/select.c</span><br><span>+++ b/src/select.c</span><br><span>@@ -4,7 +4,7 @@</span><br><span> * userspace logging daemon for the iptables ULOG target</span><br><span> * of the linux 2.4 netfilter subsystem. */</span><br><span> /*</span><br><span style="color: hsl(0, 100%, 40%);">- * (C) 2000-2009 by Harald Welte <laforge@gnumonks.org></span><br><span style="color: hsl(120, 100%, 40%);">+ * (C) 2000-2020 by Harald Welte <laforge@gnumonks.org></span><br><span> * All Rights Reserverd.</span><br><span> *</span><br><span> * SPDX-License-Identifier: GPL-2.0+</span><br><span>@@ -43,6 +43,7 @@</span><br><span> </span><br><span> #ifdef HAVE_SYS_SELECT_H</span><br><span> #include <sys/select.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <poll.h></span><br><span> </span><br><span> /*! \addtogroup select</span><br><span> * @{</span><br><span>@@ -56,6 +57,19 @@</span><br><span> static __thread struct llist_head osmo_fds; /* TLS cannot use LLIST_HEAD() */</span><br><span> static __thread int unregistered_count;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* g_poll.poll will be allocated in chunks of POLL_ALLOC_GRANULARITY */</span><br><span style="color: hsl(120, 100%, 40%);">+#define POLL_ALLOC_GRANULARITY 1024</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+struct poll_state {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* array of pollfd */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct pollfd *poll;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* number of entries in pollfd allocated */</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int poll_size;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* number of osmo_fd registered */</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int num_registered;</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span style="color: hsl(120, 100%, 40%);">+static __thread struct poll_state g_poll;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! Set up an osmo-fd. Will not register it.</span><br><span> * \param[inout] ofd Osmo FD to be set-up</span><br><span> * \param[in] fd OS-level file descriptor number</span><br><span>@@ -91,6 +105,17 @@</span><br><span> return false;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+struct osmo_fd *osmo_fd_find(int fd)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_fd *entry;</span><br><span style="color: hsl(120, 100%, 40%);">+ llist_for_each_entry(entry, &osmo_fds, list) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (entry->fd == fd)</span><br><span style="color: hsl(120, 100%, 40%);">+ return entry;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! Register a new file descriptor with select loop abstraction</span><br><span> * \param[in] fd osmocom file descriptor to be registered</span><br><span> * \returns 0 on success; negative in case of error</span><br><span>@@ -127,8 +152,20 @@</span><br><span> return 0;</span><br><span> }</span><br><span> #endif</span><br><span style="color: hsl(120, 100%, 40%);">+ if (g_poll.num_registered + 1> g_poll.poll_size) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct pollfd *p;</span><br><span style="color: hsl(120, 100%, 40%);">+ p = talloc_realloc(OTC_GLOBAL, g_poll.poll, struct pollfd,</span><br><span style="color: hsl(120, 100%, 40%);">+ g_poll.poll_size + POLL_ALLOC_GRANULARITY);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!p)</span><br><span style="color: hsl(120, 100%, 40%);">+ return -ENOMEM;</span><br><span style="color: hsl(120, 100%, 40%);">+ memset(p + g_poll.poll_size, 0, POLL_ALLOC_GRANULARITY*sizeof(struct pollfd));</span><br><span style="color: hsl(120, 100%, 40%);">+ g_poll.poll = p;</span><br><span style="color: hsl(120, 100%, 40%);">+ g_poll.poll_size = g_poll.poll_size + POLL_ALLOC_GRANULARITY;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> llist_add_tail(&fd->list, &osmo_fds);</span><br><span style="color: hsl(120, 100%, 40%);">+ g_poll.num_registered++;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> </span><br><span> return 0;</span><br><span> }</span><br><span>@@ -143,6 +180,7 @@</span><br><span> * osmo_fd_is_registered() */</span><br><span> unregistered_count++;</span><br><span> llist_del(&fd->list);</span><br><span style="color: hsl(120, 100%, 40%);">+ g_poll.num_registered--;</span><br><span> }</span><br><span> </span><br><span> /*! Close a file descriptor, mark it as closed + unregister from select loop abstraction</span><br><span>@@ -237,22 +275,92 @@</span><br><span> return work;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* fill g_poll.poll and return the number of entries filled */</span><br><span style="color: hsl(120, 100%, 40%);">+static unsigned int poll_fill_fds(void)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_fd *ufd;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int i = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ llist_for_each_entry(ufd, &osmo_fds, list) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct pollfd *p;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ufd->when)</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(i < g_poll.poll_size);</span><br><span style="color: hsl(120, 100%, 40%);">+ p = &g_poll.poll[i++];</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ p->fd = ufd->fd;</span><br><span style="color: hsl(120, 100%, 40%);">+ p->events = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ p->revents = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ufd->when & OSMO_FD_READ)</span><br><span style="color: hsl(120, 100%, 40%);">+ p->events |= POLLIN;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ufd->when & OSMO_FD_WRITE)</span><br><span style="color: hsl(120, 100%, 40%);">+ p->events |= POLLOUT;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ufd->when & OSMO_FD_EXCEPT)</span><br><span style="color: hsl(120, 100%, 40%);">+ p->events |= POLLOUT;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return i;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* iterate over first n_fd entries of g_poll.lopp + dispatch */</span><br><span style="color: hsl(120, 100%, 40%);">+static int poll_disp_fds(int n_fd)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_fd *ufd;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int i;</span><br><span style="color: hsl(120, 100%, 40%);">+ int work = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ for (i = 0; i < n_fd; i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct pollfd *p = &g_poll.poll[i];</span><br><span style="color: hsl(120, 100%, 40%);">+ int flags = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!p->revents)</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ufd = osmo_fd_find(p->fd);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ufd) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* FD might have been unregistered meanwhile */</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (p->revents & POLLIN)</span><br><span style="color: hsl(120, 100%, 40%);">+ flags |= OSMO_FD_READ;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (p->revents & POLLOUT)</span><br><span style="color: hsl(120, 100%, 40%);">+ flags |= OSMO_FD_WRITE;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (p->revents & POLLERR)</span><br><span style="color: hsl(120, 100%, 40%);">+ flags |= OSMO_FD_EXCEPT;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (flags) {</span><br><span style="color: hsl(120, 100%, 40%);">+ work = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* make sure to clear any log context before processing the next incoming message</span><br><span style="color: hsl(120, 100%, 40%);">+ * as part of some file descriptor callback. This effectively prevents "context</span><br><span style="color: hsl(120, 100%, 40%);">+ * leaking" from processing of one message into processing of the next message as part</span><br><span style="color: hsl(120, 100%, 40%);">+ * of one iteration through the list of file descriptors here. See OS#3813 */</span><br><span style="color: hsl(120, 100%, 40%);">+ log_reset_context();</span><br><span style="color: hsl(120, 100%, 40%);">+ ufd->cb(ufd, flags);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return work;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static int _osmo_select_main(int polling)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- fd_set readset, writeset, exceptset;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int n_poll;</span><br><span> int rc;</span><br><span style="color: hsl(0, 100%, 40%);">- struct timeval no_time = {0, 0};</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- FD_ZERO(&readset);</span><br><span style="color: hsl(0, 100%, 40%);">- FD_ZERO(&writeset);</span><br><span style="color: hsl(0, 100%, 40%);">- FD_ZERO(&exceptset);</span><br><span> </span><br><span> /* prepare read and write fdsets */</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_fd_fill_fds(&readset, &writeset, &exceptset);</span><br><span style="color: hsl(120, 100%, 40%);">+ n_poll = poll_fill_fds();</span><br><span> </span><br><span> if (!polling)</span><br><span> osmo_timers_prepare();</span><br><span style="color: hsl(0, 100%, 40%);">- rc = select(maxfd+1, &readset, &writeset, &exceptset, polling ? &no_time : osmo_timers_nearest());</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = poll(g_poll.poll, n_poll, polling ? 0 : osmo_timers_nearest_ms());</span><br><span> if (rc < 0)</span><br><span> return 0;</span><br><span> </span><br><span>@@ -262,7 +370,7 @@</span><br><span> OSMO_ASSERT(osmo_ctx->select);</span><br><span> </span><br><span> /* call registered callback functions */</span><br><span style="color: hsl(0, 100%, 40%);">- return osmo_fd_disp_fds(&readset, &writeset, &exceptset);</span><br><span style="color: hsl(120, 100%, 40%);">+ return poll_disp_fds(n_poll);</span><br><span> }</span><br><span> </span><br><span> /*! select main loop integration</span><br><span>diff --git a/src/timer.c b/src/timer.c</span><br><span>index d3129a7..bcd9f5b 100644</span><br><span>--- a/src/timer.c</span><br><span>+++ b/src/timer.c</span><br><span>@@ -184,6 +184,22 @@</span><br><span> return nearest_p;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! Determine time between now and the nearest timer in milliseconds</span><br><span style="color: hsl(120, 100%, 40%);">+ * \returns number of milliseconds until nearest timer expires; -1 if no timers pending</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+int osmo_timers_nearest_ms(void)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ int nearest_ms;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!nearest_p)</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ nearest_ms = nearest_p->tv_sec * 1000;</span><br><span style="color: hsl(120, 100%, 40%);">+ nearest_ms += nearest_p->tv_usec / 1000;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return nearest_ms;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static void update_nearest(struct timeval *cand, struct timeval *current)</span><br><span> {</span><br><span> if (cand->tv_sec != LONG_MAX) {</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/20732">change 20732</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmocore/+/20732"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I9e80da68a144b36926066610d0d3df06abe09bca </div>
<div style="display:none"> Gerrit-Change-Number: 20732 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>