Re: [PATCH v4 7/7] passt: Move main event loop processing into passt_worker()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3228 bytes --]

On Wed, Oct 22, 2025 at 08:49:38AM +0200, Laurent Vivier wrote:
> On 22/10/2025 02:53, David Gibson wrote:
> > On Tue, Oct 21, 2025 at 10:00:58AM +0200, Laurent Vivier wrote:
> > > On 20/10/2025 03:43, David Gibson wrote:
> > > > On Fri, Oct 17, 2025 at 12:31:29PM +0200, Laurent Vivier wrote:
> > > > > Extract the epoll event processing logic from main() into a separate
> > > > > passt_worker() function. This refactoring prepares the code for future
> > > > > threading support where passt_worker() will be called as a worker thread
> > > > > callback.
> > > > > 
> > > > > The new function handles:
> > > > > - Processing epoll events and dispatching to protocol handlers
> > > > > - Event statistics tracking and printing
> > > > > - Post-handler periodic tasks (timers, deferred work)
> > > > > - Migration handling
> > > > > 
> > > > > No functional changes, purely a code restructuring.
> > > > > 
> > > > > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > > > 
> > > > Looks good as far as it goes, and I've though often in the past that
> > > > it would make more sense for the "engine" to go in its own function.
> > > > 
> > > > Wondering if it would make more sense to include the epoll_wait()
> > > > itself and the loop in this function, rather than leaving that
> > > > outside.
> > > > 
> > > 
> > > When I introduce the multithreading and the multiqueue, as the thread is
> > > driven by the epollfd, the events are managed by the multiqueue part and the
> > > epollfd by the multithread part.
> > > 
> > > The "threading" worker is:
> > > 
> > > static void *threading_worker(void *opaque)
> > > {
> > >          struct threading_context *tc = opaque;
> > > 
> > >          while (true) {
> > >                  struct epoll_event events[NUM_EPOLL_EVENTS];
> > >                  int nfds;
> > > 
> > >                  nfds = epoll_wait(tc->epollfd, events, NUM_EPOLL_EVENTS,
> > >                                    TIMER_INTERVAL);
> > >                  if (nfds == -1 && errno != EINTR)
> > >                          die_perror("epoll_wait() failed");
> > > 
> > >                  tc->worker(tc->opaque, nfds, events);
> > 
> > IIUC the point here is that eventually the epoll_wait() will be
> > common, but the worker might be different for different threads.  Is
> > that correct?
> > 
> 
> Yes, we can have the passt_worker() for all threads, but we can also have a
> specific worker for passt main (with netlink, listen, ...), a specific for
> TX vhost-user (with kickfd), and another one for RX (with all the ICMP, UDP,
> TCP sockets).

Ok.  But do the different workers need to be different in their code,
rather than just what fds end up in their epoll pool?  I can see that
there might be some worthwhile improvements to be had by specialising
the worker functions.  On the other hand, if we keep the worker loop
generic, it would make it easier to refine our workload balancing by
moving different fds between the threads.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]