On Mon, 27 Jul 2015 13:40:08 -0700
Ravi Kerur <rkerur at gmail.com> wrote:
> On Sun, Jul 26, 2015 at 2:54 PM, Thomas Monjalon <thomas.monjalon at
> 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.
> >
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?
threaded applications. The thread name is a meaningful C language
string, whose length is restricted to 16 characters, including the ter?
minating null byte ('\0').
> 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?
Not that I see on Linux
$ find /usr/include -type f | xargs grep PTHREAD_MAX_NAMELEN_NP
> >
> > > + 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.
You can always pass a char * when function takes const char *
