[dpdk-dev,2/2] event/sw: code refractor for sw_refill_pp_buf

Message ID 1519932900-10571-2-git-send-email-vipin.varghese@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Varghese, Vipin March 1, 2018, 7:35 p.m. UTC
  Code changes how shadow buffer are filled up in each calls.
Refilling the shadow buffer helped in improving 0.2 Mpps.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
---
 drivers/event/sw/sw_evdev_scheduler.c | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Van Haaren, Harry April 3, 2018, 12:50 p.m. UTC | #1
> -----Original Message-----
> From: Varghese, Vipin
> Sent: Thursday, March 1, 2018 7:35 PM
> To: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: Varghese, Vipin <vipin.varghese@intel.com>
> Subject: [PATCH 2/2] event/sw: code refractor for sw_refill_pp_buf
> 
> Code changes how shadow buffer are filled up in each calls.
> Refilling the shadow buffer helped in improving 0.2 Mpps.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> ---
>  drivers/event/sw/sw_evdev_scheduler.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/event/sw/sw_evdev_scheduler.c
> b/drivers/event/sw/sw_evdev_scheduler.c
> index 70d1970..a95a22a 100644
> --- a/drivers/event/sw/sw_evdev_scheduler.c
> +++ b/drivers/event/sw/sw_evdev_scheduler.c
> @@ -451,6 +451,10 @@ __pull_port_lb(struct sw_evdev *sw, uint32_t port_id, int
> allow_reorder)
>  		port->pp_buf_count--;
>  	} /* while (avail_qes) */
> 
> +	/* replensih buffers before next iteration */
> +	if (port->pp_buf_count == 0)
> +		sw_refill_pp_buf(sw, port);
> +
>  	return pkts_iter;
>  }


I see the goal here - to ensure that the port buffer has items when we next
enter this function, possibly reducing a stall waiting for the ring access.

In theory this is a good idea - in practice, I see a small performance degradation.
Hence, I suggest we drop this patch from the patchset, and merge 1/2 alone.
  
Varghese, Vipin April 4, 2018, 11:51 a.m. UTC | #2
Sure Harry, I am ok with your suggestion.

> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Tuesday, April 3, 2018 6:20 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH 2/2] event/sw: code refractor for sw_refill_pp_buf
> 
> > -----Original Message-----
> > From: Varghese, Vipin
> > Sent: Thursday, March 1, 2018 7:35 PM
> > To: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>
> > Cc: Varghese, Vipin <vipin.varghese@intel.com>
> > Subject: [PATCH 2/2] event/sw: code refractor for sw_refill_pp_buf
> >
> > Code changes how shadow buffer are filled up in each calls.
> > Refilling the shadow buffer helped in improving 0.2 Mpps.
> >
> > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > ---
> >  drivers/event/sw/sw_evdev_scheduler.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/event/sw/sw_evdev_scheduler.c
> > b/drivers/event/sw/sw_evdev_scheduler.c
> > index 70d1970..a95a22a 100644
> > --- a/drivers/event/sw/sw_evdev_scheduler.c
> > +++ b/drivers/event/sw/sw_evdev_scheduler.c
> > @@ -451,6 +451,10 @@ __pull_port_lb(struct sw_evdev *sw, uint32_t
> > port_id, int
> > allow_reorder)
> >  		port->pp_buf_count--;
> >  	} /* while (avail_qes) */
> >
> > +	/* replensih buffers before next iteration */
> > +	if (port->pp_buf_count == 0)
> > +		sw_refill_pp_buf(sw, port);
> > +
> >  	return pkts_iter;
> >  }
> 
> 
> I see the goal here - to ensure that the port buffer has items when we next enter
> this function, possibly reducing a stall waiting for the ring access.
> 
> In theory this is a good idea - in practice, I see a small performance degradation.
> Hence, I suggest we drop this patch from the patchset, and merge 1/2 alone.
  
Van Haaren, Harry April 4, 2018, 12:26 p.m. UTC | #3
> -----Original Message-----
> From: Varghese, Vipin
> Sent: Wednesday, April 4, 2018 12:52 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH 2/2] event/sw: code refractor for sw_refill_pp_buf
> 
> Sure Harry, I am ok with your suggestion.

Great!

Next steps;
1) Would you respin v3 the 1/2 patch to fix the compiler error?
   -- You may include my Acked-by: in the fixed commit.

2) Send v3 patch from above as reply to the 1/2 patch (http://dpdk.org/dev/patchwork/patch/35564/)
   -- Put Jerin on CC, as the patch should be ready for merge

3) Mark 1/2 and 2/2 (current revisions) as superseded to ensure 2/2 doesn't get merged :)


Cheers, -Harry


> > -----Original Message-----
> > From: Van Haaren, Harry
> > Sent: Tuesday, April 3, 2018 6:20 PM
> > To: Varghese, Vipin <vipin.varghese@intel.com>; dev@dpdk.org
> > Subject: RE: [PATCH 2/2] event/sw: code refractor for sw_refill_pp_buf
> >
> > > -----Original Message-----
> > > From: Varghese, Vipin
> > > Sent: Thursday, March 1, 2018 7:35 PM
> > > To: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>
> > > Cc: Varghese, Vipin <vipin.varghese@intel.com>
> > > Subject: [PATCH 2/2] event/sw: code refractor for sw_refill_pp_buf

<snip patch code changes>

> > I see the goal here - to ensure that the port buffer has items when we next
> enter
> > this function, possibly reducing a stall waiting for the ring access.
> >
> > In theory this is a good idea - in practice, I see a small performance
> degradation.
> > Hence, I suggest we drop this patch from the patchset, and merge 1/2 alone.
  

Patch

diff --git a/drivers/event/sw/sw_evdev_scheduler.c b/drivers/event/sw/sw_evdev_scheduler.c
index 70d1970..a95a22a 100644
--- a/drivers/event/sw/sw_evdev_scheduler.c
+++ b/drivers/event/sw/sw_evdev_scheduler.c
@@ -451,6 +451,10 @@  __pull_port_lb(struct sw_evdev *sw, uint32_t port_id, int allow_reorder)
 		port->pp_buf_count--;
 	} /* while (avail_qes) */
 
+	/* replensih buffers before next iteration */
+	if (port->pp_buf_count == 0)
+		sw_refill_pp_buf(sw, port);
+
 	return pkts_iter;
 }