Re: [dpdk-dev] [PATCH] vhost: allow for many vhost user ports

[Jan Wickbom <jan.wickbom@ericsson.com>]

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: den 8 december 2016 06:51
> To: Jan Wickbom <jan.wickbom@ericsson.com>
> Cc: dev@dpdk.org; Patrik Andersson R <patrik.r.andersson@ericsson.com>
> Subject: Re: [PATCH] vhost: allow for many vhost user ports
> 
> On Wed, Dec 07, 2016 at 01:23:48PM +0000, Jan Wickbom wrote:
> > > On Thu, Dec 01, 2016 at 04:26:50PM +0100, Jan Wickbom wrote:
> > > >  static int
> > > > -fdset_fill(fd_set *rfset, fd_set *wfset, struct fdset *pfdset)
> > > > +fdset_fill(struct pollfd *rwfds, struct fdset *pfdset)
> ...
> > > > +		rwfds[i].fd = pfdentry->fd;
> > > > +		rwfds[i].events = pfdentry->rcb ? POLLIN : 0;
> > > > +		rwfds[i].events |= pfdentry->wcb ? POLLOUT :
> > > 0;
> > >
> > > Another thing is we don't have to re-init this rwfds array again
> > > and again. Instead, we could
> > >
> > > - set it up correctly when fdset_add is invoked: set the fd and
> > >   events.
> > >
> > > - reset revents when it's been handled at fdset_event_dispatch().
> > >
> > > - swap with the last one and shrink the array on fd delete
> > >
> > > Could you make a follow up patch for that?
> >
> > I don't see how that could easily be done. The loop index, i, is a direct
> reference between
> > an entry in the rwfds array and an entry in the pfdset array. It should stay
> like that while we are
> > hanging in the poll(). If  an entry in the pfdset array is removed while we
> are hanging in the poll()
> > and we then immediately replaces it with the last entry in the array we will
> end up in trouble if the
> > revent gets set for the "replaced" index. The direct reference is gone.
> > Or am I missing something?
> 
> Yes, we should not shrink the rwfds during the poll, but we could later, at
> the end of the while() loop.

Did something in v3, please have a look.

> 
> Talking about that, you should not invoke fdset_shrink() inside fdset_del(),
> since it could be in the poll stage.

Thanks! Forgot this! Will send a v4

> 
> 	--yliu