From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 23F566833 for ; Mon, 3 Sep 2018 18:06:44 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Sep 2018 09:06:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,325,1531810800"; d="scan'208";a="80540775" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga003.jf.intel.com with ESMTP; 03 Sep 2018 09:04:35 -0700 Received: from fmsmsx119.amr.corp.intel.com (10.18.124.207) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 3 Sep 2018 09:04:34 -0700 Received: from fmsmsx117.amr.corp.intel.com ([169.254.3.210]) by FMSMSX119.amr.corp.intel.com ([169.254.14.38]) with mapi id 14.03.0319.002; Mon, 3 Sep 2018 09:04:34 -0700 From: "Wiles, Keith" To: Ilya Maximets CC: "dev@dpdk.org" , "Wu, Jingjing" , "Ananyev, Konstantin" , "Lu, Wenzhuo" , "Xing, Beilei" , "Zhang, Qi Z" , "Wang, Xiao W" , "Richardson, Bruce" Thread-Topic: [dpdk-dev] [RFC 2/2] drivers/net: use sleep delay by default for intel NICs Thread-Index: AQHUQ5s5HFpVM2Ft1U6850iJHABsoqTfJqoAgAADEQCAAAQBAA== Date: Mon, 3 Sep 2018 16:04:34 +0000 Message-ID: <78507543-C3B8-4FC8-92D2-043713C496F1@intel.com> References: <20180831124517.27619-1-i.maximets@samsung.com> <20180831124404eucas1p20daff43600dfe450c9106616f886eab4~P_LLRV5aM1664316643eucas1p2w@eucas1p2.samsung.com> <20180903153131eucas1p122e24d751d0f87d0cff88f3360c50e37~Q7ZOaSu4X0341203412eucas1p1A@eucas1p1.samsung.com> <832A4FA5-2615-4CD1-8B48-0176557BE6E6@intel.com> <20180903154841eucas1p1016af55e5cd460a7e1029e20ad5c5e4a~Q7oN_vC0l2380923809eucas1p1N@eucas1p1.samsung.com> In-Reply-To: <20180903154841eucas1p1016af55e5cd460a7e1029e20ad5c5e4a~Q7oN_vC0l2380923809eucas1p1N@eucas1p1.samsung.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.237.220.39] Content-Type: text/plain; charset="us-ascii" Content-ID: <397AC616E7FE3F49B3BCB796A3B7E91F@intel.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC 2/2] drivers/net: use sleep delay by default for intel NICs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Sep 2018 16:06:45 -0000 > On Sep 3, 2018, at 4:50 PM, Ilya Maximets wrote: >=20 > On 03.09.2018 18:39, Wiles, Keith wrote: >>=20 >>=20 >>> On Sep 3, 2018, at 4:33 PM, Ilya Maximets wrot= e: >>>=20 >>> On 03.09.2018 18:14, Wiles, Keith wrote: >>>>=20 >>>>=20 >>>>> On Aug 31, 2018, at 1:45 PM, Ilya Maximets w= rote: >>>>>=20 >>>>> NICs uses different delays up to a second during their >>>>> configuration. It makes no sense to busy-wait so long wasting >>>>> CPU cycles and preventing any other threads to execute on the >>>>> same CPU core. These busy polling are the rudiments that came >>>>> from the kernel drivers where you can not sleep in interrupt >>>>> context, but as we're in userspace, we're able and should >>>>> sleep to allow other threads to run. >>>>> Delays never called on rx/tx path, so this should not affect >>>>> performance. >>>>=20 >>>> I have a question, the only thread being effected by the busy wait is = the thread assigned to the core or the master lcore in the case of rte_eal_= init(). It should not effect other threads running on other cores, right? I= f you do have other threads running in the same code then I see the need, p= lease help me understand the use and how it is effecting DPDK. >>>=20 >>> Originally, this patch comes from this issue: >>> http://mails.dpdk.org/archives/dev/2018-August/110640.html >>>=20 >>> non-EAL threads is one of the answers. >>> For example, main thread in OVS periodically checks the link statuses >>> and hangs there busy waiting on different NIC configuration operations. >>=20 >> We have link state get no wait why is that not working instead of wait p= olling the line state? >=20 > Yes, and OVS uses it, but ixgbe driver contains the workaround for a > fiber links configuration issue that calls ixgbe_setup_link, that > busy waits trying to configure multispeed fiber. You may found > details in the patch I mentioned. I moved this code to the separate > alarm thread, but it will eat CPU cycles anyway preventing others > from working. OK. I see. I think using nanosleep is ok as long as that works on all of the platforms= FreeBSD, Linux, Windows will be coming I assume later. This should not be = a show stopper unless it does not work on FreeBSD, but I am pretty it does = as I do not have access to look. I will look at the rest of the patch. >=20 >>=20 >>> Also, main OVS tread is responsible for port configuration and >>> re-configuration. There are few other in-dpdk threads like interrupts >>> handling thread (alarm handling thread). vhost_thread is working on >>> the same lcores and wants some time to work too. >>>=20 >>> In case of hyperthreading busy-waiting will significantly affect >>> threads on the siblings. >>>=20 >>> Best regards, Ilya Maximets. >>>=20 >>>>>=20 >>>>> Signed-off-by: Ilya Maximets >>>>> --- >>>>> drivers/net/avf/Makefile | 1 + >>>>> drivers/net/avf/base/avf_osdep.h | 4 ++-- >>>>> drivers/net/e1000/Makefile | 1 + >>>>> drivers/net/e1000/base/e1000_osdep.h | 2 +- >>>>> drivers/net/i40e/base/i40e_osdep.h | 6 +++--- >>>>> drivers/net/ifc/base/ifcvf_osdep.h | 2 +- >>>>> drivers/net/ixgbe/base/ixgbe_osdep.h | 2 +- >>>>> 7 files changed, 10 insertions(+), 8 deletions(-) >>>>>=20 >>>>> diff --git a/drivers/net/avf/Makefile b/drivers/net/avf/Makefile >>>>> index 3f815bbc4..8ee707529 100644 >>>>> --- a/drivers/net/avf/Makefile >>>>> +++ b/drivers/net/avf/Makefile >>>>> @@ -9,6 +9,7 @@ include $(RTE_SDK)/mk/rte.vars.mk >>>>> LIB =3D librte_pmd_avf.a >>>>>=20 >>>>> CFLAGS +=3D -O3 >>>>> +CFLAGS +=3D -DALLOW_EXPERIMENTAL_API >>>>> LDLIBS +=3D -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring >>>>> LDLIBS +=3D -lrte_ethdev -lrte_net -lrte_kvargs -lrte_hash >>>>> LDLIBS +=3D -lrte_bus_pci >>>>> diff --git a/drivers/net/avf/base/avf_osdep.h b/drivers/net/avf/base/= avf_osdep.h >>>>> index 9ef45968e..442a5acd0 100644 >>>>> --- a/drivers/net/avf/base/avf_osdep.h >>>>> +++ b/drivers/net/avf/base/avf_osdep.h >>>>> @@ -93,8 +93,8 @@ typedef uint64_t u64; >>>>> #define avf_memset(a, b, c, d) memset((a), (b), (c)) >>>>> #define avf_memcpy(a, b, c, d) rte_memcpy((a), (b), (c)) >>>>>=20 >>>>> -#define avf_usec_delay(x) rte_delay_us(x) >>>>> -#define avf_msec_delay(x) rte_delay_us(1000*(x)) >>>>> +#define avf_usec_delay(x) rte_delay_us_sleep(x) >>>>> +#define avf_msec_delay(x) avf_usec_delay(1000 * (x)) >>>>>=20 >>>>> #define AVF_PCI_REG(reg) rte_read32(reg) >>>>> #define AVF_PCI_REG_ADDR(a, reg) \ >>>>> diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile >>>>> index 9c87e883b..0ed627656 100644 >>>>> --- a/drivers/net/e1000/Makefile >>>>> +++ b/drivers/net/e1000/Makefile >>>>> @@ -10,6 +10,7 @@ LIB =3D librte_pmd_e1000.a >>>>>=20 >>>>> CFLAGS +=3D -O3 >>>>> CFLAGS +=3D $(WERROR_FLAGS) >>>>> +CFLAGS +=3D -DALLOW_EXPERIMENTAL_API >>>>> LDLIBS +=3D -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring >>>>> LDLIBS +=3D -lrte_ethdev -lrte_net -lrte_kvargs >>>>> LDLIBS +=3D -lrte_bus_pci >>>>> diff --git a/drivers/net/e1000/base/e1000_osdep.h b/drivers/net/e1000= /base/e1000_osdep.h >>>>> index b8868049f..5958ea157 100644 >>>>> --- a/drivers/net/e1000/base/e1000_osdep.h >>>>> +++ b/drivers/net/e1000/base/e1000_osdep.h >>>>> @@ -48,7 +48,7 @@ >>>>>=20 >>>>> #include "../e1000_logs.h" >>>>>=20 >>>>> -#define DELAY(x) rte_delay_us(x) >>>>> +#define DELAY(x) rte_delay_us_sleep(x) >>>>> #define usec_delay(x) DELAY(x) >>>>> #define usec_delay_irq(x) DELAY(x) >>>>> #define msec_delay(x) DELAY(1000*(x)) >>>>> diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/ba= se/i40e_osdep.h >>>>> index 8e5c593c9..a6072e153 100644 >>>>> --- a/drivers/net/i40e/base/i40e_osdep.h >>>>> +++ b/drivers/net/i40e/base/i40e_osdep.h >>>>> @@ -233,9 +233,9 @@ struct i40e_spinlock { >>>>> #define i40e_memcpy(a, b, c, d) rte_memcpy((a), (b), (c)) >>>>>=20 >>>>> #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d)) >>>>> -#define DELAY(x) rte_delay_us(x) >>>>> -#define i40e_usec_delay(x) rte_delay_us(x) >>>>> -#define i40e_msec_delay(x) rte_delay_us(1000*(x)) >>>>> +#define DELAY(x) rte_delay_us_sleep(x) >>>>> +#define i40e_usec_delay(x) DELAY(x) >>>>> +#define i40e_msec_delay(x) DELAY(1000 * (x)) >>>>> #define udelay(x) DELAY(x) >>>>> #define msleep(x) DELAY(1000*(x)) >>>>> #define usleep_range(min, max) msleep(DIV_ROUND_UP(min, 1000)) >>>>> diff --git a/drivers/net/ifc/base/ifcvf_osdep.h b/drivers/net/ifc/bas= e/ifcvf_osdep.h >>>>> index cf151ef52..6aef25ea4 100644 >>>>> --- a/drivers/net/ifc/base/ifcvf_osdep.h >>>>> +++ b/drivers/net/ifc/base/ifcvf_osdep.h >>>>> @@ -17,7 +17,7 @@ >>>>> #define DEBUGOUT(S, args...) RTE_LOG(DEBUG, PMD, S, ##args) >>>>> #define STATIC static >>>>>=20 >>>>> -#define msec_delay rte_delay_ms >>>>> +#define msec_delay(x) rte_delay_us_sleep(1000 * (x)) >>>>>=20 >>>>> #define IFCVF_READ_REG8(reg) rte_read8(reg) >>>>> #define IFCVF_WRITE_REG8(val, reg) rte_write8((val), (reg)) >>>>> diff --git a/drivers/net/ixgbe/base/ixgbe_osdep.h b/drivers/net/ixgbe= /base/ixgbe_osdep.h >>>>> index bb5dfd2af..94ede9bc2 100644 >>>>> --- a/drivers/net/ixgbe/base/ixgbe_osdep.h >>>>> +++ b/drivers/net/ixgbe/base/ixgbe_osdep.h >>>>> @@ -51,7 +51,7 @@ >>>>>=20 >>>>> #define ASSERT(x) if(!(x)) rte_panic("IXGBE: x") >>>>>=20 >>>>> -#define DELAY(x) rte_delay_us(x) >>>>> +#define DELAY(x) rte_delay_us_sleep(x) >>>>> #define usec_delay(x) DELAY(x) >>>>> #define msec_delay(x) DELAY(1000*(x)) >>>>>=20 >>>>> --=20 >>>>> 2.17.1 >>>>>=20 >>>>=20 >>>> Regards, >>>> Keith >>=20 >> Regards, >> Keith Regards, Keith