[dpdk-dev,1/2] eal/thread: fix return codes for rte_thread_setname()
Checks
Commit Message
The doc says this function returns negative errno
on error, but it currently returns either -1 or
positive errno.
It was incorrectly assumed that pthread_setname_np()
returns negative error numbers. It always returns
positive ones, so this patch negates its return value
before returning.
While here, also ignore rte_thread_setname() failure
in rte_ctrl_thread_create() and print a debug message
instead.
Fixes: 3901ed99c2f8 ("eal: fix thread naming on FreeBSD")
Cc: thomas.monjalon@6wind.com
Cc: stable@dpdk.org
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
---
lib/librte_eal/common/eal_common_thread.c | 3 ++-
lib/librte_eal/linuxapp/eal/eal_thread.c | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
Comments
+CC thomas@monjalon.net
The previous @6wind email appears to be unavailable now.
> -----Original Message-----
> From: Stojaczyk, DariuszX
> Sent: Friday, June 8, 2018 2:37 PM
> To: dev@dpdk.org
> Cc: Stojaczyk, DariuszX <dariuszx.stojaczyk@intel.com>;
> thomas.monjalon@6wind.com; stable@dpdk.org
> Subject: [PATCH 1/2] eal/thread: fix return codes for rte_thread_setname()
>
> The doc says this function returns negative errno on error, but it currently
> returns either -1 or positive errno.
>
> It was incorrectly assumed that pthread_setname_np() returns negative
> error numbers. It always returns positive ones, so this patch negates its
> return value before returning.
>
> While here, also ignore rte_thread_setname() failure in
> rte_ctrl_thread_create() and print a debug message instead.
>
> Fixes: 3901ed99c2f8 ("eal: fix thread naming on FreeBSD")
> Cc: thomas.monjalon@6wind.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> ---
> lib/librte_eal/common/eal_common_thread.c | 3 ++-
> lib/librte_eal/linuxapp/eal/eal_thread.c | 4 ++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_thread.c
> b/lib/librte_eal/common/eal_common_thread.c
> index 4239863..8110ac2 100644
> --- a/lib/librte_eal/common/eal_common_thread.c
> +++ b/lib/librte_eal/common/eal_common_thread.c
> @@ -191,7 +191,8 @@ rte_ctrl_thread_create(pthread_t *thread, const
> char *name,
> if (name != NULL) {
> ret = rte_thread_setname(*thread, name);
> if (ret < 0)
> - goto fail;
> + RTE_LOG(DEBUG, EAL,
> + "Cannot set name for ctrl thread\n");
> }
>
> cpu_found = 0;
> diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c
> b/lib/librte_eal/linuxapp/eal/eal_thread.c
> index f652ff9..b496fc7 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_thread.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
> @@ -176,7 +176,7 @@ int rte_sys_gettid(void)
>
> int rte_thread_setname(pthread_t id, const char *name) {
> - int ret = -1;
> + int ret = ENOSYS;
> #if defined(__GLIBC__) && defined(__GLIBC_PREREQ) #if
> __GLIBC_PREREQ(2, 12)
> ret = pthread_setname_np(id, name);
> @@ -184,5 +184,5 @@ int rte_thread_setname(pthread_t id, const char
> *name) #endif
> RTE_SET_USED(id);
> RTE_SET_USED(name);
> - return ret;
> + return -ret;
> }
> --
> 2.7.4
On 08-Jun-18 1:37 PM, Dariusz Stojaczyk wrote:
> The doc says this function returns negative errno
> on error, but it currently returns either -1 or
> positive errno.
>
> It was incorrectly assumed that pthread_setname_np()
> returns negative error numbers. It always returns
> positive ones, so this patch negates its return value
> before returning.
>
> While here, also ignore rte_thread_setname() failure
> in rte_ctrl_thread_create() and print a debug message
> instead.
>
> Fixes: 3901ed99c2f8 ("eal: fix thread naming on FreeBSD")
> Cc: thomas.monjalon@6wind.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> ---
For patch contents,
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
However, maybe this should be split in two patches.
On Mon, Jun 18, 2018 at 11:00:55AM +0100, Burakov, Anatoly wrote:
> On 08-Jun-18 1:37 PM, Dariusz Stojaczyk wrote:
> > The doc says this function returns negative errno
> > on error, but it currently returns either -1 or
> > positive errno.
> >
> > It was incorrectly assumed that pthread_setname_np()
> > returns negative error numbers. It always returns
> > positive ones, so this patch negates its return value
> > before returning.
> >
> > While here, also ignore rte_thread_setname() failure
> > in rte_ctrl_thread_create() and print a debug message
> > instead.
> >
> > Fixes: 3901ed99c2f8 ("eal: fix thread naming on FreeBSD")
> > Cc: thomas.monjalon@6wind.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> > ---
>
> For patch contents,
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
> However, maybe this should be split in two patches.
Agree it should be split.
Reviewed-by: Olivier Matz <olivier.matz@6wind.com>
Out of curiosity, do you have a use-case where rte_thread_setname()
fails? The only reason I see is a too long name. Why this error
should be ignored?
> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Monday, June 25, 2018 4:36 PM
> To: Burakov, Anatoly <anatoly.burakov@intel.com>
> Cc: Stojaczyk, DariuszX <dariuszx.stojaczyk@intel.com>; dev@dpdk.org;
> thomas.monjalon@6wind.com; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/2] eal/thread: fix return codes for
> rte_thread_setname()
>
> On Mon, Jun 18, 2018 at 11:00:55AM +0100, Burakov, Anatoly wrote:
> > On 08-Jun-18 1:37 PM, Dariusz Stojaczyk wrote:
> > > The doc says this function returns negative errno
> > > on error, but it currently returns either -1 or
> > > positive errno.
> > >
> > > It was incorrectly assumed that pthread_setname_np()
> > > returns negative error numbers. It always returns
> > > positive ones, so this patch negates its return value
> > > before returning.
> > >
> > > While here, also ignore rte_thread_setname() failure
> > > in rte_ctrl_thread_create() and print a debug message
> > > instead.
> > >
> > > Fixes: 3901ed99c2f8 ("eal: fix thread naming on FreeBSD")
> > > Cc: thomas.monjalon@6wind.com
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> > > ---
> >
> > For patch contents,
> >
> > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >
> > However, maybe this should be split in two patches.
>
> Agree it should be split.
>
> Reviewed-by: Olivier Matz <olivier.matz@6wind.com>
>
> Out of curiosity, do you have a use-case where rte_thread_setname()
> fails? The only reason I see is a too long name. Why this error
> should be ignored?
I don't have any use-case like that. It's just that the error is not fatal and we can physically continue creating the thread. EAL does the same thing for the lcore threads.
@@ -191,7 +191,8 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
if (name != NULL) {
ret = rte_thread_setname(*thread, name);
if (ret < 0)
- goto fail;
+ RTE_LOG(DEBUG, EAL,
+ "Cannot set name for ctrl thread\n");
}
cpu_found = 0;
@@ -176,7 +176,7 @@ int rte_sys_gettid(void)
int rte_thread_setname(pthread_t id, const char *name)
{
- int ret = -1;
+ int ret = ENOSYS;
#if defined(__GLIBC__) && defined(__GLIBC_PREREQ)
#if __GLIBC_PREREQ(2, 12)
ret = pthread_setname_np(id, name);
@@ -184,5 +184,5 @@ int rte_thread_setname(pthread_t id, const char *name)
#endif
RTE_SET_USED(id);
RTE_SET_USED(name);
- return ret;
+ return -ret;
}