Thread (19 messages) 19 messages, 3 authors, 2017-01-12

Re: [PATCH] vhost: allow for many vhost user ports

From: Yuanhan Liu <hidden>
Date: 2016-12-06 06:55:50

On Thu, Dec 01, 2016 at 04:26:50PM +0100, Jan Wickbom wrote:
Currently select() is used to monitor file descriptors for vhostuser
ports. This limits the number of ports possible to create since the
fd number is used as index in the fd_set and we have seen fds > 1023.
Yeah, it's a known issue for a while. Thanks for making a patch to fix
that!
This patch changes select() to poll(). This way we can keep an
packed (pollfd) array for the fds, e.g. as many fds as the size of
the array.

Also see:
http://dpdk.org/ml/archives/dev/2016-April/037024.html
...
+/**
+ * Adjusts the highest index populated in the array of fds
This function only shrinks (but not extends) the fdset array, so why
not naming it to something like "fdset_shrink".
+ * @return
+ *   Index of highest position populated + 1.
I think it's clearer to say "The new size of fdset".

  
quoted hunk ↗ jump to hunk
@@ -189,7 +206,7 @@
 			pfdset->fd[i].fd = -1;
 			pfdset->fd[i].rcb = pfdset->fd[i].wcb = NULL;
 			pfdset->fd[i].dat = NULL;
-			pfdset->num--;
+			(void) fdset_adjust_num(pfdset);
Unncessary cast.
quoted hunk ↗ jump to hunk
 			i = -1;
 		}
 		pthread_mutex_unlock(&pfdset->fd_mutex);
@@ -211,12 +228,12 @@
 
 	pfdset->fd[index].fd = -1;
 	pfdset->fd[index].rcb = pfdset->fd[index].wcb = NULL;
-	pfdset->fd[index].dat = NULL;
-	pfdset->num--;
+	(void) fdset_adjust_num(pfdset);
 
 	pthread_mutex_unlock(&pfdset->fd_mutex);
 }
 
+
 /**
  * This functions runs in infinite blocking loop until there is no fd in
  * pfdset. It calls corresponding r/w handler if there is event on the fd.
@@ -229,44 +246,48 @@
 void
 fdset_event_dispatch(struct fdset *pfdset)
 {
-	fd_set rfds, wfds;
-	int i, maxfds;
+	int i;
 	struct fdentry *pfdentry;
-	int num = MAX_FDS;
+	int numfds;
 	fd_cb rcb, wcb;
 	void *dat;
 	int fd;
 	int remove1, remove2;
 	int ret;
+	int handled;
 
 	if (pfdset == NULL)
 		return;
 
+	struct pollfd * const rwfds =
+		rte_malloc("struct pollfd", MAX_FDS * sizeof(*rwfds), 0);
There is neither a NULL check, nor a free after the usage. Since it's
a fixed size at compile time (MAX_FDS), you might want to define a
static array for that.

	--yliu
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help