From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <gage.eads@intel.com>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
by dpdk.org (Postfix) with ESMTP id 088F72C54
for <dev@dpdk.org>; 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 <gage.eads@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 30 Nov 2017 03:09:14 -0000
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 <gage.eads@intel.com>
---
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 */
--
2.7.4