[dpdk-dev,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(-)
  

+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.