changeset 5578:57e4bf2cc9e7 draft

eloop: Allow eloop to process all fds returned from poll(2) We do this by ensuring the events list or pollfd struct storage is not modified during the revent processing. An event with a fd of -1 means it's been deleted and one without a pollfd struct reference has been newly added. This also allows us to count down the number of fd's that returned a revent so we can break the loop early if possible. This is a really minor optimisation that at best only applies if more than one revent is returned via poll(2). In the case on dhcpcd on NetBSD with privsep, the number of fd's is really low. And on other platforms or without privsep it's low also (just not as low). It's only when you run dhcpcd per interface that the number of fd's starts to creep upwards as you then need one per address dhcpcd is monitoring (as well as the ARP listener per IPv4 address for non NetBSD). However, I use eloop in other code where this could be a good saving and dhcpcd is where the master version of this lives!
author Roy Marples <roy@marples.name>
date Sun, 24 Jan 2021 22:22:25 +0000
parents 4da45107d87a
children 22d473eabfcc
files src/eloop.c
diffstat 1 files changed, 39 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/src/eloop.c	Mon Jan 18 11:31:05 2021 +0000
+++ b/src/eloop.c	Sun Jan 24 22:22:25 2021 +0000
@@ -32,6 +32,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <poll.h>
+#include <stdbool.h>
 #include <signal.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -136,6 +137,7 @@
 	TAILQ_HEAD (event_head, eloop_event) events;
 	size_t nevents;
 	struct event_head free_events;
+	bool events_need_setup;
 
 	struct timespec now;
 	TAILQ_HEAD (timeout_head, eloop_timeout) timeouts;
@@ -282,11 +284,16 @@
 static void
 eloop_event_setup_fds(struct eloop *eloop)
 {
-	struct eloop_event *e;
+	struct eloop_event *e, *ne;
 	struct pollfd *pfd;
 
 	pfd = eloop->fds;
-	TAILQ_FOREACH(e, &eloop->events, next) {
+	TAILQ_FOREACH_SAFE(e, &eloop->events, next, ne) {
+		if (e->fd == -1) {
+			TAILQ_REMOVE(&eloop->events, e, next);
+			TAILQ_INSERT_TAIL(&eloop->free_events, e, next);
+			continue;
+		}
 #ifdef ELOOP_DEBUG
 		fprintf(stderr, "%s(%d) fd=%d, rcb=%p, wcb=%p\n",
 		    __func__, getpid(), e->fd, e->read_cb, e->write_cb);
@@ -301,6 +308,7 @@
 		pfd->revents = 0;
 		pfd++;
 	}
+	eloop->events_need_setup = false;
 }
 
 size_t
@@ -368,7 +376,8 @@
 	}
 
 setup:
-	eloop_event_setup_fds(eloop);
+	e->pollfd = NULL;
+	eloop->events_need_setup = true;
 	return 0;
 }
 
@@ -394,6 +403,10 @@
 	struct eloop_event *e;
 
 	assert(eloop != NULL);
+	if (fd == -1) {
+		errno = EINVAL;
+		return -1;
+	}
 
 	TAILQ_FOREACH(e, &eloop->events, next) {
 		if (e->fd == fd)
@@ -409,16 +422,17 @@
 			goto remove;
 		e->write_cb = NULL;
 		e->write_cb_arg = NULL;
-		goto done;
+		if (e->pollfd != NULL) {
+			e->pollfd->events &= ~POLLOUT;
+			e->pollfd->revents &= ~POLLOUT;
+		}
+		return 1;
 	}
 
 remove:
-	TAILQ_REMOVE(&eloop->events, e, next);
-	TAILQ_INSERT_TAIL(&eloop->free_events, e, next);
+	e->fd = -1;
 	eloop->nevents--;
-
-done:
-	eloop_event_setup_fds(eloop);
+	eloop->events_need_setup = true;
 	return 1;
 }
 
@@ -736,6 +750,9 @@
 		} else
 			tsp = NULL;
 
+		if (eloop->events_need_setup)
+			eloop_event_setup_fds(eloop);
+
 		n = ppoll(eloop->fds, (nfds_t)eloop->nevents, tsp, signals);
 		if (n == -1) {
 			if (errno == EINTR)
@@ -746,18 +763,23 @@
 			continue;
 
 		TAILQ_FOREACH(e, &eloop->events, next) {
-			if (e->pollfd->revents & POLLOUT) {
-				if (e->write_cb != NULL) {
+			/* Skip freshly added events */
+			if (e->pollfd == NULL)
+				continue;
+			if (e->pollfd->revents)
+				n--;
+			if (e->fd != -1 && e->pollfd->revents & POLLOUT) {
+				if (e->write_cb != NULL)
 					e->write_cb(e->write_cb_arg);
-					break;
-				}
 			}
-			if (e->pollfd->revents) {
-				if (e->read_cb != NULL) {
+			if (e->fd != -1 &&
+			    e->pollfd != NULL && e->pollfd->revents)
+			{
+				if (e->read_cb != NULL)
 					e->read_cb(e->read_cb_arg);
-					break;
-				}
 			}
+			if (n == 0)
+				break;
 		}
 	}