From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 7E6201B911 for ; Wed, 27 Jun 2018 05:48:20 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Jun 2018 20:48:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,277,1526367600"; d="scan'208";a="50834923" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga008.fm.intel.com with ESMTP; 26 Jun 2018 20:48:17 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 26 Jun 2018 20:48:17 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.82]) by shsmsx102.ccr.corp.intel.com ([169.254.2.223]) with mapi id 14.03.0319.002; Wed, 27 Jun 2018 11:48:15 +0800 From: "Xing, Beilei" To: "Zhang, Qi Z" CC: "Wu, Jingjing" , "Yu, De" , "dev@dpdk.org" Thread-Topic: [PATCH] net/i40e: remove VF interrupt handler Thread-Index: AQHT/f9MV/He1TOWkEKDhN7Z0IpeGKRzlsGA Date: Wed, 27 Jun 2018 03:48:14 +0000 Message-ID: <94479800C636CB44BD422CB454846E01321CF9F1@SHSMSX101.ccr.corp.intel.com> References: <20180607013156.28763-1-qi.z.zhang@intel.com> In-Reply-To: <20180607013156.28763-1-qi.z.zhang@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] net/i40e: remove VF interrupt handler 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: Wed, 27 Jun 2018 03:48:21 -0000 > -----Original Message----- > From: Zhang, Qi Z > Sent: Thursday, June 7, 2018 9:32 AM > To: Xing, Beilei > Cc: Wu, Jingjing ; Yu, De ; > dev@dpdk.org; Zhang, Qi Z > Subject: [PATCH] net/i40e: remove VF interrupt handler >=20 > For i40evf, internal rx interrupt and adminq interrupt share the same sou= rce, > that cause a lot cpu cycles be wasted on interrupt handler on rx path. Th= is is > complained by customers which require low latency (when set > I40E_ITR_INTERVAL to small value), but have to be sufferred by tremendous > interrupts handling that eat significant CPU resources. >=20 > The patch disable pci interrupt and remove the interrupt handler, replace= it > with a low frequency (50ms) interrupt polling daemon which is implemented > by registering a alarm callback periodly, this save CPU time significentl= y: On a > typical x86 server with 2.1GHz CPU, with low latency configure (32us) we = saw > CPU usage from top commmand reduced from 20% to 0% on management > core in testpmd). >=20 > Also with the new method we can remove compile option: > I40E_ITR_INTERVAL which is used to balance between low latency and low > CPU usage previously. > Now we don't need it since we can reach both at same time. >=20 > Suggested-by: Jingjing Wu > Signed-off-by: Qi Zhang > --- > config/common_base | 2 -- > drivers/net/i40e/i40e_ethdev.c | 3 +-- > drivers/net/i40e/i40e_ethdev.h | 22 +++++++++++----------- > drivers/net/i40e/i40e_ethdev_vf.c | 36 ++++++++++++++-------------------= --- > 4 files changed, 26 insertions(+), 37 deletions(-) >=20 > diff --git a/config/common_base b/config/common_base index > 6b0d1cbbb..9e21c6865 100644 > --- a/config/common_base > +++ b/config/common_base > @@ -264,8 +264,6 @@ CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=3Dy > CONFIG_RTE_LIBRTE_I40E_16BYTE_RX_DESC=3Dn > CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF=3D64 > CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM=3D4 > -# interval up to 8160 us, aligned to 2 (or default value) > -CONFIG_RTE_LIBRTE_I40E_ITR_INTERVAL=3D-1 Seems " High Performance and per Packet Latency Tradeoff" is missed. >=20 > # > # Compile burst-oriented FM10K PMD > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethde= v.c > index 13c5d3296..c8f9566e0 100644 > --- a/drivers/net/i40e/i40e_ethdev.c > +++ b/drivers/net/i40e/i40e_ethdev.c > @@ -1829,8 +1829,7 @@ __vsi_queues_bind_intr(struct i40e_vsi *vsi, > uint16_t msix_vect, > /* Write first RX queue to Link list register as the head element */ > if (vsi->type !=3D I40E_VSI_SRIOV) { > uint16_t interval =3D > - > i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL, 1, > - pf->support_multi_driver); > + i40e_calc_itr_interval(1, pf->support_multi_driver); >=20 > if (msix_vect =3D=3D I40E_MISC_VEC_ID) { > I40E_WRITE_REG(hw, I40E_PFINT_LNKLST0, diff --git > a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h index > 11c4c76bd..599993dac 100644 > --- a/drivers/net/i40e/i40e_ethdev.h > +++ b/drivers/net/i40e/i40e_ethdev.h > @@ -178,7 +178,7 @@ enum i40e_flxpld_layer_idx { > #define I40E_ITR_INDEX_NONE 3 > #define I40E_QUEUE_ITR_INTERVAL_DEFAULT 32 /* 32 us */ > #define I40E_QUEUE_ITR_INTERVAL_MAX 8160 /* 8160 us */ > -#define I40E_VF_QUEUE_ITR_INTERVAL_DEFAULT 8160 /* 8160 us */ > +#define I40E_VF_QUEUE_ITR_INTERVAL_DEFAULT 32 /* 32 us */ > /* Special FW support this floating VEB feature */ #define > FLOATING_VEB_SUPPORTED_FW_MAJ 5 #define > FLOATING_VEB_SUPPORTED_FW_MIN 0 @@ -1328,17 +1328,17 @@ > i40e_align_floor(int n) } >=20 > static inline uint16_t > -i40e_calc_itr_interval(int16_t interval, bool is_pf, bool is_multi_drv) > +i40e_calc_itr_interval(bool is_pf, bool is_multi_drv) > { > - if (interval < 0 || interval > I40E_QUEUE_ITR_INTERVAL_MAX) { > - if (is_multi_drv) { > - interval =3D I40E_QUEUE_ITR_INTERVAL_MAX; > - } else { > - if (is_pf) > - interval =3D > I40E_QUEUE_ITR_INTERVAL_DEFAULT; > - else > - interval =3D > I40E_VF_QUEUE_ITR_INTERVAL_DEFAULT; > - } > + uint16_t interval =3D 0; > + > + if (is_multi_drv) { > + interval =3D I40E_QUEUE_ITR_INTERVAL_MAX; > + } else { > + if (is_pf) > + interval =3D I40E_QUEUE_ITR_INTERVAL_DEFAULT; > + else > + interval =3D I40E_VF_QUEUE_ITR_INTERVAL_DEFAULT; > } >=20 > /* Convert to hardware count, as writing each 1 represents 2 us */ > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c > b/drivers/net/i40e/i40e_ethdev_vf.c > index 804e44530..ad5c069e8 100644 > --- a/drivers/net/i40e/i40e_ethdev_vf.c > +++ b/drivers/net/i40e/i40e_ethdev_vf.c > @@ -44,6 +44,8 @@ > #define I40EVF_BUSY_WAIT_COUNT 50 > #define MAX_RESET_WAIT_CNT 20 >=20 > +#define I40EVF_ALARM_INTERVAL 50000 /* us */ > + > struct i40evf_arq_msg_info { > enum virtchnl_ops ops; > enum i40e_status_code result; > @@ -1133,7 +1135,7 @@ i40evf_init_vf(struct rte_eth_dev *dev) > struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > struct i40e_vf *vf =3D I40EVF_DEV_PRIVATE_TO_VF(dev->data- > >dev_private); > uint16_t interval =3D > - i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL, 0, 0); > + i40e_calc_itr_interval(0, 0); >=20 > vf->adapter =3D I40E_DEV_PRIVATE_TO_ADAPTER(dev->data- > >dev_private); > vf->dev_data =3D dev->data; > @@ -1370,7 +1372,7 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev) > * void > */ > static void > -i40evf_dev_interrupt_handler(void *param) > +i40evf_dev_alarm_handler(void *param) > { > struct rte_eth_dev *dev =3D (struct rte_eth_dev *)param; > struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > @@ -1399,6 +1401,8 @@ i40evf_dev_interrupt_handler(void *param) >=20 > done: > i40evf_enable_irq0(hw); > + rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, > + i40evf_dev_alarm_handler, dev); > } >=20 > static int > @@ -1442,12 +1446,8 @@ i40evf_dev_init(struct rte_eth_dev *eth_dev) > return -1; > } >=20 > - /* register callback func to eal lib */ > - rte_intr_callback_register(&pci_dev->intr_handle, > - i40evf_dev_interrupt_handler, (void *)eth_dev); > - > - /* enable uio intr after callback register */ > - rte_intr_enable(&pci_dev->intr_handle); > + rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, > + i40evf_dev_alarm_handler, eth_dev); >=20 > /* configure and enable device interrupt */ > i40evf_enable_irq0(hw); > @@ -1836,7 +1836,7 @@ i40evf_dev_rx_queue_intr_enable(struct > rte_eth_dev *dev, uint16_t queue_id) > struct rte_intr_handle *intr_handle =3D &pci_dev->intr_handle; > struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > uint16_t interval =3D > - i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL, 0, 0); > + i40e_calc_itr_interval(0, 0); > uint16_t msix_intr; >=20 > msix_intr =3D intr_handle->intr_vec[queue_id]; @@ -1859,8 +1859,6 > @@ i40evf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t > queue_id) >=20 > I40EVF_WRITE_FLUSH(hw); >=20 > - rte_intr_enable(&pci_dev->intr_handle); > - > return 0; > } >=20 > @@ -2023,10 +2021,8 @@ i40evf_dev_start(struct rte_eth_dev *dev) > * queue interrupt to other VFIO vectors. > * So clear uio/vfio intr/evevnfd first to avoid failure. > */ > - if (dev->data->dev_conf.intr_conf.rxq !=3D 0) { > - rte_intr_disable(intr_handle); Do we need to delete "rte_intr_disable" here? If so, should the comments be= changed? > + if (dev->data->dev_conf.intr_conf.rxq !=3D 0) > rte_intr_enable(intr_handle); > - } >=20 > i40evf_enable_queues_intr(dev); >=20 > @@ -2050,6 +2046,9 @@ i40evf_dev_stop(struct rte_eth_dev *dev) >=20 > PMD_INIT_FUNC_TRACE(); >=20 > + if (dev->data->dev_conf.intr_conf.rxq !=3D 0) > + rte_intr_disable(intr_handle); > + > if (hw->adapter_stopped =3D=3D 1) > return; > i40evf_stop_queues(dev); > @@ -2285,9 +2284,8 @@ static void > i40evf_dev_close(struct rte_eth_dev *dev) { > struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > - struct rte_pci_device *pci_dev =3D RTE_ETH_DEV_TO_PCI(dev); > - struct rte_intr_handle *intr_handle =3D &pci_dev->intr_handle; >=20 > + rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); > i40evf_dev_stop(dev); > i40e_dev_free_queues(dev); > /* > @@ -2300,12 +2298,6 @@ i40evf_dev_close(struct rte_eth_dev *dev) >=20 > i40evf_reset_vf(hw); > i40e_shutdown_adminq(hw); > - /* disable uio intr before callback unregister */ > - rte_intr_disable(intr_handle); > - > - /* unregister callback func from eal lib */ > - rte_intr_callback_unregister(intr_handle, > - i40evf_dev_interrupt_handler, dev); > i40evf_disable_irq0(hw); > } >=20 > -- > 2.13.6