> -----Original Message----- > From: Jerin Jacob [mailto:jerin.ja...@caviumnetworks.com] > Sent: Monday, March 6, 2017 7:06 AM > To: Eads, Gage <gage.e...@intel.com> > Cc: dev@dpdk.org; Richardson, Bruce <bruce.richard...@intel.com>; > hemant.agra...@nxp.com; Van Haaren, Harry <harry.van.haa...@intel.com>; > nipun.gu...@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.e...@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?
