DPDK patches and discussions
 help / color / mirror / Atom feed
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>
Subject: Re: [dpdk-dev] [PATCH 2/2] l3fwd-power: fix interrupt disable
Date: Wed, 22 Jan 2020 14:32:26 +0000	[thread overview]
Message-ID: <B7F2E978279D1D49A3034B7786DACF407B06E2FA@SHSMSX106.ccr.corp.intel.com> (raw)
In-Reply-To: <20200122133004.GA45794@outlook.office365.com>

Hi Kalra,

This patch is more about bug fix on user interrupt, no powering saving tuning.
The target scenario is, a worker core is dealing with 2 Rx queues, and it go sleep due to no traffic for some time,
and then the first queue has new traffic arrived, and wakes up this core, so this worker core is busy polling again.
The issue is that even though the core comes to polling state, the second queue will still send interrupt (if packet flushes in) to host.. This is what 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
> 
> On Mon, Jan 20, 2020 at 10:06:57PM -0500, Xiao Wang wrote:
> > Since all related queues' interrupts are turned on before epoll, we need
> > 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 = ((uintptr_t)data) >> CHAR_BIT;
> >  		queue_id = ((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 = 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);
> 
> Hi Wang
> 
> 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.
> 
> Thanks
> Harman
> 
> >  		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
> >

  reply	other threads:[~2020-01-22 14:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-21  3:06 [dpdk-dev] [PATCH 0/2] l3fwd-power fixes Xiao Wang
2020-01-21  3:06 ` [dpdk-dev] [PATCH 1/2] l3fwd-power: fix a typo Xiao Wang
2020-02-14 10:46   ` Hunt, David
2020-02-14 10:46   ` Hunt, David
2020-01-21  3:06 ` [dpdk-dev] [PATCH 2/2] l3fwd-power: fix interrupt disable Xiao Wang
2020-01-22 13:30   ` Harman Kalra
2020-01-22 14:32     ` Wang, Xiao W [this message]
2020-01-23 17:42       ` [dpdk-dev] [EXT] " Harman Kalra
2020-01-30 17:40   ` [dpdk-dev] " Liang, Ma
2020-02-14 10:43     ` Hunt, David
2020-02-05 17:25   ` Liang, Ma
2020-02-14 12:46 ` [dpdk-dev] [PATCH 0/2] l3fwd-power fixes David Marchand

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=B7F2E978279D1D49A3034B7786DACF407B06E2FA@SHSMSX106.ccr.corp.intel.com \
    --to=xiao.w.wang@intel.com \
    --cc=david.hunt@intel.com \
    --cc=dev@dpdk.org \
    --cc=hkalra@marvell.com \
    --cc=stable@dpdk.org \
    /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).