On Mon, Mar 06, 2017 at 02:38:05PM +0000, Eads, Gage wrote: > > -----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.)
I see. It was not the case for HW PMD. But if it is helping SW PMD case then we can use your original patch. > > 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? I think your original patch is fine. IMO, you can change the commit message to reflect the issue and send v2 based on your existing v1.
