Change in libosmocore[master]: select: Migrate over to poll()

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

laforge gerrit-no-reply at lists.osmocom.org
Sun Oct 18 19:37:18 UTC 2020


laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/20732 )


Change subject: select: Migrate over to poll()
......................................................................

select: Migrate over to poll()

select is an ancient interface with weird restrictions, such as
the fact that it cannot be used for file descriptor values > 1024.

This may have been sufficient 40 years ago, but certainly is not in
2020.  I wanted to migrate to epoll(), but unfortunately it doesn't
work well with the fact that existing programs simply set osmo_fd.flags
without making any API calls at the time they change those flags.

So let's do the migration to poll() as a first step, and then consider
epoll() as a second step further down the road, after introducing new
APIs and porting applications over.

The poll() code introduced in this patch is not extremely efficient,
as it needs to do extensive linked list iterations after poll() returns
in order to find the osmo_fd from the fd.  Optimization is possible,
but let's postpone that to a follow-up patch.

Change-Id: I9e80da68a144b36926066610d0d3df06abe09bca
---
M include/osmocom/core/timer.h
M src/select.c
M src/timer.c
3 files changed, 135 insertions(+), 10 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/32/20732/1

diff --git a/include/osmocom/core/timer.h b/include/osmocom/core/timer.h
index 1979766..6ffc3b1 100644
--- a/include/osmocom/core/timer.h
+++ b/include/osmocom/core/timer.h
@@ -84,6 +84,7 @@
  * internal timer list management
  */
 struct timeval *osmo_timers_nearest(void);
+int osmo_timers_nearest_ms(void);
 void osmo_timers_prepare(void);
 int osmo_timers_update(void);
 int osmo_timers_check(void);
diff --git a/src/select.c b/src/select.c
index 496beea..870809f 100644
--- a/src/select.c
+++ b/src/select.c
@@ -4,7 +4,7 @@
  * userspace logging daemon for the iptables ULOG target
  * of the linux 2.4 netfilter subsystem. */
 /*
- * (C) 2000-2009 by Harald Welte <laforge at gnumonks.org>
+ * (C) 2000-2020 by Harald Welte <laforge at gnumonks.org>
  * All Rights Reserverd.
  *
  * SPDX-License-Identifier: GPL-2.0+
@@ -43,6 +43,7 @@
 
 #ifdef HAVE_SYS_SELECT_H
 #include <sys/select.h>
+#include <poll.h>
 
 /*! \addtogroup select
  *  @{
@@ -56,6 +57,19 @@
 static __thread struct llist_head osmo_fds; /* TLS cannot use LLIST_HEAD() */
 static __thread int unregistered_count;
 
+/* g_poll.poll will be allocated in chunks of POLL_ALLOC_GRANULARITY */
+#define POLL_ALLOC_GRANULARITY	1024
+
+struct poll_state {
+	/* array of pollfd */
+	struct pollfd *poll;
+	/* number of entries in pollfd allocated */
+	unsigned int poll_size;
+	/* number of osmo_fd registered */
+	unsigned int num_registered;
+};
+static __thread struct poll_state g_poll;
+
 /*! Set up an osmo-fd. Will not register it.
  *  \param[inout] ofd Osmo FD to be set-up
  *  \param[in] fd OS-level file descriptor number
@@ -91,6 +105,17 @@
 	return false;
 }
 
+struct osmo_fd *osmo_fd_find(int fd)
+{
+	struct osmo_fd *entry;
+	llist_for_each_entry(entry, &osmo_fds, list) {
+		if (entry->fd == fd)
+			return entry;
+	}
+	return NULL;
+}
+
+
 /*! Register a new file descriptor with select loop abstraction
  *  \param[in] fd osmocom file descriptor to be registered
  *  \returns 0 on success; negative in case of error
@@ -127,8 +152,20 @@
 		return 0;
 	}
 #endif
+	if (g_poll.num_registered + 1> g_poll.poll_size) {
+		struct pollfd *p;
+		p = talloc_realloc(OTC_GLOBAL, g_poll.poll, struct pollfd,
+				   g_poll.poll_size + POLL_ALLOC_GRANULARITY);
+		if (!p)
+			return -ENOMEM;
+		memset(p + g_poll.poll_size, 0, POLL_ALLOC_GRANULARITY*sizeof(struct pollfd));
+		g_poll.poll = p;
+		g_poll.poll_size = g_poll.poll_size + POLL_ALLOC_GRANULARITY;
+	}
 
 	llist_add_tail(&fd->list, &osmo_fds);
+	g_poll.num_registered++;
+
 
 	return 0;
 }
@@ -143,6 +180,7 @@
 	 * osmo_fd_is_registered() */
 	unregistered_count++;
 	llist_del(&fd->list);
+	g_poll.num_registered--;
 }
 
 /*! Close a file descriptor, mark it as closed + unregister from select loop abstraction
@@ -237,22 +275,92 @@
 	return work;
 }
 
+
+/* fill g_poll.poll and return the number of entries filled */
+static unsigned int poll_fill_fds(void)
+{
+	struct osmo_fd *ufd;
+	unsigned int i = 0;
+
+	llist_for_each_entry(ufd, &osmo_fds, list) {
+		struct pollfd *p;
+
+		if (!ufd->when)
+			continue;
+
+		OSMO_ASSERT(i < g_poll.poll_size);
+		p = &g_poll.poll[i++];
+
+		p->fd = ufd->fd;
+		p->events = 0;
+		p->revents = 0;
+
+		if (ufd->when & OSMO_FD_READ)
+			p->events |= POLLIN;
+
+		if (ufd->when & OSMO_FD_WRITE)
+			p->events |= POLLOUT;
+
+		if (ufd->when & OSMO_FD_EXCEPT)
+			p->events |= POLLOUT;
+
+	}
+
+	return i;
+}
+
+/* iterate over first n_fd entries of g_poll.lopp + dispatch */
+static int poll_disp_fds(int n_fd)
+{
+	struct osmo_fd *ufd;
+	unsigned int i;
+	int work = 0;
+
+	for (i = 0; i < n_fd; i++) {
+		struct pollfd *p = &g_poll.poll[i];
+		int flags = 0;
+
+		if (!p->revents)
+			continue;
+
+		ufd = osmo_fd_find(p->fd);
+		if (!ufd) {
+			/* FD might have been unregistered meanwhile */
+			continue;
+		}
+		if (p->revents & POLLIN)
+			flags |= OSMO_FD_READ;
+		if (p->revents & POLLOUT)
+			flags |= OSMO_FD_WRITE;
+		if (p->revents & POLLERR)
+			flags |= OSMO_FD_EXCEPT;
+
+		if (flags) {
+			work = 1;
+			/* make sure to clear any log context before processing the next incoming message
+			 * as part of some file descriptor callback.  This effectively prevents "context
+			 * leaking" from processing of one message into processing of the next message as part
+			 * of one iteration through the list of file descriptors here.  See OS#3813 */
+			log_reset_context();
+			ufd->cb(ufd, flags);
+		}
+	}
+
+	return work;
+}
+
 static int _osmo_select_main(int polling)
 {
-	fd_set readset, writeset, exceptset;
+	unsigned int n_poll;
 	int rc;
-	struct timeval no_time = {0, 0};
-
-	FD_ZERO(&readset);
-	FD_ZERO(&writeset);
-	FD_ZERO(&exceptset);
 
 	/* prepare read and write fdsets */
-	osmo_fd_fill_fds(&readset, &writeset, &exceptset);
+	n_poll = poll_fill_fds();
 
 	if (!polling)
 		osmo_timers_prepare();
-	rc = select(maxfd+1, &readset, &writeset, &exceptset, polling ? &no_time : osmo_timers_nearest());
+
+	rc = poll(g_poll.poll, n_poll, polling ? 0 : osmo_timers_nearest_ms());
 	if (rc < 0)
 		return 0;
 
@@ -262,7 +370,7 @@
 	OSMO_ASSERT(osmo_ctx->select);
 
 	/* call registered callback functions */
-	return osmo_fd_disp_fds(&readset, &writeset, &exceptset);
+	return poll_disp_fds(n_poll);
 }
 
 /*! select main loop integration
diff --git a/src/timer.c b/src/timer.c
index d3129a7..bcd9f5b 100644
--- a/src/timer.c
+++ b/src/timer.c
@@ -184,6 +184,22 @@
 	return nearest_p;
 }
 
+/*! Determine time between now and the nearest timer in milliseconds
+ *  \returns number of milliseconds until nearest timer expires; -1 if no timers pending
+ */
+int osmo_timers_nearest_ms(void)
+{
+	int nearest_ms;
+
+	if (!nearest_p)
+		return -1;
+
+	nearest_ms = nearest_p->tv_sec * 1000;
+	nearest_ms += nearest_p->tv_usec / 1000;
+
+	return nearest_ms;
+}
+
 static void update_nearest(struct timeval *cand, struct timeval *current)
 {
 	if (cand->tv_sec != LONG_MAX) {

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/20732
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I9e80da68a144b36926066610d0d3df06abe09bca
Gerrit-Change-Number: 20732
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201018/de5c9088/attachment.htm>


More information about the gerrit-log mailing list