Re: [PATCH] eal: check cpu flags at init
From: Aaron Conole <aconole@redhat.com>
Date: 2016-09-26 15:43:39
Subsystem:
library code, the rest · Maintainers:
Andrew Morton, Linus Torvalds
Flavio Leitner [off-list ref] writes:
quoted hunk ↗ jump to hunk
An application might be linked to DPDK but not really use it, so move the cpu flag check to the EAL initialization instead. Signed-off-by: Flavio Leitner <redacted> --- lib/librte_eal/bsdapp/eal/eal.c | 3 +++ lib/librte_eal/common/eal_common_cpuflags.c | 6 ------ lib/librte_eal/linuxapp/eal/eal.c | 3 +++ 3 files changed, 6 insertions(+), 6 deletions(-)diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c index a0c8f8c..c4b22af 100644 --- a/lib/librte_eal/bsdapp/eal/eal.c +++ b/lib/librte_eal/bsdapp/eal/eal.c@@ -496,6 +496,9 @@ rte_eal_init(int argc, char **argv) char cpuset[RTE_CPU_AFFINITY_STR_LEN]; char thread_name[RTE_MAX_THREAD_NAME_LEN]; + /* checks if the machine is adequate */ + rte_cpu_check_supported(); +
I think it makes sense to return a result here; after all, since this is no longer a *constructor*, we can actually handle a failure case. So maybe the following diff:
diff --git a/lib/librte_eal/common/eal_common_cpuflags.c b/lib/librte_eal/common/eal_common_cpuflags.c
index ecb1240..eccf5f8 100644
--- a/lib/librte_eal/common/eal_common_cpuflags.c
+++ b/lib/librte_eal/common/eal_common_cpuflags.c@@ -38,15 +38,9 @@ /** * Checks if the machine is adequate for running the binary. If it is not, the - * program exits with status 1. - * The function attribute forces this function to be called before main(). But - * with ICC, the check is generated by the compiler. + * function returns ENOTSUP. */ -#ifndef __INTEL_COMPILER -void __attribute__ ((__constructor__)) -#else -void -#endif +int rte_cpu_check_supported(void) { /* This is generated at compile-time by the build system */
@@ -63,14 +57,15 @@ rte_cpu_check_supported(void) fprintf(stderr, "ERROR: CPU feature flag lookup failed with error %d\n", ret); - exit(1); + return ENOTSUP; } if (!ret) { fprintf(stderr, "ERROR: This system does not support \"%s\".\n" "Please check that RTE_MACHINE is set correctly.\n", rte_cpu_get_flag_name(compile_time_flags[i])); - exit(1); + return ENOTSUP; } } + return 0; }
diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h b/lib/librte_eal/common/include/generic/rte_cpuflags.h
index 71321f3..6e4eb5a 100644
--- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
+++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h@@ -79,7 +79,7 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature); * that were specified at compile time. It is called automatically within the * EAL, so does not need to be used by applications. */ -void +int rte_cpu_check_supported(void); #endif /* _RTE_CPUFLAGS_H_ */ --
and the change these hunks to:
if (!rte_cpu_check_supported()) {
return -1;
}
My only concern is whether this change would be considered ABI
breaking. I wouldn't think so, since it doesn't seem as though an
application would want to call this explicitly (and is spelled out as
such), but I can't be sure that it isn't already included in the
standard application API, and therefore needs to go through the change
process.
My $.02
-Aaron
quoted hunk ↗ jump to hunk
if (!rte_atomic32_test_and_set(&run_once)) return -1;diff --git a/lib/librte_eal/common/eal_common_cpuflags.c b/lib/librte_eal/common/eal_common_cpuflags.c index ecb1240..b5f76f7 100644 --- a/lib/librte_eal/common/eal_common_cpuflags.c +++ b/lib/librte_eal/common/eal_common_cpuflags.c@@ -39,14 +39,8 @@ /** * Checks if the machine is adequate for running the binary. If it is not, the * program exits with status 1. - * The function attribute forces this function to be called before main(). But - * with ICC, the check is generated by the compiler. */ -#ifndef __INTEL_COMPILER -void __attribute__ ((__constructor__)) -#else void -#endif rte_cpu_check_supported(void) { /* This is generated at compile-time by the build system */diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c index d5b81a3..4e88cfc 100644 --- a/lib/librte_eal/linuxapp/eal/eal.c +++ b/lib/librte_eal/linuxapp/eal/eal.c@@ -740,6 +740,9 @@ rte_eal_init(int argc, char **argv) char cpuset[RTE_CPU_AFFINITY_STR_LEN]; char thread_name[RTE_MAX_THREAD_NAME_LEN]; + /* checks if the machine is adequate */ + rte_cpu_check_supported(); + if (!rte_atomic32_test_and_set(&run_once)) return -1;