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.
