[ovs-dev] [PATCH] Optimize the poll loop for poll_immediate_wake()

[Ben Pfaff]

On Fri, Jul 09, 2021 at 06:19:06PM +0100, anton.ivanov at cambridgegreys.com wrote:
> From: Anton Ivanov <anton.ivanov at cambridgegreys.com>
> 
> If we are not obtaining any useful information out of the poll(),
> such as is a fd busy or not, we do not need to do a poll() if
> an immediate_wake() has been requested.
> 
> This cuts out all the pollfd hash additions, forming the poll
> arguments and the actual poll() after a call to
> poll_immediate_wake()
> 
> Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com>

Thanks for the patch.

I think that this will have some undesirable side effects because it
avoids calling time_poll() if the wakeup should happen immediately, and
time_poll() does some thing that we always want to happen between one
main loop and another.  In particular the following calls from
time_poll() are important:

    coverage_clear();
    coverage_run();
    if (*last_wakeup && !thread_is_pmd()) {
        log_poll_interval(*last_wakeup);
    }

...

            if (!time_left) {
                ovsrcu_quiesce();
            } else {
                ovsrcu_quiesce_start();
            }

...

        if (deadline <= time_msec()) {
#ifndef _WIN32
            fatal_signal_handler(SIGALRM);
#else
            VLOG_ERR("wake up from WaitForMultipleObjects after deadline");
            fatal_signal_handler(SIGTERM);
#endif
            if (retval < 0) {
                retval = 0;
            }
            break;
        }

...

    *last_wakeup = time_msec();
    refresh_rusage();

Instead of this change, I'd suggest something more like the following,
along with the changes you made to suppress the file descriptors if the
timeout is already zero:

diff --git a/lib/timeval.c b/lib/timeval.c
index 193c7bab1781..f080a9999742 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -323,7 +323,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles OVS_UNUSED,
         }
 
 #ifndef _WIN32
-        retval = poll(pollfds, n_pollfds, time_left);
+        retval = time_left ? poll(pollfds, n_pollfds, time_left) : 0;
         if (retval < 0) {
             retval = -errno;
         }