From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 63A5D2BCD for ; Tue, 6 Dec 2016 07:55:50 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga104.jf.intel.com with ESMTP; 05 Dec 2016 22:55:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,751,1477983600"; d="scan'208";a="199449535" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga004.fm.intel.com with ESMTP; 05 Dec 2016 22:55:48 -0800 Date: Tue, 6 Dec 2016 14:56:33 +0800 From: Yuanhan Liu To: Jan Wickbom Cc: huawei.xie@intel.com, dev@dpdk.org, patrik.r.andersson@ericsson.com Message-ID: <20161206065633.GQ24403@yliu-dev.sh.intel.com> References: <1480606010-6132-1-git-send-email-jan.wickbom@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1480606010-6132-1-git-send-email-jan.wickbom@ericsson.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] vhost: allow for many vhost user ports X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Dec 2016 06:55:51 -0000 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". > @@ -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. > 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