From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH] Use pthread_setname APIs
Date: Sun, 26 Jul 2015 23:54:40 +0200
Message-ID: <2280623.OZV87NhkWH@xps13>
References: <1429736748-16874-1-git-send-email-rkerur@gmail.com>
 <1429736803-16943-1-git-send-email-rkerur@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org
To: Ravi Kerur <rkerur@gmail.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wi0-f174.google.com (mail-wi0-f174.google.com
 [209.85.212.174]) by dpdk.org (Postfix) with ESMTP id 06D72C488
 for <dev@dpdk.org>; Sun, 26 Jul 2015 23:55:57 +0200 (CEST)
Received: by wibud3 with SMTP id ud3so117989432wib.1
 for <dev@dpdk.org>; Sun, 26 Jul 2015 14:55:56 -0700 (PDT)
In-Reply-To: <1429736803-16943-1-git-send-email-rkerur@gmail.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Ravi,
It seems to be a nice improvement but it needs some cleanup.

Checkpatch returns some errors.

2015-04-22 14:06, Ravi Kerur:
> use pthread_setname_np and pthread_set_name_np for Linux and
> FreeBSD respectively.
> Restrict pthread name len to 16 via config for both Linux and FreeBSD.

One of the most important part of the commit message is to answer why.
Here you probably should explain that the goal is to help debugging.

>  #
> +# Max pthread name len
> +#
> +CONFIG_RTE_MAX_THREAD_NAME_LEN=16

It doesn't have to be configurable. A define would be sufficient.

> +		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "print-stats");

Why not put this line just before use of thread_name?

> +
> +		ret = pthread_create(&tid, NULL, (void*)print_stats, NULL );
> +
> +		if (ret != 0)
> +			rte_exit(EXIT_FAILURE,
> +				"Cannot create print-stats thread\n");
> +
> +		ret = pthread_setname_np(tid, thread_name);
> +
> +		if (ret != 0)
> +			RTE_LOG(ERR, VHOST_CONFIG,
> +				"Cannot set print-stats name\n");
[...]

> +		pthread_set_name_np(lcore_config[i].thread_id,
> +					(const char *)thread_name);

Is const casting really needed?