[dpdk-dev] [RFC 2/3] tqs: add thread quiescent state library

Stephen Hemminger stephen at networkplumber.org
Tue Dec 18 07:31:48 CET 2018


On Tue, 18 Dec 2018 04:30:39 +0000
Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com> wrote:

> > 
> > On 11-Dec-18 6:40 AM, Honnappa Nagarahalli wrote:  
> > >>  
> > >>>>>  
> > >>>>>>> +
> > >>>>>>> +/* Add a reader thread, running on an lcore, to the list of
> > >>>>>>> +threads
> > >>>>>>> + * reporting their quiescent state on a TQS variable.
> > >>>>>>> + */
> > >>>>>>> +int __rte_experimental
> > >>>>>>> +rte_tqs_register_lcore(struct rte_tqs *v, unsigned int lcore_id) {
> > >>>>>>> +	TQS_RETURN_IF_TRUE((v == NULL || lcore_id >=  
> > >>>>>> RTE_TQS_MAX_LCORE),  
> > >>>>>>> +				-EINVAL);  
> > >>>>>>
> > >>>>>> It is not very good practice to make function return different
> > >>>>>> values and behave in a different way in debug/non-debug mode.
> > >>>>>> I'd say that for slow-path (functions in .c) it is always good to
> > >>>>>> check input parameters.
> > >>>>>> For fast-path (functions in .h) we sometimes skip such checking,
> > >>>>>> but debug mode can probably use RTE_ASSERT() or so.  
> > >>>>> Makes sense, I will change this in the next version.
> > >>>>>  
> > >>>>>>
> > >>>>>>
> > >>>>>> lcore_id >= RTE_TQS_MAX_LCORE
> > >>>>>>
> > >>>>>> Is this limitation really necessary?  
> > >>>>> I added this limitation because currently DPDK application cannot
> > >>>>> take a mask more than 64bit wide. Otherwise, this should be as big
> > >>>>> as  
> > >>>> RTE_MAX_LCORE.  
> > >>>>> I see that in the case of '-lcores' option, the number of lcores
> > >>>>> can be more than the number of PEs. In this case, we still need a
> > >>>>> MAX limit (but  
> > >>>> can be bigger than 64).  
> > >>>>>  
> > >>>>>> First it means that only lcores can use that API (at least
> > >>>>>> data-path part), second even today many machines have more than
> > >>>>>> 64  
> > >> cores.  
> > >>>>>> I think you can easily avoid such limitation, if instead of
> > >>>>>> requiring lcore_id as input parameter, you'll just make it return
> > >>>>>> index of  
> > >>>> next available entry in w[].  
> > >>>>>> Then tqs_update() can take that index as input parameter.  
> > >>>>> I had thought about a similar approach based on IDs. I was
> > >>>>> concerned that ID will be one more thing to manage for the
> > >>>>> application. But, I see the  
> > >>>> limitations of the current approach now. I will change it to
> > >>>> allocation  
> > >> based.  
> > >>>> This will support even non-EAL pthreads as well.
> > >>>>
> > >>>> Yes, with such approach non-lcore threads will be able to use it also.
> > >>>>  
> > >>> I realized that rte_tqs_register_lcore/ rte_tqs_unregister_lcore
> > >>> need to be  
> > >> efficient as they can be called from the worker's packet processing
> > >> loop (rte_event_dequeue_burst allows blocking. So, the worker thread
> > >> needs to call rte_tqs_unregister_lcore before calling
> > >> rte_event_dequeue_burst and rte_tqs_register_lcore before starting
> > >> packet processing). Allocating the thread ID in these functions will make  
> > them more complex.  
> > >>>
> > >>> I suggest that we change the argument 'lcore_id' to 'thread_id'. The  
> > >> application could use 'lcore_id' as 'thread_id' if threads are mapped
> > >> to physical cores 1:1.  
> > >>>
> > >>> If the threads are not mapped 1:1 to physical cores, the threads
> > >>> need to use  
> > >> a thread_id in the range of 0 - RTE_TQS_MAX_THREADS. I do not see
> > >> that DPDK has a thread_id concept. For TQS, the thread IDs are global
> > >> (i.e. not per TQS variable). I could provide APIs to do the thread ID
> > >> allocation, but I think the thread ID allocation should not be part
> > >> of this library. Such thread ID might be useful for other libraries.  
> > >>>
> > >>> <snip  
> > >>
> > >>
> > >> Thread id is problematic since Glibc doesn't want to give it out.
> > >> You have to roll your own function to do gettid().
> > >> It is not as easy as just that.  Plus what about preemption?  
> > >
> > > Agree. I looked into this further. The rte_gettid function uses a system call  
> > (BSD and Linux). I am not clear on the space of the ID returned (as well). I do
> > not think it is guaranteed that it will be with in a narrow range that is required
> > here.  
> > >
> > > My suggestion would be to add a set of APIs that would allow for
> > > allocation of thread IDs which are within a given range of 0 to
> > > <predefined MAX>
> > >  
> > 
> > System-provided thread-ID's would probably also be potentially non-unique in
> > multiprocess scenario?  
> For linux, rte_gettid is implemented as:
> int rte_sys_gettid(void)
> {
>         return (int)syscall(SYS_gettid);
> }
> 
> Came across [1] which states, thread-IDs are unique across the system.
> 
> For BSD, thr_self is used. [2] says it provides system wide unique thread IDs.
> 
> [1] https://stackoverflow.com/questions/6372102/what-is-the-difference-between-pthread-self-and-gettid-which-one-should-i-u
> [2] https://nxmnpg.lemoda.net/2/thr_self
> 
> > 
> > --
> > Thanks,
> > Anatoly  

Using thread id directly on Linux is battling against the glibc gods wishes.
Bad things may come to those that disobey them :-)

But really many libraries need to do the same thing, it is worth looking around.
The bigger issue is pid and thread id recycling with the limited range allowed.


More information about the dev mailing list