From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-f173.google.com (mail-ob0-f173.google.com [209.85.214.173]) by dpdk.org (Postfix) with ESMTP id F41CFC5AC for ; Mon, 27 Jul 2015 23:48:44 +0200 (CEST) Received: by obnw1 with SMTP id w1so69850655obn.3 for ; Mon, 27 Jul 2015 14:48:44 -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=yA6mgKXpwBSxkMMkF4v/lSDqorLji0QJs6jETR1qGQk=; b=oYw0UYPWq0i7UYvO1QS7traM8wBWqCY10UDCdUwkt3/YdVTQfz2KsBxPGuTqj4Z4rx aaCSHonTfmm44T1OHScapWSxIoqKDdYiuEnoH8Oc7rBJnrGQIiNVglglJR82tuNid6BZ WGQ06baLnmNS3PB7/gMS+ug162M65J7sQTvZBMFtmmRyUA7UUbojH+J78Od9i1KjvKE7 yK6Hc/dvFB3lJAz7x0jLNJjK3I7yXW0pG3qxUyXNHdyBrbEezh/1spVsiH9fDXRpUvKY m5L9oXer46cyYW/S3MpAtJaGHZyATeZFk/QGE+VVHfQ/snWPc/Z2qf9Q2vt1l6X4C3cy iMdA== MIME-Version: 1.0 X-Received: by 10.182.18.72 with SMTP id u8mr29469141obd.8.1438033724483; Mon, 27 Jul 2015 14:48:44 -0700 (PDT) Received: by 10.202.179.195 with HTTP; Mon, 27 Jul 2015 14:48:44 -0700 (PDT) In-Reply-To: <20150727140923.2a1cc852@urahara> References: <1429736748-16874-1-git-send-email-rkerur@gmail.com> <1429736803-16943-1-git-send-email-rkerur@gmail.com> <2280623.OZV87NhkWH@xps13> <20150727140923.2a1cc852@urahara> Date: Mon, 27 Jul 2015 14:48:44 -0700 Message-ID: From: Ravi Kerur To: Stephen Hemminger Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Jul 2015 21:48:45 -0000 On Mon, Jul 27, 2015 at 2:09 PM, Stephen Hemminger < stephen@networkplumber.org> wrote: > On Mon, 27 Jul 2015 13:40:08 -0700 > Ravi Kerur wrote: > > > 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=3D16 > > > > > > It doesn't have to be configurable. A define would be sufficient. > > > > > Definitely should not be in the DPDK config. > It is property of system, which change some config parameter doesn't > change. > > Manpage implies it is fixed in glibc. > > The pthread_setname_np() function can be used to set a > unique name for a thread, which can be useful for debugging > multi=E2=80=90 > threaded applications. The thread name is a meaningful C > language > string, whose length is restricted to 16 characters, including the > ter=E2=80=90 > minating null byte ('\0'). > > > Had run into issues(reported by Bruce) as name_len =3D 32 worked fine f= or > > 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_le= n > =3D > > 16? > > Not that I see on Linux > $ find /usr/include -type f | xargs grep PTHREAD_MAX_NAMELEN_NP > > That's correct on Linux and look like it's not defined for FreeBSD as well. FreeBSD man page doesn't mention anything about string length too. > > > > > > > > > + 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 =3D pthread_create(&tid, NULL, (void*)print_stats= , > NULL > > > ); > > > > + > > > > + if (ret !=3D 0) > > > > + rte_exit(EXIT_FAILURE, > > > > + "Cannot create print-stats thread\n")= ; > > > > + > > > > + ret =3D pthread_setname_np(tid, thread_name); > > > > + > > > > + if (ret !=3D 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. > > You can always pass a char * when function takes const char * > That's correct, will remove it.