From patchwork Thu Nov 30 03:08:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eads, Gage" X-Patchwork-Id: 31767 X-Patchwork-Delegate: jerinj@marvell.com Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 807592C55; Thu, 30 Nov 2017 04:09:16 +0100 (CET) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 088F72C54 for ; Thu, 30 Nov 2017 04:09:13 +0100 (CET) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Nov 2017 19:09:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,339,1508828400"; d="scan'208";a="179388539" Received: from txasoft-yocto.an.intel.com (HELO txasoft-yocto.an.intel.com.) ([10.123.72.111]) by orsmga005.jf.intel.com with ESMTP; 29 Nov 2017 19:09:12 -0800 From: Gage Eads To: dev@dpdk.org Cc: jerin.jacob@caviumnetworks.com, harry.van.haaren@intel.com, bruce.richardson@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com, santosh.shukla@caviumnetworks.com, pbhagavatula@caviumnetworks.com Date: Wed, 29 Nov 2017 21:08:33 -0600 Message-Id: <1512011314-19682-1-git-send-email-gage.eads@intel.com> X-Mailer: git-send-email 2.7.4 Subject: [dpdk-dev] [PATCH 1/2] event/sw: fix queue memory leak and multi-link bug X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit reinitializes a queue before it is reconfigured, such that reorder buffer memory is not leaked. This bug masked a few other problems, which this commit corrects as well: - sw_port_link() allowed a port to link to a queue twice, such that the port could then successfully unlink the queue twice. Now the link function checks whether a port is already linked to the queue, and if so returns success but doesn't assign the a port a second slot in the queue's cq map. - test_eventdev.c's test_eventdev_unlink() was unlinking a queue twice from the same port, and expecting the second unlink to succeed. Now the test unlinks, links, then unlinks again. - test_eventdev.c's test_eventdev_link_get() was linking a single queue but expecting the unlink function to return nb_queues (where nb_queues > 1). The test now checks for a return value of 1. Fixes: 5ffb2f142d95 ("event/sw: support event queues") Fixes: 371a688fc159 ("event/sw: support linking queues to ports") Fixes: f8f9d233ea0e ("test/eventdev: add unit tests") Signed-off-by: Gage Eads Acked-by: Harry van Haaren --- drivers/event/sw/sw_evdev.c | 46 +++++++++++++++++++++++++++++---------------- test/test/test_eventdev.c | 7 +++++-- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c index aed8b72..5d03b7b 100644 --- a/drivers/event/sw/sw_evdev.c +++ b/drivers/event/sw/sw_evdev.c @@ -63,6 +63,7 @@ sw_port_link(struct rte_eventdev *dev, void *port, const uint8_t queues[], RTE_SET_USED(priorities); for (i = 0; i < num; i++) { struct sw_qid *q = &sw->qids[queues[i]]; + unsigned int j; /* check for qid map overflow */ if (q->cq_num_mapped_cqs >= RTE_DIM(q->cq_map)) { @@ -75,6 +76,15 @@ sw_port_link(struct rte_eventdev *dev, void *port, const uint8_t queues[], break; } + for (j = 0; j < q->cq_num_mapped_cqs; j++) { + if (q->cq_map[j] == p->id) + break; + } + + /* check if port is already linked */ + if (j < q->cq_num_mapped_cqs) + continue; + if (q->type == SW_SCHED_TYPE_DIRECT) { /* check directed qids only map to one port */ if (p->num_qids_mapped > 0) { @@ -339,6 +349,23 @@ qid_init(struct sw_evdev *sw, unsigned int idx, int type, return -EINVAL; } +static void +sw_queue_release(struct rte_eventdev *dev, uint8_t id) +{ + struct sw_evdev *sw = sw_pmd_priv(dev); + struct sw_qid *qid = &sw->qids[id]; + uint32_t i; + + for (i = 0; i < SW_IQS_MAX; i++) + iq_ring_destroy(qid->iq[i]); + + if (qid->type == RTE_SCHED_TYPE_ORDERED) { + rte_free(qid->reorder_buffer); + rte_ring_free(qid->reorder_buffer_freelist); + } + memset(qid, 0, sizeof(*qid)); +} + static int sw_queue_setup(struct rte_eventdev *dev, uint8_t queue_id, const struct rte_event_queue_conf *conf) @@ -370,24 +397,11 @@ sw_queue_setup(struct rte_eventdev *dev, uint8_t queue_id, } struct sw_evdev *sw = sw_pmd_priv(dev); - return qid_init(sw, queue_id, type, conf); -} - -static void -sw_queue_release(struct rte_eventdev *dev, uint8_t id) -{ - struct sw_evdev *sw = sw_pmd_priv(dev); - struct sw_qid *qid = &sw->qids[id]; - uint32_t i; - for (i = 0; i < SW_IQS_MAX; i++) - iq_ring_destroy(qid->iq[i]); + if (sw->qids[queue_id].initialized) + sw_queue_release(dev, queue_id); - if (qid->type == RTE_SCHED_TYPE_ORDERED) { - rte_free(qid->reorder_buffer); - rte_ring_free(qid->reorder_buffer_freelist); - } - memset(qid, 0, sizeof(*qid)); + return qid_init(sw, queue_id, type, conf); } static void diff --git a/test/test/test_eventdev.c b/test/test/test_eventdev.c index d6ade78..2a55a7a 100644 --- a/test/test/test_eventdev.c +++ b/test/test/test_eventdev.c @@ -840,6 +840,9 @@ test_eventdev_unlink(void) for (i = 0; i < nb_queues; i++) queues[i] = i; + ret = rte_event_port_link(TEST_DEV_ID, 0, NULL, NULL, 0); + TEST_ASSERT(ret >= 0, "Failed to link with NULL device%d", + TEST_DEV_ID); ret = rte_event_port_unlink(TEST_DEV_ID, 0, queues, nb_queues); TEST_ASSERT(ret == nb_queues, "Failed to unlink(device%d) ret=%d", @@ -900,9 +903,9 @@ test_eventdev_link_get(void) ret = rte_event_port_links_get(TEST_DEV_ID, 0, queues, priorities); TEST_ASSERT(ret == 1, "(%d)Wrong link get ret=%d expected=%d", TEST_DEV_ID, ret, 1); - /* unlink all*/ + /* unlink the queue */ ret = rte_event_port_unlink(TEST_DEV_ID, 0, NULL, 0); - TEST_ASSERT(ret == nb_queues, "Failed to unlink(device%d) ret=%d", + TEST_ASSERT(ret == 1, "Failed to unlink(device%d) ret=%d", TEST_DEV_ID, ret); /* 4links and 2 unlinks */