On 02/08/2021 23:53, Gaëtan Rivet wrote:
On Tue, Jul 20, 2021, at 17:06, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov <anton.iva...@cambridgegreys.com>
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 <anton.iva...@cambridgegreys.com>
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?
Nope.
The other patch in these series has some effect on the standard OVN scale
ovn-heater benchmark.
This one does not result in noticeable difference on end-to-end benchmarks.
It is still the right thing to do (IMHO).
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.
Thanks, will update accordingly in the next version.
--
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
