[v7,1/5] eal: add sleep API
Checks
Commit Message
POSIX sleep(3) is missing from Windows.
Add generic rte_thread_sleep() to suspend current OS thread.
Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: Khoa To <khot@microsoft.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
---
lib/librte_eal/common/eal_common_timer.c | 5 +++--
lib/librte_eal/include/rte_thread.h | 11 +++++++++++
lib/librte_eal/rte_eal_exports.def | 2 ++
lib/librte_eal/unix/rte_thread.c | 10 +++++++++-
lib/librte_eal/version.map | 1 +
lib/librte_eal/windows/eal_thread.c | 9 ++++++++-
6 files changed, 34 insertions(+), 4 deletions(-)
Comments
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dmitry Kozlyuk
> Sent: Sunday, April 4, 2021 1:41 AM
>
> POSIX sleep(3) is missing from Windows.
> Add generic rte_thread_sleep() to suspend current OS thread.
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Acked-by: Khoa To <khot@microsoft.com>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>
How is this better than the rte_delay_us_sleep() function in rte_cycles.h?
Med venlig hilsen / kind regards
- Morten Brørup
2021-04-06 16:34 (UTC+0200), Morten Brørup:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dmitry Kozlyuk
> > Sent: Sunday, April 4, 2021 1:41 AM
> >
> > POSIX sleep(3) is missing from Windows.
> > Add generic rte_thread_sleep() to suspend current OS thread.
> >
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > Acked-by: Khoa To <khot@microsoft.com>
> > Acked-by: Ray Kinsella <mdr@ashroe.eu>
>
> How is this better than the rte_delay_us_sleep() function in rte_cycles.
Honestly, I just assumed the sleep() shim existed for a reason.
Now that you pointed to it, I think indeed no new API is needed.
I wonder why rte_delay_us_sleep() is not used throughout DPDK.
Should their usages be converted as soon as code is enabled for Windows?
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dmitry Kozlyuk
> Sent: Wednesday, April 7, 2021 1:30 AM
>
> 2021-04-06 16:34 (UTC+0200), Morten Brørup:
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dmitry Kozlyuk
> > > Sent: Sunday, April 4, 2021 1:41 AM
> > >
> > > POSIX sleep(3) is missing from Windows.
> > > Add generic rte_thread_sleep() to suspend current OS thread.
> > >
> > > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > > Acked-by: Khoa To <khot@microsoft.com>
> > > Acked-by: Ray Kinsella <mdr@ashroe.eu>
> >
> > How is this better than the rte_delay_us_sleep() function in
> rte_cycles.
>
> Honestly, I just assumed the sleep() shim existed for a reason.
> Now that you pointed to it, I think indeed no new API is needed.
>
> I wonder why rte_delay_us_sleep() is not used throughout DPDK.
I think it's just tradition from Linux/BSD developers not being used to cross-platform development or having forgotten how to develop software for deeply embedded systems without a standard O/S below. We use POSIX functions like sleep() without thinking about it, and simply forget looking for an EAL variant of the function first.
> Should their usages be converted as soon as code is enabled for
> Windows?
Yes. The Environment Abstraction Layer is there to avoid using O/S specific function calls. So it should be safe to replace all these sleep() calls with the EAL variant at any time.
On Wed, Apr 07, 2021 at 09:31:43AM +0200, Morten Brørup wrote:
>
> I think it's just tradition from Linux/BSD developers not being used to cross-platform development or having forgotten how to develop software for deeply embedded systems without a standard O/S below. We use POSIX functions like sleep() without thinking about it, and simply forget looking for an EAL variant of the function first.
>
it's easy to make this mistake if by including dpdk application headers
we indirectly introduce platform specific headers into the applications
namespace.
if this wasn't happening the application would be forced to confront the
issue by directly including unistd.h itself directly making it more
obvious that they were writing inherently non-portable code.
i think this is being fixed but it is a work in progress but it
highlights the reason why there needs to be scrutiny around what goes
into the eal and other library headers.
@@ -16,6 +16,7 @@
#include <rte_cycles.h>
#include <rte_pause.h>
#include <rte_eal.h>
+#include <rte_thread.h>
#include "eal_private.h"
#include "eal_memcfg.h"
@@ -47,9 +48,9 @@ estimate_tsc_freq(void)
#define CYC_PER_10MHZ 1E7
RTE_LOG(WARNING, EAL, "WARNING: TSC frequency estimated roughly"
" - clock timings may be less accurate.\n");
- /* assume that the sleep(1) will sleep for 1 second */
+ /* assume that the rte_thread_sleep(1) will sleep for 1 second */
uint64_t start = rte_rdtsc();
- sleep(1);
+ rte_thread_sleep(1);
/* Round up to 10Mhz. 1E7 ~ 10Mhz */
return RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, CYC_PER_10MHZ);
}
@@ -119,6 +119,17 @@ int rte_thread_value_set(rte_thread_key key, const void *value);
__rte_experimental
void *rte_thread_value_get(rte_thread_key key);
+/**
+ * Suspend current OS thread for the specified time, yielding CPU to scheduler.
+ *
+ * @param sec
+ * Number of seconds to sleep. The system may return control later,
+ * but not earlier. Zero value always yields the CPU, but control may be
+ * returned immediately.
+ */
+__rte_experimental
+void rte_thread_sleep(unsigned int sec);
+
#ifdef __cplusplus
}
#endif
@@ -334,3 +334,5 @@ EXPORTS
rte_mem_map
rte_mem_page_size
rte_mem_unmap
+
+ rte_thread_sleep
@@ -3,10 +3,12 @@
*/
#include <errno.h>
-#include <pthread.h>
#include <stdlib.h>
#include <string.h>
+#include <pthread.h>
+#include <unistd.h>
+
#include <rte_common.h>
#include <rte_errno.h>
#include <rte_log.h>
@@ -90,3 +92,9 @@ rte_thread_value_get(rte_thread_key key)
}
return pthread_getspecific(key->thread_index);
}
+
+void
+rte_thread_sleep(unsigned int sec)
+{
+ sleep(sec);
+}
@@ -413,6 +413,7 @@ EXPERIMENTAL {
# added in 21.05
rte_thread_key_create;
rte_thread_key_delete;
+ rte_thread_sleep;
rte_thread_value_get;
rte_thread_value_set;
rte_version_minor;
@@ -11,9 +11,10 @@
#include <rte_per_lcore.h>
#include <rte_common.h>
#include <rte_memory.h>
-#include <eal_thread.h>
+#include <rte_thread.h>
#include "eal_private.h"
+#include "eal_thread.h"
#include "eal_windows.h"
/*
@@ -154,3 +155,9 @@ rte_thread_setname(__rte_unused pthread_t id, __rte_unused const char *name)
/* This is a stub, not the expected result */
return 0;
}
+
+void
+rte_thread_sleep(unsigned int sec)
+{
+ return Sleep(MS_PER_S * sec);
+}