[dpdk-dev] [PATCH] eventdev: Fix links_map initialization

Eads, Gage gage.eads at intel.com
Mon Mar 6 15:38:05 CET 2017


>  -----Original Message-----
>  From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
>  Sent: Monday, March 6, 2017 7:06 AM
>  To: Eads, Gage <gage.eads at intel.com>
>  Cc: dev at dpdk.org; Richardson, Bruce <bruce.richardson at intel.com>;
>  hemant.agrawal at nxp.com; Van Haaren, Harry <harry.van.haaren at intel.com>;
>  nipun.gupta at nxp.com
>  Subject: Re: [PATCH] eventdev: Fix links_map initialization
>  
>  On Wed, Mar 01, 2017 at 10:47:36PM -0600, Gage Eads wrote:
>  > This patch initializes the links_map array entries to
>  > EVENT_QUEUE_SERVICE_PRIORITY_INVALID, as expected by
>  > rte_event_port_links_get().
>  >
>  > Signed-off-by: Gage Eads <gage.eads at intel.com>
>  > ---
>  >  lib/librte_eventdev/rte_eventdev.c | 17 ++++++++++++-----
>  >  1 file changed, 12 insertions(+), 5 deletions(-)
>  >
>  > diff --git a/lib/librte_eventdev/rte_eventdev.c
>  > b/lib/librte_eventdev/rte_eventdev.c
>  > index 68bfc3b..b8cd92b 100644
>  > --- a/lib/librte_eventdev/rte_eventdev.c
>  > +++ b/lib/librte_eventdev/rte_eventdev.c
>  > @@ -190,6 +190,8 @@ rte_event_dev_queue_config(struct rte_eventdev
>  *dev, uint8_t nb_queues)
>  >  	return 0;
>  >  }
>  >
>  > +#define EVENT_QUEUE_SERVICE_PRIORITY_INVALID (0xdead)
>  > +
>  >  static inline int
>  >  rte_event_dev_port_config(struct rte_eventdev *dev, uint8_t nb_ports)
>  > { @@ -251,6 +253,9 @@ rte_event_dev_port_config(struct rte_eventdev
>  > *dev, uint8_t nb_ports)
>  >  					"nb_ports %u", nb_ports);
>  >  			return -(ENOMEM);
>  >  		}
>  > +		for (i = 0; i < nb_ports * RTE_EVENT_MAX_QUEUES_PER_DEV;
>  i++)
>  > +			dev->data->links_map[i] =
>  > +				EVENT_QUEUE_SERVICE_PRIORITY_INVALID;
>  >  	} else if (dev->data->ports != NULL && nb_ports != 0) {/* re-config */
>  >  		RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->port_release,
>  -ENOTSUP);
>  >
>  > @@ -305,6 +310,10 @@ rte_event_dev_port_config(struct rte_eventdev
>  > *dev, uint8_t nb_ports)
>  >
>  >  		if (nb_ports > old_nb_ports) {
>  >  			uint8_t new_ps = nb_ports - old_nb_ports;
>  > +			unsigned int old_links_map_end =
>  > +				old_nb_ports *
>  RTE_EVENT_MAX_QUEUES_PER_DEV;
>  > +			unsigned int links_map_end =
>  > +				nb_ports *
>  RTE_EVENT_MAX_QUEUES_PER_DEV;
>  >
>  >  			memset(ports + old_nb_ports, 0,
>  >  				sizeof(ports[0]) * new_ps);
>  > @@ -312,9 +321,9 @@ rte_event_dev_port_config(struct rte_eventdev
>  *dev, uint8_t nb_ports)
>  >  				sizeof(ports_dequeue_depth[0]) * new_ps);
>  >  			memset(ports_enqueue_depth + old_nb_ports, 0,
>  >  				sizeof(ports_enqueue_depth[0]) * new_ps);
>  > -			memset(links_map +
>  > -				(old_nb_ports *
>  RTE_EVENT_MAX_QUEUES_PER_DEV),
>  > -				0, sizeof(ports_enqueue_depth[0]) * new_ps);
>  > +			for (i = old_links_map_end; i < links_map_end; i++)
>  > +				links_map[i] =
>  > +
>  	EVENT_QUEUE_SERVICE_PRIORITY_INVALID;
>  
>  rte_event_port_setup() has rte_event_port_unlink() at the end of the function.
>  On rte_event_port_unlink, we are doing the same operation(writing
>  EVENT_QUEUE_SERVICE_PRIORITY_INVALID) and
>  rte_event_port_links_get() should be called after rte_event_dev_start(), If so,
>  Do you still think this duplicates writes are required? or Do you have any other
>  call sequence in mind?

Ah, I didn't realize that was a purpose of calling port_unlink at the end of
port_setup. There is, however, an issue with initializing through the port unlink
when called by rte_event_port_setup(). The for-loop in rte_event_port_unlink() to
reset the links_map runs from 0 to diag, and diag is 0 when the port is being set up
since it has no queues to unlink at that time. (This is at least true of the sw PMD,
but would be the case for others, I imagine.)

Perhaps a simpler form of this patch is to copy that for-loop, with the bound being
dev->data->nb_queues, into rte_event_port_setup() after rte_event_port_unlink()
is called (if it is successful). What do you think?


More information about the dev mailing list