DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: dev@dpdk.org, Jianfeng Tan <jianfeng.tan@intel.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	konstantin.ananyev@intel.com, qi.z.zhang@intel.com,
	stephen@networkplumber.org, olivier.matz@6wind.com,
	david.marchand@6wind.com, ferruh.yigit@intel.com
Subject: Re: [dpdk-dev] [PATCH v2 5/7] eal: bring forward init of interrupt handling
Date: Fri, 13 Jul 2018 08:41:34 +0100	[thread overview]
Message-ID: <162242b3-388d-93c9-9046-e4ed76e49ae2@intel.com> (raw)
In-Reply-To: <5431396.k1M6VaMcRs@xps>

On 12-Jul-18 11:36 PM, Thomas Monjalon wrote:
> 26/06/2018 12:53, Anatoly Burakov:
>> From: Jianfeng Tan <jianfeng.tan@intel.com>
>>
>> Next commit will make asynchronous IPC requests rely on alarm API,
>> which in turn relies on interrupts to work. Therefore, move the EAL
>> interrupt initialization before IPC initialization to avoid breaking
>> IPC in the next commit.
>>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -839,6 +839,11 @@ rte_eal_init(int argc, char **argv)
>>   
>>   	rte_config_init();
>>   
>> +	if (rte_eal_intr_init() < 0) {
>> +		rte_eal_init_alert("Cannot init interrupt-handling thread\n");
>> +		return -1;
>> +	}
>> +
>>   	/* Put mp channel init before bus scan so that we can init the vdev
>>   	 * bus through mp channel in the secondary process before the bus scan.
>>   	 */
>> @@ -968,11 +973,6 @@ rte_eal_init(int argc, char **argv)
>>   		rte_config.master_lcore, (int)thread_id, cpuset,
>>   		ret == 0 ? "" : "...");
>>   
>> -	if (rte_eal_intr_init() < 0) {
>> -		rte_eal_init_alert("Cannot init interrupt-handling thread\n");
>> -		return -1;
>> -	}
>> -
>>   	RTE_LCORE_FOREACH_SLAVE(i) {
> 
> I am almost sure it will bring regressions.
> 
> Please think again about the consequences of initializing interrupt thread
> before affinity setting, memory init, device init.
> 
> Opinions / ideas from anyone?
> 

Since interrupt thread is no longer relying on rte_malloc, i do not 
expect there to be any consequences for memory.

I also do not expect any consequences due to moving intr init before 
setting CPU thread affinity, because first of all interrupt thread is 
*already* run before setting thread affinity of the lcores (but after 
setting thread affinity of the master), and it picks its own affinity 
based on parsed coremask anyway, which is already parsed at a point 
where i'm moving it to. Affinities will be set similarly to how they 
were set before, because lcore information is already parsed.

As for device init, that is debatable. The only consequence i can think 
of is if device interrupts happen right after enabling the intr handler 
for that device and this causes some kind of issue or a race. But, we 
already support device hotplug which essentially causes the same 
situation to happen (interrupt handler initialized before bus 
scan/probe), so i'm not really convinced this could have any negative 
consequences either.

-- 
Thanks,
Anatoly

  reply	other threads:[~2018-07-13  7:41 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-15 14:25 [dpdk-dev] [PATCH 0/8] Remove IPC threads Anatoly Burakov
2018-06-15 14:25 ` [dpdk-dev] [PATCH 1/8] eal/linux: use glibc malloc in alarm Anatoly Burakov
2018-06-15 14:25 ` [dpdk-dev] [PATCH 2/8] eal/linux: use glibc malloc in interrupt handling Anatoly Burakov
2018-06-15 14:25 ` [dpdk-dev] [PATCH 3/8] ipc: remove IPC thread for async requests Anatoly Burakov
2018-06-15 14:25 ` [dpdk-dev] [PATCH 4/8] eal: bring forward init of interrupt handling Anatoly Burakov
2018-06-15 14:25 ` [dpdk-dev] [PATCH 5/8] eal: add IPC type for interrupt thread Anatoly Burakov
2018-06-15 14:25 ` [dpdk-dev] [PATCH 6/8] eal/bsdapp: add " Anatoly Burakov
2018-06-15 14:25 ` [dpdk-dev] [PATCH 7/8] eal/bsdapp: add alarm support Anatoly Burakov
2018-06-15 14:25 ` [dpdk-dev] [PATCH 8/8] ipc: remove main IPC thread Anatoly Burakov
2018-06-26  1:19 ` [dpdk-dev] [PATCH 0/8] Remove IPC threads Zhang, Qi Z
2018-06-26  7:03 ` Zhang, Qi Z
2018-06-26 10:53 ` [dpdk-dev] [PATCH v2 0/7] Remove asynchronous IPC thread Anatoly Burakov
2018-07-13 10:44   ` Thomas Monjalon
2018-06-26 10:53 ` [dpdk-dev] [PATCH v2 1/7] eal/linux: use glibc malloc in alarm Anatoly Burakov
2018-06-26 10:53 ` [dpdk-dev] [PATCH v2 2/7] eal/linux: use glibc malloc in interrupt handling Anatoly Burakov
2018-06-26 10:53 ` [dpdk-dev] [PATCH v2 3/7] eal/bsdapp: add interrupt thread Anatoly Burakov
2018-06-26 10:53 ` [dpdk-dev] [PATCH v2 4/7] eal/bsdapp: add alarm support Anatoly Burakov
2018-06-26 10:53 ` [dpdk-dev] [PATCH v2 5/7] eal: bring forward init of interrupt handling Anatoly Burakov
2018-07-12 22:36   ` Thomas Monjalon
2018-07-13  7:41     ` Burakov, Anatoly [this message]
2018-07-13  8:09     ` David Marchand
2018-07-13  9:10       ` Tiwei Bie
2018-07-13 11:28         ` David Marchand
2018-06-26 10:53 ` [dpdk-dev] [PATCH v2 6/7] ipc: remove IPC thread for async requests Anatoly Burakov
2018-06-26 10:53 ` [dpdk-dev] [PATCH v2 7/7] doc: document IPC callback limitations Anatoly Burakov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=162242b3-388d-93c9-9046-e4ed76e49ae2@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@6wind.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jianfeng.tan@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=qi.z.zhang@intel.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).