From: "Eads, Gage" <gage.eads@intel.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
"Van Haaren, Harry" <harry.van.haaren@intel.com>,
"nipun.gupta@nxp.com" <nipun.gupta@nxp.com>
Subject: Re: [dpdk-dev] [PATCH] eventdev: Fix links_map initialization
Date: Mon, 6 Mar 2017 14:38:05 +0000 [thread overview]
Message-ID: <9184057F7FC11744A2107296B6B8EB1E01E6F9F5@FMSMSX108.amr.corp.intel.com> (raw)
In-Reply-To: <20170306130625.GA4250@localhost.localdomain>
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, March 6, 2017 7:06 AM
> To: Eads, Gage <gage.eads@intel.com>
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>;
> hemant.agrawal@nxp.com; Van Haaren, Harry <harry.van.haaren@intel.com>;
> nipun.gupta@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@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?
next prev parent reply other threads:[~2017-03-06 14:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-02 4:47 Gage Eads
2017-03-06 13:06 ` Jerin Jacob
2017-03-06 14:38 ` Eads, Gage [this message]
2017-03-06 16:51 ` Jerin Jacob
2017-03-06 17:02 ` [dpdk-dev] [PATCH v2] eventdev: Fix links_map initialization for sw PMD Gage Eads
2017-03-07 6:12 ` Jerin Jacob
2017-03-07 16:25 ` [dpdk-dev] [PATCH v3] eventdev: fix links map initialization for SW PMD Gage Eads
2017-03-13 8:52 ` Jerin Jacob
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9184057F7FC11744A2107296B6B8EB1E01E6F9F5@FMSMSX108.amr.corp.intel.com \
--to=gage.eads@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=harry.van.haaren@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=jerin.jacob@caviumnetworks.com \
--cc=nipun.gupta@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).