From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <rkerur@gmail.com>
Received: from mail-oi0-f46.google.com (mail-oi0-f46.google.com
 [209.85.218.46]) by dpdk.org (Postfix) with ESMTP id 06860C5AA
 for <dev@dpdk.org>; Mon, 27 Jul 2015 22:40:09 +0200 (CEST)
Received: by oige126 with SMTP id e126so58429475oig.0
 for <dev@dpdk.org>; Mon, 27 Jul 2015 13:40:08 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;
 h=mime-version:in-reply-to:references:date:message-id:subject:from:to
 :cc:content-type;
 bh=hKT+8t6yLBL1coPfjNr/rV3aVnpag/Ie9PEAHfx9P6g=;
 b=qiZKpUFHdEEAU6aA4tnyTpr+vZ/EuecPeLuWAloCh9LOQAv5+0IsWWsga2t1h914d0
 dEu2/CX2Su4jobx2BcX21mfBTybFAwqD7+gfdirLVcgSSO5cqQMpibRDWL3/ytqd9c0i
 69Oy9PRVbxFwcEH+aYondxXFGJ+TZm0YfoLLkHXt3+ihcSAAek8Ssd6C25k14CclfauY
 kwVDWNsFdYzq1lZfDonYG4eLbN/kOuoX+etvKpdhrRuRTc2y/psAURo9nX9VuqudIRwY
 z4tb5GxjMEXdUkCVrQJVoEXpFgcVLzsjwdLyTZ1y8Qxg7pAEMO2X41gvFjNgpjpxYDU2
 +g7Q==
MIME-Version: 1.0
X-Received: by 10.202.186.132 with SMTP id k126mr28976840oif.60.1438029608369; 
 Mon, 27 Jul 2015 13:40:08 -0700 (PDT)
Received: by 10.202.179.195 with HTTP; Mon, 27 Jul 2015 13:40:08 -0700 (PDT)
In-Reply-To: <2280623.OZV87NhkWH@xps13>
References: <1429736748-16874-1-git-send-email-rkerur@gmail.com>
 <1429736803-16943-1-git-send-email-rkerur@gmail.com>
 <2280623.OZV87NhkWH@xps13>
Date: Mon, 27 Jul 2015 13:40:08 -0700
Message-ID: <CAFb4SLC0waLvTB4ad85yKGkFFXvsQ_NqFMjE3akHH6nsxnmqwA@mail.gmail.com>
From: Ravi Kerur <rkerur@gmail.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Content-Type: text/plain; charset=UTF-8
X-Content-Filtered-By: Mailman/MimeDel 2.1.15
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] Use pthread_setname APIs
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
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>
X-List-Received-Date: Mon, 27 Jul 2015 20:40:09 -0000

On Sun, Jul 26, 2015 at 2:54 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

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

Sure will do it and will run checkpatch before next version.

>
> >  #
> > +# Max pthread name len
> > +#
> > +CONFIG_RTE_MAX_THREAD_NAME_LEN=16
>
> It doesn't have to be configurable. A define would be sufficient.
>

Had run into issues(reported by Bruce) as name_len = 32 worked fine for
FreeBSD but not for Linux, hence thought of making it configurable for
Linux/FreeBSD and be able to have different name len's. Found out that
there is also a definition PTHREAD_MAX_NAMELEN_NP, if it's available on
both Linux and FreeBSD  I will use this, else should I restrict name_len =
16?

>
> > +             snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
> "print-stats");
>
> Why not put this line just before use of thread_name?
>

Will do it.

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

Function signature has a (const char *) for second parameter, I will double
check and remove if not needed.