On Wed, Jan 22, 2020 at 02:32:26PM +0000, Wang, Xiao W wrote:
> External Email
>
> ----------------------------------------------------------------------
> 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
>
Hi Xiao
Thanks for the explaination, I tested the scenario as explained by you
with and without your patch. I do see a huge difference in no of interrupts
sent by other queues and it is worth avoiding these interrupts as they
have no significance.
Thanks
Harman
Tested-by: Harman Kalra <hka...@marvell.com>
> > -----Original Message-----
> > From: Harman Kalra <hka...@marvell.com>
> > Sent: Wednesday, January 22, 2020 9:30 PM
> > To: Wang, Xiao W <xiao.w.w...@intel.com>
> > Cc: Hunt, David <david.h...@intel.com>; dev@dpdk.org; sta...@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: sta...@dpdk.org
> > >
> > > Signed-off-by: Xiao Wang <xiao.w.w...@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
> > >
