From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pavan Nikhilesh Bhagavatula Subject: Re: [PATCH] eventdev: use links_map to unlink queues Date: Tue, 12 Dec 2017 12:53:46 +0530 Message-ID: <20171212072345.blcsaqs6q22qiwwf@Pavan-LT> References: <20171211150528.13236-1-pbhagavatula@caviumnetworks.com> <9184057F7FC11744A2107296B6B8EB1E2BB16442@FMSMSX108.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org To: "Eads, Gage" , "jerin.jacob@caviumnetworks.com" , "Richardson, Bruce" , "nipun.gupta@nxp.com" , "santosh.shukla@caviumnetworks.com" , "Van Haaren, Harry" , "hemant.agrawal@nxp.com" Return-path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0084.outbound.protection.outlook.com [104.47.34.84]) by dpdk.org (Postfix) with ESMTP id 06695199B3 for ; Tue, 12 Dec 2017 08:24:05 +0100 (CET) Content-Disposition: inline In-Reply-To: <9184057F7FC11744A2107296B6B8EB1E2BB16442@FMSMSX108.amr.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, Dec 11, 2017 at 05:24:57PM +0000, Eads, Gage wrote: > > > -----Original Message----- > > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com] > > Sent: Monday, December 11, 2017 9:05 AM > > To: Eads, Gage ; jerin.jacob@caviumnetworks.com; > > Richardson, Bruce ; nipun.gupta@nxp.com; > > santosh.shukla@caviumnetworks.com; Van Haaren, Harry > > ; hemant.agrawal@nxp.com > > Cc: dev@dpdk.org; Pavan Nikhilesh > > Subject: [dpdk-dev] [PATCH] eventdev: use links_map to unlink queues > > > > The octeontx event device doesn't store the queues to port mapping as a result > > it cannot return the exact number of queues unlinked from a port when > > application wants to unlink all the queues mapped (supplies queues param as > > NULL). > > > > Using links_map we can determine the exact queues mapped to a specific port > > and unlink them. > > > > Signed-off-by: Pavan Nikhilesh > > --- > > lib/librte_eventdev/rte_eventdev.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_eventdev/rte_eventdev.c > > b/lib/librte_eventdev/rte_eventdev.c > > index e0c2a78..e17f8fc 100644 > > --- a/lib/librte_eventdev/rte_eventdev.c > > +++ b/lib/librte_eventdev/rte_eventdev.c > > @@ -888,7 +888,8 @@ rte_event_port_unlink(uint8_t dev_id, uint8_t port_id, { > > struct rte_eventdev *dev; > > uint8_t all_queues[RTE_EVENT_MAX_QUEUES_PER_DEV]; > > - int i, diag; > > + uint8_t linked_queues[RTE_EVENT_MAX_QUEUES_PER_DEV]; > > + int i, diag, j; > > uint16_t *links_map; > > > > RTE_EVENTDEV_VALID_DEVID_OR_ERRNO_RET(dev_id, -EINVAL, 0); > > @@ -918,6 +919,18 @@ rte_event_port_unlink(uint8_t dev_id, uint8_t port_id, > > rte_errno = -EINVAL; > > return 0; > > } > > + j = 0; > > + links_map = dev->data->links_map; > > + links_map += (port_id * RTE_EVENT_MAX_QUEUES_PER_DEV); > > + for (i = 0; i < nb_unlinks; i++) { > > + if (links_map[queues[i]] != > > + EVENT_QUEUE_SERVICE_PRIORITY_INVALID) { > > + linked_queues[j] = queues[i]; > > + j++; > > + } > > + } > > + queues = linked_queues; > > + nb_unlinks = j; > > Consider the following case where queues is non-NULL: nb_unlinks = 3, queues = [0, 5, 2], and queue 5 is not linked to the port. > > This new code block will unlink queues 0 and 2, and the function will return 2. However the documentation states that "if the return value is less than *nb_unlinks*, the remaining queues at the end of queues[] are not established", so a return value of 2 would imply that queues 0 and 5 were unlinked, but queue 2 was not. > > Perhaps this code could be moved to the "if (queues == NULL)" block, and in its place you can update nb_unlinks like so to handle the non-NULL queues case: > > links_map = dev->data->links_map; > links_map += (port_id * RTE_EVENT_MAX_QUEUES_PER_DEV); > for (i = 0; i < nb_unlinks; i++) { > if (links_map[queues[i]] != > EVENT_QUEUE_SERVICE_PRIORITY_INVALID) > break; > } > nb_unlinks = i; > > Thanks, > Gage Agreed, will send out a cleaned up v2 addressing both the cases. Thanks, Pavan.