[v4,1/2] eal: error number enhancement for thread TLS API
Checks
Commit Message
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
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?
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>
> 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?
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:"
@@ -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);
@@ -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;
@@ -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;