[dpdk-dev] [RFC PATCH 0/7] support multi-phtread per lcore

[Ananyev, Konstantin]

Hi Steve,

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Liang, Cunming
> Sent: Tuesday, December 23, 2014 9:52 AM
> To: Stephen Hemminger; Richardson, Bruce
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [RFC PATCH 0/7] support multi-phtread per lcore
> 
> 
> 
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> > Sent: Tuesday, December 23, 2014 2:29 AM
> > To: Richardson, Bruce
> > Cc: Liang, Cunming; dev at dpdk.org
> > Subject: Re: [dpdk-dev] [RFC PATCH 0/7] support multi-phtread per lcore
> >
> > On Mon, 22 Dec 2014 09:46:03 +0000
> > Bruce Richardson <bruce.richardson at intel.com> wrote:
> >
> > > On Mon, Dec 22, 2014 at 01:51:27AM +0000, Liang, Cunming wrote:
> > > > ...
> > > > > I'm conflicted on this one. However, I think far more applications would be
> > > > > broken
> > > > > to start having to use thread_id in place of an lcore_id than would be
> > broken
> > > > > by having the lcore_id no longer actually correspond to a core.
> > > > > I'm actually struggling to come up with a large number of scenarios where
> > it's
> > > > > important to an app to determine the cpu it's running on, compared to the
> > large
> > > > > number of cases where you need to have a data-structure per thread. In
> > DPDK
> > > > > libs
> > > > > alone, you see this assumption that lcore_id == thread_id a large number
> > of
> > > > > times.
> > > > >
> > > > > Despite the slight logical inconsistency, I think it's better to avoid
> > introducing
> > > > > a thread-id and continue having lcore_id representing a unique thread.
> > > > >
> > > > > /Bruce
> > > >
> > > > Ok, I understand it.
> > > > I list the implicit meaning if using lcore_id representing the unique thread.
> > > > 1). When lcore_id less than RTE_MAX_LCORE, it still represents the logical
> > core id.
> > > > 2). When lcore_id large equal than RTE_MAX_LCORE, it represents an unique
> > id for thread.
> > > > 3). Most of APIs(except rte_lcore_id()) in rte_lcore.h suggest to be used only
> > in CASE 1)
> > > > 4). rte_lcore_id() can be used in CASE 2), but the return value no matter
> > represent a logical core id.
> > > >
> > > > If most of us feel it's acceptable, I'll prepare for the RFC v2 base on this
> > conclusion.
> > > >
> > > > /Cunming
> > >
> > > Sorry, I don't like that suggestion either, as having lcore_id values greater
> > > than RTE_MAX_LCORE is terrible, as how will people know how to dimension
> > arrays
> > > to be indexes by lcore id? Given the choice, if we are not going to just use
> > > lcore_id as a generic thread id, which is always between 0 and
> > RTE_MAX_LCORE
> > > we can look to define a new thread_id variable to hold that. However, it should
> > > have a bounded range.
> > > From an ease-of-porting perspective, I still think that the simplest option is to
> > > use the existing lcore_id and accept the fact that it's now a thread id rather
> > > than an actual physical lcore. Question is, is would that cause us lots of issues
> > > in the future?
> > >
> > > /Bruce
> >
> > The current rte_lcore_id() has different meaning the thread. Your proposal will
> > break code that uses lcore_id to do per-cpu statistics and the lcore_config
> > code in the samples.
> > q
> [Liang, Cunming] +1.

Few more thoughts on that subject:

Actually one more place in the lib, where lcore_id is used (and it should be unique):
rte_spinlock_recursive_lock() / rte_spinlock_recursive_trylock().
So if we going to replace lcore_id with thread_id as uniques thread index, then these functions
have to be updated too.

About maintaining our own unique thread_id inside shared memory (_get_linear_tid()/_put_linear_tid()).
There is one thing that worries me with that approach:
In case of abnormal process termination, TIDs used by that process will remain 'reserved'
and there is no way to know which TIDs were used by terminated process.
So there could be a situation with DPDK multi-process model,
when after secondary process abnormal termination, It wouldn't be possible to restart it -
we just run out of 'free' TIDs. 
 
Which makes me think probably there is no need to introduce new globally unique 'thread_id'?
Might be just lcore_id is enough?  
As Mirek and Bruce suggested we can treat it a sort of 'unique thread id' inside EAL.
Or as 'virtual' core id that can run on set of physical cpus, and these subsets for different 'virtual' cores can intersect.
Then basically we can keep legacy behaviour with '-c <lcores_mask>,' where each
lcore_id matches one to one  with physical cpu, and introduce new one, something like:
--lcores='(<lcore_set1>)=(<phys_cpu_set1>),..(<lcore_setN)=(<phys_cpu_setN>)'.
So let say: --lcores=(0-7)=(0,2-4),(10)=(7),(8)=(all)' would mean:
Create 10 EAL threads, bind threads with clore_id=[0-7] to cpuset: <0,2,3,4>, 
thread  with lcore_id=10 is binded to  cpu 7, and allow to run lcore_id=8 on any cpu in the system.    
Of course '-c' and '-lcores' would be mutually exclusive, and we will need to update  rte_lcore_to_socket_id()
and introduce: rte_lcore_(set|get)_affinity().

Does it make sense to you?

BTW, one more thing: while we are on it  - it is probably a good time to do something with our interrupt thread?
It is a bit strange that we can't use rte_pktmbuf_free() or  rte_spinlock_recursive_lock() from our own interrupt/alarm handlers

Konstantin