[dpdk-dev] [PATCH] Use pthread_setname APIs
Ravi Kerur
rkerur at gmail.com
Mon Jul 27 22:40:08 CEST 2015
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.
>
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.
More information about the dev
mailing list