Good point,

I forgot about the rcu.

I will redo the patch as per your suggestions and resubmit it next week.

A.

On 09/07/2021 19:45, Ben Pfaff wrote:
On Fri, Jul 09, 2021 at 06:19:06PM +0100, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov <anton.iva...@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.iva...@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;
          }



--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to