> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Friday, 27 October 2023 10.09
>
> When adding an API for creating threads,
> the real-time priority has been forbidden on Unix.
>
> There is a known issue with ring behaviour,
> but it should not be completely forbidden.
>
> Real-time thread can block some kernel threads on the same core,
> making the system unstable.
> That's why a sleep is added in the test thread,
> and a warning is logged when using real-time priority.
>
> Fixes: ca04c78b6262 ("eal: get/set thread priority per thread
> identifier")
> Fixes: ce6e911d20f6 ("eal: add thread lifetime API")
> Fixes: a7ba40b2b1bf ("drivers: convert to internal control threads")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Thomas Monjalon <tho...@monjalon.net>
> Acked-by: Morten Brørup <m...@smartsharesystems.com>
> ---
[...]
> enum rte_thread_priority {
> + /** Normal thread priority, the default. */
> RTE_THREAD_PRIORITY_NORMAL = 0,
> - /**< normal thread priority, the default */
> + /**
> + * Highest thread priority, use with caution.
> + * WARNING: System may be unstable because of a real-time busy
> loop.
> + * @see rte_thread_yield_realtime().
Please remove the reference to the now non-existing function.
Also, I'd prefer to move the warning comments (about real-time threads having
priority over kernel threads, and issues with rte_ring) up here, so it goes
into the public API documentation.
> + */
> RTE_THREAD_PRIORITY_REALTIME_CRITICAL = 1,
> - /**< highest thread priority allowed */
> };
>
> /**
> diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
> index 278d8d342d..17ffb86c17 100644
> --- a/lib/eal/unix/rte_thread.c
> +++ b/lib/eal/unix/rte_thread.c
> @@ -33,6 +33,8 @@ static int
> thread_map_priority_to_os_value(enum rte_thread_priority eal_pri, int
> *os_pri,
> int *pol)
> {
> + static bool warned;
> +
> /* Clear the output parameters. */
> *os_pri = sched_get_priority_min(SCHED_OTHER) - 1;
> *pol = -1;
> @@ -51,6 +53,17 @@ thread_map_priority_to_os_value(enum
> rte_thread_priority eal_pri, int *os_pri,
> sched_get_priority_max(SCHED_OTHER)) / 2;
> break;
> case RTE_THREAD_PRIORITY_REALTIME_CRITICAL:
> + /*
> + * WARNING: Real-time busy loop takes priority on kernel
> threads,
> + * making the system unstable.
> + * There is also a known issue when using
> rte_ring.
> + */
> + if (!warned) {
> + RTE_LOG(NOTICE, EAL,
> + "Real-time thread is unstable if polling
> without sleep.\n");
> + warned = true;
> + }
Is it 100 % certain that the system becomes unstable if not sleeping or using
blocking system calls from a real-time thread?
And technically, it's not the thread itself that becomes unstable.
How about:
"System may be unstable unless real-time thread uses blocking system calls or
sleeps."
or:
"Real-time thread usually requires the use of blocking system calls or sleeps."
or something else.
My ACK is still valid.
