Re: [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-VfR2kkLFssw@public.gmane.org
Subject: RE: [dpdk-dev] [RFC PATCH 0/7] support multi-phtread per lcore


Hi Steve,

-----Original Message-----
From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of Liang, Cunming
Sent: Tuesday, December 23, 2014 9:52 AM
To: Stephen Hemminger; Richardson, Bruce
Cc: dev-VfR2kkLFssw@public.gmane.org
Subject: Re: [dpdk-dev] [RFC PATCH 0/7] support multi-phtread per 
lcore

quoted

-----Original Message-----
From: Stephen Hemminger [mailto:stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org]
Sent: Tuesday, December 23, 2014 2:29 AM
To: Richardson, Bruce
Cc: Liang, Cunming; dev-VfR2kkLFssw@public.gmane.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 [off-list ref] wrote:

quoted

On Mon, Dec 22, 2014 at 01:51:27AM +0000, Liang, Cunming wrote:

quoted

...

quoted

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

quoted

quoted

quoted

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

quoted

quoted

quoted

important to an app to determine the cpu it's running on, 
compared to the

large

quoted

quoted

quoted

number of cases where you need to have a data-structure per 
thread. In

DPDK

quoted

quoted

quoted

libs
alone, you see this assumption that lcore_id == thread_id a 
large number

of

quoted

quoted

quoted

times.

Despite the slight logical inconsistency, I think it's better 
to avoid

introducing

quoted

quoted

quoted

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.

quoted

quoted

2). When lcore_id large equal than RTE_MAX_LCORE, it represents 
an unique

id for thread.

quoted

quoted

3). Most of APIs(except rte_lcore_id()) in rte_lcore.h suggest 
to be used only

in CASE 1)

quoted

quoted

4). rte_lcore_id() can be used in CASE 2), but the return value 
no matter

represent a logical core id.

quoted

quoted

If most of us feel it's acceptable, I'll prepare for the RFC v2 
base on this

conclusion.

quoted

quoted

/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

quoted

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

quoted

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