diff mbox series

[ovs-dev,v13,11/12] dpdk: Cache result of CPU ISA checks.

Message ID 20210617161825.94741-12-cian.ferriter@intel.com
State Changes Requested
Headers show
Series DPIF Framework + Optimizations | expand

Commit Message

Ferriter, Cian June 17, 2021, 4:18 p.m. UTC
From: Harry van Haaren <harry.van.haaren@intel.com>

As a small optimization, this patch caches the result of a CPU ISA
check from DPDK. Particularly in the case of running the DPCLS
autovalidator (which repeatedly probes subtables) this reduces
the amount of CPU ISA lookups from the DPDK level.

By caching them at the OVS/dpdk.c level, the ISA checks remain
runtime for the CPU where they are executed, but subsequent checks
for the same ISA feature become much cheaper.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
Co-authored-by: Cian Ferriter <cian.ferriter@intel.com>
Signed-off-by: Cian Ferriter <cian.ferriter@intel.com>
---
 lib/dpdk.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

Comments

Flavio Leitner June 24, 2021, 3:47 a.m. UTC | #1
On Thu, Jun 17, 2021 at 05:18:24PM +0100, Cian Ferriter wrote:
> From: Harry van Haaren <harry.van.haaren@intel.com>
> 
> As a small optimization, this patch caches the result of a CPU ISA
> check from DPDK. Particularly in the case of running the DPCLS
> autovalidator (which repeatedly probes subtables) this reduces
> the amount of CPU ISA lookups from the DPDK level.
> 
> By caching them at the OVS/dpdk.c level, the ISA checks remain
> runtime for the CPU where they are executed, but subsequent checks
> for the same ISA feature become much cheaper.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> Co-authored-by: Cian Ferriter <cian.ferriter@intel.com>
> Signed-off-by: Cian Ferriter <cian.ferriter@intel.com>
> ---

The current approach uses a static int8_t per CPU flag.  Perhaps
using two static int8_t (one for DONE and another for AVAIL) and
then use a bit on them for each CPU flag would result in
allocating less static variables.

Anyways, 2 or 3 CPU flags make no relevant difference now.

Acked-by: Flavio Leitner <fbl@sysclose.org>
diff mbox series

Patch

diff --git a/lib/dpdk.c b/lib/dpdk.c
index 319540394..c883a4b8b 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -614,13 +614,33 @@  print_dpdk_version(void)
     puts(rte_version());
 }
 
+/* Avoid calling rte_cpu_get_flag_enabled() excessively, by caching the
+ * result of the call for each CPU flag in a static variable. To avoid
+ * allocating large numbers of static variables, use a uint8 as a bitfield.
+ * Note the macro must only return if the ISA check is done and available.
+ */
+#define ISA_CHECK_DONE_BIT (1 << 0)
+#define ISA_AVAILABLE_BIT  (1 << 1)
+
 #define CHECK_CPU_FEATURE(feature, name_str, RTE_CPUFLAG)               \
     do {                                                                \
         if (strncmp(feature, name_str, strlen(name_str)) == 0) {        \
-            int has_isa = rte_cpu_get_flag_enabled(RTE_CPUFLAG);        \
-            VLOG_DBG("CPU flag %s, available %s\n", name_str,           \
-                      has_isa ? "yes" : "no");                          \
-            return true;                                                \
+            static uint8_t isa_check_##RTE_CPUFLAG;                     \
+            int check = isa_check_##RTE_CPUFLAG & ISA_CHECK_DONE_BIT;   \
+            if (OVS_UNLIKELY(!check)) {                                 \
+                int has_isa = rte_cpu_get_flag_enabled(RTE_CPUFLAG);    \
+                VLOG_DBG("CPU flag %s, available %s\n",                 \
+                         name_str, has_isa ? "yes" : "no");             \
+                isa_check_##RTE_CPUFLAG = ISA_CHECK_DONE_BIT;           \
+                if (has_isa) {                                          \
+                    isa_check_##RTE_CPUFLAG |= ISA_AVAILABLE_BIT;       \
+                }                                                       \
+            }                                                           \
+            if (isa_check_##RTE_CPUFLAG & ISA_AVAILABLE_BIT) {          \
+                return true;                                            \
+            } else {                                                    \
+                return false;                                           \
+            }                                                           \
         }                                                               \
     } while (0)