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

["Liang, Cunming" <cunming.liang@intel.com>]

I see. Will update soon.
Thanks for all the comments.

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Friday, January 09, 2015 1:24 AM
> To: Ananyev, Konstantin; Liang, Cunming; Stephen Hemminger
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [RFC PATCH 0/7] support multi-phtread per lcore
> 
> My opinion on this is that the lcore_id is rarely (if ever) used to find the actual
> core a thread is being run on. Instead it is used 99% of the time as a unique array
> index per thread, and therefore that we can keep that usage by just assigning a
> valid lcore_id to any extra threads created. The suggestion to get/set affinities on
> top of that seems a good one to me also.
> 
> /Bruce
> 
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, January 8, 2015 5:06 PM
> To: Liang, Cunming; Stephen Hemminger; Richardson, Bruce
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [RFC PATCH 0/7] support multi-phtread per lcore
> 
> 
> Hi Steve,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Liang, Cunming
> > Sent: Tuesday, December 23, 2014 9:52 AM
> > To: Stephen Hemminger; Richardson, Bruce
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [RFC PATCH 0/7] support multi-phtread per
> > lcore
> >
> >
> >
> > > -----Original Message-----
> > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > Sent: Tuesday, December 23, 2014 2:29 AM
> > > To: Richardson, Bruce
> > > Cc: Liang, Cunming; dev@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@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