From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 2B07CA0530;
	Wed, 22 Jan 2020 15:32:33 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id F07583772;
	Wed, 22 Jan 2020 15:32:32 +0100 (CET)
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by dpdk.org (Postfix) with ESMTP id 1B0062F4F;
 Wed, 22 Jan 2020 15:32:30 +0100 (CET)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga006.jf.intel.com ([10.7.209.51])
 by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 22 Jan 2020 06:32:29 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.70,350,1574150400"; d="scan'208";a="229356070"
Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201])
 by orsmga006.jf.intel.com with ESMTP; 22 Jan 2020 06:32:29 -0800
Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by
 FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS)
 id 14.3.439.0; Wed, 22 Jan 2020 06:32:28 -0800
Received: from shsmsx106.ccr.corp.intel.com ([169.254.10.139]) by
 shsmsx102.ccr.corp.intel.com ([169.254.2.202]) with mapi id 14.03.0439.000;
 Wed, 22 Jan 2020 22:32:27 +0800
From: "Wang, Xiao W" <xiao.w.wang@intel.com>
To: Harman Kalra <hkalra@marvell.com>
CC: "Hunt, David" <david.hunt@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
 "stable@dpdk.org" <stable@dpdk.org>
Thread-Topic: [dpdk-dev] [PATCH 2/2] l3fwd-power: fix interrupt disable
Thread-Index: AQHVz6QXCX+0gzBuR0qz3niTpkguUqf2KtEAgACUpHA=
Date: Wed, 22 Jan 2020 14:32:26 +0000
Message-ID: <B7F2E978279D1D49A3034B7786DACF407B06E2FA@SHSMSX106.ccr.corp.intel.com>
References: <20200121030657.39048-1-xiao.w.wang@intel.com>
 <20200121030657.39048-3-xiao.w.wang@intel.com>
 <20200122133004.GA45794@outlook.office365.com>
In-Reply-To: <20200122133004.GA45794@outlook.office365.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 2/2] l3fwd-power: fix interrupt disable
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Kalra,

This patch is more about bug fix on user interrupt, no powering saving tuni=
ng.
The target scenario is, a worker core is dealing with 2 Rx queues, and it g=
o sleep due to no traffic for some time,
and then the first queue has new traffic arrived, and wakes up this core, s=
o this worker core is busy polling again.
The issue is that even though the core comes to polling state, the second q=
ueue will still send interrupt (if packet flushes in) to host.. This is wha=
t the core doesn't need, and unnecessary MSIX messages can cause throughput=
 to degrade.

Best Regards,
Xiao

> -----Original Message-----
> From: Harman Kalra <hkalra@marvell.com>
> Sent: Wednesday, January 22, 2020 9:30 PM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: Hunt, David <david.hunt@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] l3fwd-power: fix interrupt disable
>=20
> On Mon, Jan 20, 2020 at 10:06:57PM -0500, Xiao Wang wrote:
> > Since all related queues' interrupts are turned on before epoll, we nee=
d
> > to turn off all the interrupts after wakeup. This patch fixes the issue
> > of only turning off the interrupted queues.
> >
> > Fixes: b736d64787fc ("examples/l3fwd-power: disable Rx interrupt when
> waking up")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > ---
> >  examples/l3fwd-power/main.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
> > index ffcc7ecf4..e9b2cb5b3 100644
> > --- a/examples/l3fwd-power/main.c
> > +++ b/examples/l3fwd-power/main.c
> > @@ -880,9 +880,6 @@ sleep_until_rx_interrupt(int num)
> >  		port_id =3D ((uintptr_t)data) >> CHAR_BIT;
> >  		queue_id =3D ((uintptr_t)data) &
> >  			RTE_LEN2MASK(CHAR_BIT, uint8_t);
> > -		rte_spinlock_lock(&(locks[port_id]));
> > -		rte_eth_dev_rx_intr_disable(port_id, queue_id);
> > -		rte_spinlock_unlock(&(locks[port_id]));
> >  		RTE_LOG(INFO, L3FWD_POWER,
> >  			"lcore %u is waked up from rx interrupt on"
> >  			" port %d queue %d\n",
> > @@ -892,7 +889,7 @@ sleep_until_rx_interrupt(int num)
> >  	return 0;
> >  }
> >
> > -static void turn_on_intr(struct lcore_conf *qconf)
> > +static void turn_on_off_intr(struct lcore_conf *qconf, bool on)
> >  {
> >  	int i;
> >  	struct lcore_rx_queue *rx_queue;
> > @@ -905,7 +902,10 @@ static void turn_on_intr(struct lcore_conf *qconf)
> >  		queue_id =3D rx_queue->queue_id;
> >
> >  		rte_spinlock_lock(&(locks[port_id]));
> > -		rte_eth_dev_rx_intr_enable(port_id, queue_id);
> > +		if (on)
> > +			rte_eth_dev_rx_intr_enable(port_id, queue_id);
> > +		else
> > +			rte_eth_dev_rx_intr_disable(port_id, queue_id);
>=20
> Hi Wang
>=20
> I tested this patch on octeontx2 platform and have some queries
> regarding the same:
> Difference what I observed with this patch is, you are disabling
> interrupts for all the queues handled by the core which woke up but
> what is the advantage of doing so?
> I dont see any difference wrt octeontx2, with and without this patch in
> term of power saving. Can you please explain what I am missing.
>=20
> Thanks
> Harman
>=20
> >  		rte_spinlock_unlock(&(locks[port_id]));
> >  	}
> >  }
> > @@ -1340,9 +1340,10 @@ main_loop(__attribute__((unused)) void
> *dummy)
> >  			else {
> >  				/* suspend until rx interrupt triggers */
> >  				if (intr_en) {
> > -					turn_on_intr(qconf);
> > +					turn_on_off_intr(qconf, 1);
> >  					sleep_until_rx_interrupt(
> >  						qconf->n_rx_queue);
> > +					turn_on_off_intr(qconf, 0);
> >  					/**
> >  					 * start receiving packets immediately
> >  					 */
> > --
> > 2.15.1
> >