[dpdk-dev] [PATCH 1/2] event/sw: code refractor to reduce the fetch stall

[dpdk-dev] [PATCH 1/2] event/sw: code refractor to reduce the fetch stall

[Vipin Varghese]

With rearranging the code to prefetch the contents before
loop check increases performance from single and multistage
atomic pipeline.

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

diff --git a/drivers/event/sw/sw_evdev_scheduler.c b/drivers/event/sw/sw_evdev_scheduler.c
index e3a41e0..70d1970 100644
--- a/drivers/event/sw/sw_evdev_scheduler.c
+++ b/drivers/event/sw/sw_evdev_scheduler.c
@@ -44,12 +44,13 @@ sw_schedule_atomic_to_cq(struct sw_evdev *sw, struct sw_qid * const qid,
 	uint32_t qid_id = qid->id;
 
 	iq_dequeue_burst(sw, &qid->iq[iq_num], qes, count);
-	for (i = 0; i < count; i++) {
-		const struct rte_event *qe = &qes[i];
-		const uint16_t flow_id = SW_HASH_FLOWID(qes[i].flow_id);
-		struct sw_fid_t *fid = &qid->fids[flow_id];
-		int cq = fid->cq;
 
+	const struct rte_event *qe = &qes[0];
+	const uint16_t flow_id = SW_HASH_FLOWID(qes[0].flow_id);
+	struct sw_fid_t *fid = &qid->fids[flow_id];
+	int cq = fid->cq;
+
+	for (i = 0; i < count; i++) {
 		if (cq < 0) {
 			uint32_t cq_idx = qid->cq_next_tx++;
 			if (qid->cq_next_tx == qid->cq_num_mapped_cqs)
@@ -101,6 +102,13 @@ sw_schedule_atomic_to_cq(struct sw_evdev *sw, struct sw_qid * const qid,
 					&sw->cq_ring_space[cq]);
 			p->cq_buf_count = 0;
 		}
+
+		if (likely(i+1 < count)) {
+			qe = (qes + i + 1);
+			flow_id = SW_HASH_FLOWID(qes[i + 1].flow_id);
+			fid = &qid->fids[flow_id];
+			cq = fid->cq;
+		}
 	}
 	iq_put_back(sw, &qid->iq[iq_num], blocked_qes, nb_blocked);
 
-- 
2.7.4

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

[Vipin Varghese]

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;
 }
 
-- 
2.7.4

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

[Van Haaren, Harry]

> -----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.

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

[Varghese, Vipin]

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.

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

[Van Haaren, Harry]

> -----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.

Re: [dpdk-dev] [PATCH 1/2] event/sw: code refractor to reduce the fetch stall

[Jerin Jacob]

-----Original Message-----
> Date: Fri, 2 Mar 2018 01:04:59 +0530
> From: Vipin Varghese <vipin.varghese@intel.com>
> To: dev@dpdk.org, harry.van.haaren@intel.com
> CC: Vipin Varghese <vipin.varghese@intel.com>
> Subject: [dpdk-dev] [PATCH 1/2] event/sw: code refractor to reduce the
>  fetch stall
> X-Mailer: git-send-email 2.7.4
> 
> With rearranging the code to prefetch the contents before
> loop check increases performance from single and multistage
> atomic pipeline.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>

Harry,

Can you review this patches(1/1 and 1/2)so that I can include it in RC1 pull
request.

Re: [dpdk-dev] [PATCH 1/2] event/sw: code refractor to reduce the fetch stall

[Van Haaren, Harry]

Hey,

> -----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 1/2] event/sw: code refractor to reduce the fetch stall
> 
> With rearranging the code to prefetch the contents before
> loop check increases performance from single and multistage
> atomic pipeline.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>

There seems to be a compilation issue with this, see "const" in flow_id.
The flow_id variable is updated later, so it can't be marked const.

After the compilation fix, I see a small performance improvement here,
so you can include my Ack for V2 of this patch:

Acked-by: Harry van Haaren <harry.van.haaren@intel.com


> ---
>  drivers/event/sw/sw_evdev_scheduler.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/event/sw/sw_evdev_scheduler.c
> b/drivers/event/sw/sw_evdev_scheduler.c
> index e3a41e0..70d1970 100644
> --- a/drivers/event/sw/sw_evdev_scheduler.c
> +++ b/drivers/event/sw/sw_evdev_scheduler.c
> @@ -44,12 +44,13 @@ sw_schedule_atomic_to_cq(struct sw_evdev *sw, struct
> sw_qid * const qid,
>  	uint32_t qid_id = qid->id;
> 
>  	iq_dequeue_burst(sw, &qid->iq[iq_num], qes, count);
> -	for (i = 0; i < count; i++) {
> -		const struct rte_event *qe = &qes[i];
> -		const uint16_t flow_id = SW_HASH_FLOWID(qes[i].flow_id);
> -		struct sw_fid_t *fid = &qid->fids[flow_id];
> -		int cq = fid->cq;
> 
> +	const struct rte_event *qe = &qes[0];
> +	const uint16_t flow_id = SW_HASH_FLOWID(qes[0].flow_id);

     ^^^^^^ remove the const here.

[dpdk-dev] [PATCH v2] event/sw: code refractor to reduce the fetch stall

[Vipin Varghese]

With rearranging the code to prefetch the contents before
loop check increases performance from single and multistage
atomic pipeline.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
---

Changes in V2:
 - compilation fix for const flowid - Harry
 - Removal of sw_refill_pp_buf logic - Harry
---
 drivers/event/sw/sw_evdev_scheduler.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/event/sw/sw_evdev_scheduler.c b/drivers/event/sw/sw_evdev_scheduler.c
index e3a41e0..98dcf68 100644
--- a/drivers/event/sw/sw_evdev_scheduler.c
+++ b/drivers/event/sw/sw_evdev_scheduler.c
@@ -44,12 +44,13 @@
 	uint32_t qid_id = qid->id;
 
 	iq_dequeue_burst(sw, &qid->iq[iq_num], qes, count);
-	for (i = 0; i < count; i++) {
-		const struct rte_event *qe = &qes[i];
-		const uint16_t flow_id = SW_HASH_FLOWID(qes[i].flow_id);
-		struct sw_fid_t *fid = &qid->fids[flow_id];
-		int cq = fid->cq;
 
+	const struct rte_event *qe = &qes[i];
+	uint16_t flow_id = SW_HASH_FLOWID(qes[i].flow_id);
+	struct sw_fid_t *fid = &qid->fids[flow_id];
+	int cq = fid->cq;
+
+	for (i = 0; i < count; i++) {
 		if (cq < 0) {
 			uint32_t cq_idx = qid->cq_next_tx++;
 			if (qid->cq_next_tx == qid->cq_num_mapped_cqs)
@@ -101,6 +102,14 @@
 					&sw->cq_ring_space[cq]);
 			p->cq_buf_count = 0;
 		}
+
+		if (likely(i+1 < count)) {
+			qe = (qes + i + 1);
+			flow_id = SW_HASH_FLOWID(qes[i + 1].flow_id);
+			fid = &qid->fids[flow_id];
+			cq = fid->cq;
+		}
+
 	}
 	iq_put_back(sw, &qid->iq[iq_num], blocked_qes, nb_blocked);
 
-- 
1.9.1

[dpdk-dev] [PATCH v3] event/sw: code refractor to reduce the fetch stall

[Vipin Varghese]

With rearranging the code to prefetch the contents before
loop check increases performance from single and multistage
atomic pipeline.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
---

Changes in V3:
 - fix compilation for initial element - Vipin
 - fix the time and date for email sent - Vipin

Changes in V2:
 - compilation fix for const flowid - Harry
 - Removal of sw_refill_pp_buf logic - Harry
---
 drivers/event/sw/sw_evdev_scheduler.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/event/sw/sw_evdev_scheduler.c b/drivers/event/sw/sw_evdev_scheduler.c
index e3a41e0..5eb3157 100644
--- a/drivers/event/sw/sw_evdev_scheduler.c
+++ b/drivers/event/sw/sw_evdev_scheduler.c
@@ -44,12 +44,13 @@
 	uint32_t qid_id = qid->id;
 
 	iq_dequeue_burst(sw, &qid->iq[iq_num], qes, count);
-	for (i = 0; i < count; i++) {
-		const struct rte_event *qe = &qes[i];
-		const uint16_t flow_id = SW_HASH_FLOWID(qes[i].flow_id);
-		struct sw_fid_t *fid = &qid->fids[flow_id];
-		int cq = fid->cq;
 
+	const struct rte_event *qe = &qes[0];
+	uint16_t flow_id = SW_HASH_FLOWID(qes[0].flow_id);
+	struct sw_fid_t *fid = &qid->fids[flow_id];
+	int cq = fid->cq;
+
+	for (i = 0; i < count; i++) {
 		if (cq < 0) {
 			uint32_t cq_idx = qid->cq_next_tx++;
 			if (qid->cq_next_tx == qid->cq_num_mapped_cqs)
@@ -101,6 +102,14 @@
 					&sw->cq_ring_space[cq]);
 			p->cq_buf_count = 0;
 		}
+
+		if (likely(i+1 < count)) {
+			qe = (qes + i + 1);
+			flow_id = SW_HASH_FLOWID(qes[i + 1].flow_id);
+			fid = &qid->fids[flow_id];
+			cq = fid->cq;
+		}
+
 	}
 	iq_put_back(sw, &qid->iq[iq_num], blocked_qes, nb_blocked);
 
-- 
1.9.1

Re: [dpdk-dev] [PATCH v3] event/sw: code refractor to reduce the fetch stall

[Jerin Jacob]

-----Original Message-----
> Date: Thu, 5 Apr 2018 11:26:30 +0530
> From: Vipin Varghese <vipin.varghese@intel.com>
> To: dev@dpdk.org, harry.van.haaren@intel.com
> CC: jerin.jacob@caviumnetworks.com, Vipin Varghese
>  <vipin.varghese@intel.com>
> Subject: [PATCH v3] event/sw: code refractor to reduce the fetch stall
> X-Mailer: git-send-email 1.9.1
> 
> With rearranging the code to prefetch the contents before
> loop check increases performance from single and multistage
> atomic pipeline.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Applied to dpdk-next-eventdev/master. Thanks.

> 

Re: [dpdk-dev] [PATCH v3] event/sw: code refractor to reduce the fetch stall

[Van Haaren, Harry]

> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Thursday, April 5, 2018 9:53 AM
> To: Varghese, Vipin <vipin.varghese@intel.com>
> Cc: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>
> Subject: Re: [PATCH v3] event/sw: code refractor to reduce the fetch stall
> 
> -----Original Message-----
> > Date: Thu, 5 Apr 2018 11:26:30 +0530
> > From: Vipin Varghese <vipin.varghese@intel.com>
> > To: dev@dpdk.org, harry.van.haaren@intel.com
> > CC: jerin.jacob@caviumnetworks.com, Vipin Varghese
> >  <vipin.varghese@intel.com>
> > Subject: [PATCH v3] event/sw: code refractor to reduce the fetch stall
> > X-Mailer: git-send-email 1.9.1
> >
> > With rearranging the code to prefetch the contents before
> > loop check increases performance from single and multistage
> > atomic pipeline.
> >
> > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> Applied to dpdk-next-eventdev/master. Thanks.


Hi Jerin,

On further investigation into the patch, the performance gains seem to be quite workload dependent.

Hereby a request to drop this patch from the next-eventdev tree, so that
we can investigate further, and rework / refactor for a future release.

Regards, -Harry

Re: [dpdk-dev] [PATCH v3] event/sw: code refractor to reduce the fetch stall

[Jerin Jacob]

-----Original Message-----
> Date: Fri, 13 Apr 2018 16:19:54 +0000
> From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "Varghese, Vipin"
>  <vipin.varghese@intel.com>
> Subject: RE: [PATCH v3] event/sw: code refractor to reduce the fetch stall
> 
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Thursday, April 5, 2018 9:53 AM
> > To: Varghese, Vipin <vipin.varghese@intel.com>
> > Cc: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>
> > Subject: Re: [PATCH v3] event/sw: code refractor to reduce the fetch stall
> > 
> > -----Original Message-----
> > > Date: Thu, 5 Apr 2018 11:26:30 +0530
> > > From: Vipin Varghese <vipin.varghese@intel.com>
> > > To: dev@dpdk.org, harry.van.haaren@intel.com
> > > CC: jerin.jacob@caviumnetworks.com, Vipin Varghese
> > >  <vipin.varghese@intel.com>
> > > Subject: [PATCH v3] event/sw: code refractor to reduce the fetch stall
> > > X-Mailer: git-send-email 1.9.1
> > >
> > > With rearranging the code to prefetch the contents before
> > > loop check increases performance from single and multistage
> > > atomic pipeline.
> > >
> > > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > > Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
> > 
> > Applied to dpdk-next-eventdev/master. Thanks.
> 
> 
> Hi Jerin,

Hi Harry,

> 
> On further investigation into the patch, the performance gains seem to be quite workload dependent.
> 
> Hereby a request to drop this patch from the next-eventdev tree, so that
> we can investigate further, and rework / refactor for a future release.

Removed from next-eventdev tree and updated status in the patchwork.
http://dpdk.org/dev/patchwork/patch/37219/

> 
> Regards, -Harry