On Tue, Jul 20, 2021, at 17:06, [email protected] wrote:
> From: Anton Ivanov <[email protected]>
> 
> time_poll() makes an excessive number of time_msec() calls
> which incur a performance penalty.
> 
> 1. Avoid time_msec() call for timeout calculation when time_poll()
> is asked to skip poll()
> 
> 2. Reuse the time_msec() result from deadline calculation for
> last_wakeup and timeout calculation.
> 
> Signed-off-by: Anton Ivanov <[email protected]>

Hello Anton,

Clock_gettime is implemented as a loop reading a cached value in VDSO.
It is updated periodically, and the loop will only be slow if the
value is being updated.

(cf. https://elixir.bootlin.com/linux/latest/source/lib/vdso/gettimeofday.c#L110
Though this is only part of it)

Most of the time this call should be fast.
That being said, it will result in added coherency traffic (memory barrier +
atomic reads). This is still added noise to the system, but it should not
be visible when compared to syscalls in the fastpath.

Were you able to measure an improvement from this patch?

Otherwise I have a nit below.

> ---
>  lib/timeval.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/timeval.c b/lib/timeval.c
> index ef09a15e0..d8dd959cf 100644
> --- a/lib/timeval.c
> +++ b/lib/timeval.c
> @@ -287,7 +287,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, 
> HANDLE *handles OVS_UNUSED,
>            long long int timeout_when, int *elapsed)
>  {
>      long long int *last_wakeup = last_wakeup_get();
> -    long long int start;
> +    long long int start, now;
>      bool quiescent;
>      int retval = 0;
>  
> @@ -297,13 +297,12 @@ time_poll(struct pollfd *pollfds, int n_pollfds, 
> HANDLE *handles OVS_UNUSED,
>      if (*last_wakeup && !thread_is_pmd()) {
>          log_poll_interval(*last_wakeup);
>      }
> -    start = time_msec();
> +    now = start = time_msec();
>  
>      timeout_when = MIN(timeout_when, deadline);
>      quiescent = ovsrcu_is_quiescent();
>  
>      for (;;) {
> -        long long int now = time_msec();
>          int time_left;
>          retval = 0;
>  
> @@ -331,6 +330,8 @@ time_poll(struct pollfd *pollfds, int n_pollfds, 
> HANDLE *handles OVS_UNUSED,
>           */
>          if (timeout_when != LLONG_MIN) {
>              retval = poll(pollfds, n_pollfds, time_left);
> +        } else {
> +            retval = 0;

In the previous patch, retval was already set to 0 earlier in the loop.
This chunk seems to be a remnant from an earlier version.

-- 
Gaetan Rivet
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to