diff mbox series

[v4,1/2] eal: error number enhancement for thread TLS API

Message ID 20210310124856.8188-2-talshn@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series EAL Thread TLS API enhancements |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Tal Shnaiderman March 10, 2021, 12:48 p.m. UTC 
  add error number reporting to rte_errno in all
functions in the rte_thread_tls_* API.

Suggested-by: Anatoly Burakov <anatoly.burakov@intel.com>
Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
 lib/librte_eal/include/rte_thread.h | 14 +++++++++++---
 lib/librte_eal/unix/rte_thread.c    |  6 ++++++
 lib/librte_eal/windows/rte_thread.c |  6 ++++++
 3 files changed, 23 insertions(+), 3 deletions(-)

Comments

Narcisa Ana Maria Vasile March 10, 2021, 7:45 p.m. UTC | #1 
On Wed, Mar 10, 2021 at 02:48:55PM +0200, Tal Shnaiderman wrote:
> add error number reporting to rte_errno in all
> functions in the rte_thread_tls_* API.
> 
> Suggested-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> ---
>  lib/librte_eal/include/rte_thread.h | 14 +++++++++++---
>  lib/librte_eal/unix/rte_thread.c    |  6 ++++++
>  lib/librte_eal/windows/rte_thread.c |  6 ++++++
>  3 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h

After we introduce a translation function to map from Windows error 
codes to errno style codes (as part of EAL threads API),
should we change this to directly return the error code from the functions?
Or do we follow the pattern of setting rte_errno?
Dmitry Kozlyuk March 13, 2021, 2:29 a.m. UTC | #2 
2021-03-10 14:48 (UTC+0200), Tal Shnaiderman:
> add error number reporting to rte_errno in all
> functions in the rte_thread_tls_* API.
> 
> Suggested-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>

Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Tal Shnaiderman March 15, 2021, 3:37 p.m. UTC | #3 
> Subject: Re: [PATCH v4 1/2] eal: error number enhancement for thread TLS
> API
> 
> On Wed, Mar 10, 2021 at 02:48:55PM +0200, Tal Shnaiderman wrote:
> > add error number reporting to rte_errno in all functions in the
> > rte_thread_tls_* API.
> >
> > Suggested-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> > ---
> >  lib/librte_eal/include/rte_thread.h | 14 +++++++++++---
> >  lib/librte_eal/unix/rte_thread.c    |  6 ++++++
> >  lib/librte_eal/windows/rte_thread.c |  6 ++++++
> >  3 files changed, 23 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/librte_eal/include/rte_thread.h
> > b/lib/librte_eal/include/rte_thread.h
> 
> After we introduce a translation function to map from Windows error codes
> to errno style codes (as part of EAL threads API), should we change this to
> directly return the error code from the functions?
> Or do we follow the pattern of setting rte_errno?

Sorry for the late reply,

I'd stick to errors in rte_errno, note that in cases like rte_thread_value_get the only way to get the errors is with rte_errno since it's returning the value itself.

BTW will you also add translation function for the UNIX errors to get identical errors?
Thomas Monjalon March 15, 2021, 11:14 p.m. UTC | #4 
10/03/2021 13:48, Tal Shnaiderman:
> --- a/lib/librte_eal/include/rte_thread.h
> +++ b/lib/librte_eal/include/rte_thread.h
> @@ -59,7 +59,9 @@ void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
>   *
>   * @return
>   *   On success, zero.
> - *   On failure, a negative number.
> + *   On failure, a negative number and an error number is set in rte_errno.
> + *   rte_errno can be set to: ENOMEM  - Memory allocation error.
> + *                            ENOEXEC - Specific OS error.
>   */
>  
>  __rte_experimental
> @@ -73,7 +75,9 @@ int rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *));
>   *
>   * @return
>   *   On success, zero.
> - *   On failure, a negative number.
> + *   On failure, a negative number and an error number is set in rte_errno.
> + *   rte_errno can be set to: EINVAL  - Invalid parameter passed.
> + *                            ENOEXEC - Specific OS error.
>   */
>  __rte_experimental
>  int rte_thread_tls_key_delete(rte_tls_key key);
> @@ -88,7 +92,9 @@ int rte_thread_tls_key_delete(rte_tls_key key);
>   *
>   * @return
>   *   On success, zero.
> - *   On failure, a negative number.
> + *   On failure, a negative number and an error number is set in rte_errno.
> + *   rte_errno can be set to: EINVAL  - Invalid parameter passed.
> + *                            ENOEXEC - Specific OS error.
>   */
>  __rte_experimental
>  int rte_thread_tls_value_set(rte_tls_key key, const void *value);
> @@ -102,6 +108,8 @@ int rte_thread_tls_value_set(rte_tls_key key, const void *value);
>   * @return
>   *   On success, value data pointer (can also be NULL).
>   *   On failure, NULL and an error number is set in rte_errno.
> + *   rte_errno can be set to: EINVAL  - Invalid parameter passed.
> + *                            ENOEXEC - Specific OS error.

Shorter (and less confusing): "rte_errno can be:"
diff mbox series

Patch

diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h
index e640ea1857..7a503b81c9 100644
--- a/lib/librte_eal/include/rte_thread.h
+++ b/lib/librte_eal/include/rte_thread.h
@@ -59,7 +59,9 @@  void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
  *
  * @return
  *   On success, zero.
- *   On failure, a negative number.
+ *   On failure, a negative number and an error number is set in rte_errno.
+ *   rte_errno can be set to: ENOMEM  - Memory allocation error.
+ *                            ENOEXEC - Specific OS error.
  */
 
 __rte_experimental
@@ -73,7 +75,9 @@  int rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *));
  *
  * @return
  *   On success, zero.
- *   On failure, a negative number.
+ *   On failure, a negative number and an error number is set in rte_errno.
+ *   rte_errno can be set to: EINVAL  - Invalid parameter passed.
+ *                            ENOEXEC - Specific OS error.
  */
 __rte_experimental
 int rte_thread_tls_key_delete(rte_tls_key key);
@@ -88,7 +92,9 @@  int rte_thread_tls_key_delete(rte_tls_key key);
  *
  * @return
  *   On success, zero.
- *   On failure, a negative number.
+ *   On failure, a negative number and an error number is set in rte_errno.
+ *   rte_errno can be set to: EINVAL  - Invalid parameter passed.
+ *                            ENOEXEC - Specific OS error.
  */
 __rte_experimental
 int rte_thread_tls_value_set(rte_tls_key key, const void *value);
@@ -102,6 +108,8 @@  int rte_thread_tls_value_set(rte_tls_key key, const void *value);
  * @return
  *   On success, value data pointer (can also be NULL).
  *   On failure, NULL and an error number is set in rte_errno.
+ *   rte_errno can be set to: EINVAL  - Invalid parameter passed.
+ *                            ENOEXEC - Specific OS error.
  */
 __rte_experimental
 void *rte_thread_tls_value_get(rte_tls_key key);
diff --git a/lib/librte_eal/unix/rte_thread.c b/lib/librte_eal/unix/rte_thread.c
index 86ffeebc95..b187c69a4c 100644
--- a/lib/librte_eal/unix/rte_thread.c
+++ b/lib/librte_eal/unix/rte_thread.c
@@ -24,6 +24,7 @@  rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *))
 	*key = malloc(sizeof(**key));
 	if ((*key) == NULL) {
 		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.\n");
+		rte_errno = ENOMEM;
 		return -1;
 	}
 	err = pthread_key_create(&((*key)->thread_index), destructor);
@@ -31,6 +32,7 @@  rte_thread_tls_key_create(rte_tls_key *key, void (*destructor)(void *))
 		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
 			 strerror(err));
 		free(*key);
+		rte_errno = ENOEXEC;
 		return -1;
 	}
 	return 0;
@@ -43,6 +45,7 @@  rte_thread_tls_key_delete(rte_tls_key key)
 
 	if (!key) {
 		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		rte_errno = EINVAL;
 		return -1;
 	}
 	err = pthread_key_delete(key->thread_index);
@@ -50,6 +53,7 @@  rte_thread_tls_key_delete(rte_tls_key key)
 		RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
 			 strerror(err));
 		free(key);
+		rte_errno = ENOEXEC;
 		return -1;
 	}
 	free(key);
@@ -63,12 +67,14 @@  rte_thread_tls_value_set(rte_tls_key key, const void *value)
 
 	if (!key) {
 		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		rte_errno = EINVAL;
 		return -1;
 	}
 	err = pthread_setspecific(key->thread_index, value);
 	if (err) {
 		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
 			strerror(err));
+		rte_errno = ENOEXEC;
 		return -1;
 	}
 	return 0;
diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c
index 2e2ab29177..fa9e360855 100644
--- a/lib/librte_eal/windows/rte_thread.c
+++ b/lib/librte_eal/windows/rte_thread.c
@@ -18,12 +18,14 @@  rte_thread_tls_key_create(rte_tls_key *key,
 	*key = malloc(sizeof(**key));
 	if ((*key) == NULL) {
 		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.\n");
+		rte_errno = ENOMEM;
 		return -1;
 	}
 	(*key)->thread_index = TlsAlloc();
 	if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
 		RTE_LOG_WIN32_ERR("TlsAlloc()");
 		free(*key);
+		rte_errno = ENOEXEC;
 		return -1;
 	}
 	return 0;
@@ -34,11 +36,13 @@  rte_thread_tls_key_delete(rte_tls_key key)
 {
 	if (!key) {
 		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		rte_errno = EINVAL;
 		return -1;
 	}
 	if (!TlsFree(key->thread_index)) {
 		RTE_LOG_WIN32_ERR("TlsFree()");
 		free(key);
+		rte_errno = ENOEXEC;
 		return -1;
 	}
 	free(key);
@@ -52,12 +56,14 @@  rte_thread_tls_value_set(rte_tls_key key, const void *value)
 
 	if (!key) {
 		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+		rte_errno = EINVAL;
 		return -1;
 	}
 	/* discard const qualifier */
 	p = (char *) (uintptr_t) value;
 	if (!TlsSetValue(key->thread_index, p)) {
 		RTE_LOG_WIN32_ERR("TlsSetValue()");
+		rte_errno = ENOEXEC;
 		return -1;
 	}
 	return 0;