mbox series

[v6,0/3] add eal functions for thread affinity and self

Message ID 1652361270-27116-1-git-send-email-roretzla@linux.microsoft.com (mailing list archive)
Headers show
Series add eal functions for thread affinity and self | expand

Message

Tyler Retzlaff May 12, 2022, 1:14 p.m. UTC
this series provides basic dependencies for additional eal thread api
additions. series includes

* basic platform error number conversion.
* function to get current thread identifier.
* functions to get and set affinity with platform agnostic thread
  identifier.
* minimal unit test of get and set affinity demonstrating usage.

note: previous series introducing these functions is now superseded by
this series.
http://patches.dpdk.org/project/dpdk/list/?series=20472&state=*

v6:
* rebase for asan flag addition to app/test/meson.build
* RTE_BUILD_BUG_ON(sizeof(pthread_t) > sizeof(uintptr_t)) to raise
  attention if pthread_t implementation exceeds storage available
  from uintptr_t.
  note:
    the macro is placed in rte_thread_self() body because it not
    valid syntax at file scope of the translation unit. ordinarily
    the macro would be used in headers but that would leak the
    pthread_t implementation detail into the public header.

v5:
* use rte_log instead of log_early. it appears that logging is
  now initialized before the call to eal_query_group_affinity.
  note: removal of log_early is off-topic for this series and
        will be done separately.
* rte_log DEBUG for failure related to limitation where all
  processors must be in the same processor group.
* remove redundant tests. get/get/compare, set/get/compare
  are both retained.

v4:
* combine patch eal/windows: translate Windows errors to errno-style
  errors into eal: implement functions for get/set thread affinity
  patch. the former introduced static functions that were not used
  without eal: implement functions for get/set thread affinity which
  would cause a build break when applied standalone.
* remove struct tag from rte_thread_t struct typedef.
* remove rte_ prefix from rte_convert_cpuset_to_affinity static
  function.

v3:
* fix memory leak on eal_create_cpu_map error paths.

v2:
* add missing boilerplate comments warning of experimental api
  for rte_thread_{set,get}_affinity_by_id().
* don't break literal format string to log_early to improve
  searchability.
* fix multi-line comment style to match file.
* return ENOTSUP instead of EINVAL from rte_convert_cpuset_to_affinity()
  if cpus in set are not part of the same processor group and note
  limitation in commit message.
* expand series to include rte_thread_self().
* modify unit test to remove use of implementation detail and
  get thread identifier use added rte_thread_self().
* move literal value to rhs when using memcmp in RTE_TEST_ASSERT

Tyler Retzlaff (3):
  eal: add basic thread ID and current thread identifier API
  eal: implement functions for get/set thread affinity
  test/threads: add unit test for thread API

 app/test/meson.build             |   2 +
 app/test/test_threads.c          |  81 +++++++++++++++++
 lib/eal/include/rte_thread.h     |  64 +++++++++++++
 lib/eal/unix/rte_thread.c        |  29 ++++++
 lib/eal/version.map              |   5 +
 lib/eal/windows/eal_lcore.c      | 181 +++++++++++++++++++++++++++----------
 lib/eal/windows/eal_windows.h    |  10 ++
 lib/eal/windows/include/rte_os.h |   2 +
 lib/eal/windows/rte_thread.c     | 191 ++++++++++++++++++++++++++++++++++++++-
 9 files changed, 517 insertions(+), 48 deletions(-)
 create mode 100644 app/test/test_threads.c

Comments

David Marchand May 19, 2022, 3:05 p.m. UTC | #1
Hello Tyler,

On Thu, May 12, 2022 at 3:14 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> this series provides basic dependencies for additional eal thread api
> additions. series includes
>
> * basic platform error number conversion.
> * function to get current thread identifier.
> * functions to get and set affinity with platform agnostic thread
>   identifier.
> * minimal unit test of get and set affinity demonstrating usage.
>
> note: previous series introducing these functions is now superseded by
> this series.
> http://patches.dpdk.org/project/dpdk/list/?series=20472&state=*
>
> v6:
> * rebase for asan flag addition to app/test/meson.build
> * RTE_BUILD_BUG_ON(sizeof(pthread_t) > sizeof(uintptr_t)) to raise
>   attention if pthread_t implementation exceeds storage available
>   from uintptr_t.
>   note:
>     the macro is placed in rte_thread_self() body because it not
>     valid syntax at file scope of the translation unit. ordinarily
>     the macro would be used in headers but that would leak the
>     pthread_t implementation detail into the public header.

There was a missing update of MAINTAINERS in patch 3 that I fixed.
Series lgtm, applied, thanks.


Looking at the generated documentation, rte_thread.h is not exposed,
but this was already the case before the series.
Could you look into it?