diff mbox

[dpdk-dev,2/5] eventdev: introduce specialized enqueue new op variant

Message ID 20170629141956.23132-2-jerin.jacob@caviumnetworks.com (mailing list archive)
State Accepted, archived
Delegated to: Jerin Jacob
Headers

Checks

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

Commit Message

Jerin Jacob June 29, 2017, 2:19 p.m. UTC 
  Introducing the rte_event_enqueue_new_burst() for enabling the
PMD, an optimization opportunity to optimize if all the events in
the enqueue burst has the op type of RTE_EVENT_OP_NEW.

If a PMD does not have any optimization opportunity
for this operation then the PMD can choose the generic enqueue
burst PMD callback as the fallback.

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
This patch is based on the following dicussion on ml
http://dpdk.org/ml/archives/dev/2017-June/068859.html
---
 drivers/event/octeontx/ssovf_evdev.c |  1 +
 drivers/event/sw/sw_evdev.c          |  1 +
 lib/librte_eventdev/rte_eventdev.h   | 51 ++++++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+)

Comments

Van Haaren, Harry June 30, 2017, 8:40 a.m. UTC | #1 
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Thursday, June 29, 2017 3:20 PM
> To: dev@dpdk.org
<snip>
> diff --git a/drivers/event/octeontx/ssovf_evdev.c b/drivers/event/octeontx/ssovf_evdev.c
> index 8dc7b2ef8..0d0c6a186 100644
> --- a/drivers/event/octeontx/ssovf_evdev.c
> +++ b/drivers/event/octeontx/ssovf_evdev.c
> @@ -158,6 +158,7 @@ ssovf_fastpath_fns_set(struct rte_eventdev *dev)
>  	dev->schedule      = NULL;
>  	dev->enqueue       = ssows_enq;
>  	dev->enqueue_burst = ssows_enq_burst;
> +	dev->enqueue_new_burst = ssows_enq_burst;
>  	dev->dequeue       = ssows_deq;
>  	dev->dequeue_burst = ssows_deq_burst;
> 
> diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
> index fe2a61e2f..951ad1b33 100644
> --- a/drivers/event/sw/sw_evdev.c
> +++ b/drivers/event/sw/sw_evdev.c
> @@ -796,6 +796,7 @@ sw_probe(struct rte_vdev_device *vdev)
>  	dev->dev_ops = &evdev_sw_ops;
>  	dev->enqueue = sw_event_enqueue;
>  	dev->enqueue_burst = sw_event_enqueue_burst;
> +	dev->enqueue_new_burst = sw_event_enqueue_burst;
>  	dev->dequeue = sw_event_dequeue;
>  	dev->dequeue_burst = sw_event_dequeue_burst;
>  	dev->schedule = sw_event_schedule;


I think it is possible to do this pointer-setting of new_burst() in eventdev.c, instead of adding the new_burst() to each PMD individually?
During rte_eventdev_configure(), if the dev->enqueue_new_burst() function is NULL, just point it at the ordinary one;

if (!dev->enqueue_new_burst)
    dev->enqueue_new_burst = dev->enqueue_burst;


This saves per-PMD changes for adding new parallel function pointers - and avoids PMDs accidentally not being updated. With that change;

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
Jerin Jacob June 30, 2017, 9:11 a.m. UTC | #2 
-----Original Message-----
> Date: Fri, 30 Jun 2017 08:40:06 +0000
> From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org"
>  <dev@dpdk.org>
> CC: "Richardson, Bruce" <bruce.richardson@intel.com>,
>  "hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>, "Eads, Gage"
>  <gage.eads@intel.com>, "nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
>  "Vangati, Narender" <narender.vangati@intel.com>, "Rao, Nikhil"
>  <nikhil.rao@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 2/5] eventdev: introduce specialized enqueue
>  new op variant
> 
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Thursday, June 29, 2017 3:20 PM
> > To: dev@dpdk.org
> <snip>
> > diff --git a/drivers/event/octeontx/ssovf_evdev.c b/drivers/event/octeontx/ssovf_evdev.c
> > index 8dc7b2ef8..0d0c6a186 100644
> > --- a/drivers/event/octeontx/ssovf_evdev.c
> > +++ b/drivers/event/octeontx/ssovf_evdev.c
> > @@ -158,6 +158,7 @@ ssovf_fastpath_fns_set(struct rte_eventdev *dev)
> >  	dev->schedule      = NULL;
> >  	dev->enqueue       = ssows_enq;
> >  	dev->enqueue_burst = ssows_enq_burst;
> > +	dev->enqueue_new_burst = ssows_enq_burst;
> >  	dev->dequeue       = ssows_deq;
> >  	dev->dequeue_burst = ssows_deq_burst;
> > 
> > diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
> > index fe2a61e2f..951ad1b33 100644
> > --- a/drivers/event/sw/sw_evdev.c
> > +++ b/drivers/event/sw/sw_evdev.c
> > @@ -796,6 +796,7 @@ sw_probe(struct rte_vdev_device *vdev)
> >  	dev->dev_ops = &evdev_sw_ops;
> >  	dev->enqueue = sw_event_enqueue;
> >  	dev->enqueue_burst = sw_event_enqueue_burst;
> > +	dev->enqueue_new_burst = sw_event_enqueue_burst;
> >  	dev->dequeue = sw_event_dequeue;
> >  	dev->dequeue_burst = sw_event_dequeue_burst;
> >  	dev->schedule = sw_event_schedule;
> 
> 
> I think it is possible to do this pointer-setting of new_burst() in eventdev.c, instead of adding the new_burst() to each PMD individually?
> During rte_eventdev_configure(), if the dev->enqueue_new_burst() function is NULL, just point it at the ordinary one;

I thought so, But it will break in multi process use case as on probe() we are
updating the callbacks for secondary process. Doing it in probe() may be very
early as some PMD may update the callback anywhere on or before rte_eventdev_start().

Thoughts?

> 
> if (!dev->enqueue_new_burst)
>     dev->enqueue_new_burst = dev->enqueue_burst;
> 
> 
> This saves per-PMD changes for adding new parallel function pointers - and avoids PMDs accidentally not being updated. With that change;
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
Van Haaren, Harry June 30, 2017, 9:14 a.m. UTC | #3 
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Friday, June 30, 2017 10:12 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com;
> Eads, Gage <gage.eads@intel.com>; nipun.gupta@nxp.com; Vangati, Narender
> <narender.vangati@intel.com>; Rao, Nikhil <nikhil.rao@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 2/5] eventdev: introduce specialized enqueue new op variant
> 
> -----Original Message-----
> > Date: Fri, 30 Jun 2017 08:40:06 +0000
> > From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org"
> >  <dev@dpdk.org>
> > CC: "Richardson, Bruce" <bruce.richardson@intel.com>,
> >  "hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>, "Eads, Gage"
> >  <gage.eads@intel.com>, "nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
> >  "Vangati, Narender" <narender.vangati@intel.com>, "Rao, Nikhil"
> >  <nikhil.rao@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH 2/5] eventdev: introduce specialized enqueue
> >  new op variant
> >
> > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > > Sent: Thursday, June 29, 2017 3:20 PM
> > > To: dev@dpdk.org
> > <snip>
> > > diff --git a/drivers/event/octeontx/ssovf_evdev.c
> b/drivers/event/octeontx/ssovf_evdev.c
> > > index 8dc7b2ef8..0d0c6a186 100644
> > > --- a/drivers/event/octeontx/ssovf_evdev.c
> > > +++ b/drivers/event/octeontx/ssovf_evdev.c
> > > @@ -158,6 +158,7 @@ ssovf_fastpath_fns_set(struct rte_eventdev *dev)
> > >  	dev->schedule      = NULL;
> > >  	dev->enqueue       = ssows_enq;
> > >  	dev->enqueue_burst = ssows_enq_burst;
> > > +	dev->enqueue_new_burst = ssows_enq_burst;
> > >  	dev->dequeue       = ssows_deq;
> > >  	dev->dequeue_burst = ssows_deq_burst;
> > >
> > > diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
> > > index fe2a61e2f..951ad1b33 100644
> > > --- a/drivers/event/sw/sw_evdev.c
> > > +++ b/drivers/event/sw/sw_evdev.c
> > > @@ -796,6 +796,7 @@ sw_probe(struct rte_vdev_device *vdev)
> > >  	dev->dev_ops = &evdev_sw_ops;
> > >  	dev->enqueue = sw_event_enqueue;
> > >  	dev->enqueue_burst = sw_event_enqueue_burst;
> > > +	dev->enqueue_new_burst = sw_event_enqueue_burst;
> > >  	dev->dequeue = sw_event_dequeue;
> > >  	dev->dequeue_burst = sw_event_dequeue_burst;
> > >  	dev->schedule = sw_event_schedule;
> >
> >
> > I think it is possible to do this pointer-setting of new_burst() in eventdev.c, instead
> of adding the new_burst() to each PMD individually?
> > During rte_eventdev_configure(), if the dev->enqueue_new_burst() function is NULL, just
> point it at the ordinary one;
> 
> I thought so, But it will break in multi process use case as on probe() we are
> updating the callbacks for secondary process. Doing it in probe() may be very
> early as some PMD may update the callback anywhere on or before rte_eventdev_start().
> 
> Thoughts?

Ah I forgot about the multi-proc init during my review. The current solution is more appropriate then, so this version 

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

Patch

diff --git a/drivers/event/octeontx/ssovf_evdev.c b/drivers/event/octeontx/ssovf_evdev.c
index 8dc7b2ef8..0d0c6a186 100644
--- a/drivers/event/octeontx/ssovf_evdev.c
+++ b/drivers/event/octeontx/ssovf_evdev.c
@@ -158,6 +158,7 @@  ssovf_fastpath_fns_set(struct rte_eventdev *dev)
 	dev->schedule      = NULL;
 	dev->enqueue       = ssows_enq;
 	dev->enqueue_burst = ssows_enq_burst;
+	dev->enqueue_new_burst = ssows_enq_burst;
 	dev->dequeue       = ssows_deq;
 	dev->dequeue_burst = ssows_deq_burst;
 
diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index fe2a61e2f..951ad1b33 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -796,6 +796,7 @@  sw_probe(struct rte_vdev_device *vdev)
 	dev->dev_ops = &evdev_sw_ops;
 	dev->enqueue = sw_event_enqueue;
 	dev->enqueue_burst = sw_event_enqueue_burst;
+	dev->enqueue_new_burst = sw_event_enqueue_burst;
 	dev->dequeue = sw_event_dequeue;
 	dev->dequeue_burst = sw_event_dequeue_burst;
 	dev->schedule = sw_event_schedule;
diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index c4d623a62..132f75fda 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -1065,6 +1065,8 @@  struct rte_eventdev {
 	/**< Pointer to PMD enqueue function. */
 	event_enqueue_burst_t enqueue_burst;
 	/**< Pointer to PMD enqueue burst function. */
+	event_enqueue_burst_t enqueue_new_burst;
+	/**< Pointer to PMD enqueue burst function(op new variant) */
 	event_dequeue_t dequeue;
 	/**< Pointer to PMD dequeue function. */
 	event_dequeue_burst_t dequeue_burst;
@@ -1181,6 +1183,55 @@  rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
 }
 
 /**
+ * Enqueue a burst of events objects of operation type *RTE_EVENT_OP_NEW* on
+ * an event device designated by its *dev_id* through the event port specified
+ * by *port_id*.
+ *
+ * Provides the same functionality as rte_event_enqueue_burst(), expect that
+ * application can use this API when the all objects in the burst contains
+ * the enqueue operation of the type *RTE_EVENT_OP_NEW*. This specialized
+ * function can provide the additional hint to the PMD and optimize if possible.
+ *
+ * The rte_event_enqueue_new_burst() result is undefined if the enqueue burst
+ * has event object of operation type != RTE_EVENT_OP_NEW.
+ *
+ * @param dev_id
+ *   The identifier of the device.
+ * @param port_id
+ *   The identifier of the event port.
+ * @param ev
+ *   Points to an array of *nb_events* objects of type *rte_event* structure
+ *   which contain the event object enqueue operations to be processed.
+ * @param nb_events
+ *   The number of event objects to enqueue, typically number of
+ *   rte_event_port_enqueue_depth() available for this port.
+ *
+ * @return
+ *   The number of event objects actually enqueued on the event device. The
+ *   return value can be less than the value of the *nb_events* parameter when
+ *   the event devices queue is full or if invalid parameters are specified in a
+ *   *rte_event*. If the return value is less than *nb_events*, the remaining
+ *   events at the end of ev[] are not consumed and the caller has to take care
+ *   of them, and rte_errno is set accordingly. Possible errno values include:
+ *   - -EINVAL  The port ID is invalid, device ID is invalid, an event's queue
+ *              ID is invalid, or an event's sched type doesn't match the
+ *              capabilities of the destination queue.
+ *   - -ENOSPC  The event port was backpressured and unable to enqueue
+ *              one or more events. This error code is only applicable to
+ *              closed systems.
+ * @see rte_event_port_enqueue_depth() rte_event_enqueue_burst()
+ */
+static inline uint16_t
+rte_event_enqueue_new_burst(uint8_t dev_id, uint8_t port_id,
+			const struct rte_event ev[], uint16_t nb_events)
+{
+	const struct rte_eventdev *dev = &rte_eventdevs[dev_id];
+
+	return __rte_event_enqueue_burst(dev_id, port_id, ev, nb_events,
+			dev->enqueue_new_burst);
+}
+
+/**
  * Converts nanoseconds to *timeout_ticks* value for rte_event_dequeue_burst()
  *
  * If the device is configured with RTE_EVENT_DEV_CFG_PER_DEQUEUE_TIMEOUT flag