[v4,09/10] eal: add EAL argument for setting thread priority

Message ID 1616802771-31578-10-git-send-email-navasile@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series eal: Add new API for threading |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Narcisa Ana Maria Vasile March 26, 2021, 11:52 p.m. UTC
  From: Narcisa Vasile <navasile@microsoft.com>

Allow the user to choose the thread priority through an EAL
command line argument.

The user can select the thread priority to be either 'normal'
or 'critical':
--thread-prio normal
--thread-prio realtime

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
---
 lib/librte_eal/common/eal_common_options.c | 28 +++++++++++++++++++++-
 lib/librte_eal/common/eal_internal_cfg.h   |  2 ++
 lib/librte_eal/common/eal_options.h        |  2 ++
 3 files changed, 31 insertions(+), 1 deletion(-)
  

Comments

Stephen Hemminger March 27, 2021, 4:04 p.m. UTC | #1
On Fri, 26 Mar 2021 16:52:50 -0700
Narcisa Ana Maria Vasile <navasile@linux.microsoft.com> wrote:

> From: Narcisa Vasile <navasile@microsoft.com>
> 
> Allow the user to choose the thread priority through an EAL
> command line argument.
> 
> The user can select the thread priority to be either 'normal'
> or 'critical':
> --thread-prio normal
> --thread-prio realtime
> 
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>

Please don't add this. This will open up a huge set of bugs.
See the mailing list about how users report starvation and bug
checks on Windows when using real time.

In my experience, DPDK has same problem on Linux. The DPDK applications
(usually) poll at 100% CPU without system calls. If the user sets
these threads to real-time, then those threads have priority over kernel
background tasks (like handling soft interrupt or writing to the disk).
Therefore setting RT causes data loss or eventually RCU and watchdog
timeouts.

This patch encourages the fallacy that Real Time is faster.
The DPDK poll usage model is not compatible with the OS defintion
of real time. Real time is for processes doing system calls that
have precise timing requirements about when to wakeup from those
system calls.
  

Patch

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 622c7bc42..287a89a75 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -106,6 +106,7 @@  eal_long_options[] = {
 	{OPT_TELEMETRY,         0, NULL, OPT_TELEMETRY_NUM        },
 	{OPT_NO_TELEMETRY,      0, NULL, OPT_NO_TELEMETRY_NUM     },
 	{OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
+	{OPT_THREAD_PRIORITY,   1, NULL, OPT_THREAD_PRIORITY_NUM},
 
 	/* legacy options that will be removed in future */
 	{OPT_PCI_BLACKLIST,     1, NULL, OPT_PCI_BLACKLIST_NUM    },
@@ -1383,6 +1384,24 @@  eal_parse_simd_bitwidth(const char *arg)
 	return 0;
 }
 
+static int
+eal_parse_thread_priority(const char *arg)
+{
+	struct internal_config *internal_conf =
+		eal_get_internal_configuration();
+	enum rte_thread_priority priority;
+
+	if (!strncmp("normal", arg, sizeof("normal")))
+		priority = RTE_THREAD_PRIORITY_NORMAL;
+	else if (!strncmp("realtime", arg, sizeof("realtime")))
+		priority = RTE_THREAD_PRIORITY_REALTIME_CRITICAL;
+	else
+		return -1;
+
+	internal_conf->thread_priority = priority;
+	return 0;
+}
+
 static int
 eal_parse_base_virtaddr(const char *arg)
 {
@@ -1796,7 +1815,13 @@  eal_parse_common_option(int opt, const char *optarg,
 			return -1;
 		}
 		break;
-
+	case OPT_THREAD_PRIORITY_NUM:
+		if (eal_parse_thread_priority(optarg) < 0) {
+			RTE_LOG(ERR, EAL, "invalid parameter for --"
+					OPT_THREAD_PRIORITY "\n");
+			return -1;
+		}
+		break;
 	/* don't know what to do, leave this to caller */
 	default:
 		return 1;
@@ -2059,6 +2084,7 @@  eal_common_usage(void)
 	       "                      (can be used multiple times)\n"
 	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead of native RDTSC\n"
 	       "  --"OPT_PROC_TYPE"         Type of this process (primary|secondary|auto)\n"
+	       "  --"OPT_THREAD_PRIORITY"   Set threads priority (normal|realtime)\n"
 #ifndef RTE_EXEC_ENV_WINDOWS
 	       "  --"OPT_SYSLOG"            Set syslog facility\n"
 #endif
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index 51dbe86e2..7ab1d0008 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -93,6 +93,8 @@  struct internal_config {
 	unsigned int no_telemetry; /**< true to disable Telemetry */
 	struct simd_bitwidth max_simd_bitwidth;
 	/**< max simd bitwidth path to use */
+	enum rte_thread_priority thread_priority;
+	/**< thread priority to configure */
 };
 
 void eal_reset_internal_config(struct internal_config *internal_cfg);
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index 7b348e707..9f5b209f6 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -93,6 +93,8 @@  enum {
 	OPT_NO_TELEMETRY_NUM,
 #define OPT_FORCE_MAX_SIMD_BITWIDTH  "force-max-simd-bitwidth"
 	OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
+#define OPT_THREAD_PRIORITY          "thread-prio"
+	OPT_THREAD_PRIORITY_NUM,
 
 	/* legacy option that will be removed in future */
 #define OPT_PCI_BLACKLIST     "pci-blacklist"