From e1f2dcdb8f5c01bb1296c5c6c03d39cdf368b91a Mon Sep 17 00:00:00 2001 From: Gage Eads Date: Wed, 29 Nov 2017 21:08:33 -0600 Subject: [PATCH] event/sw: fix queue memory leak and multi-link bug 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") Cc: stable@dpdk.org 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 74976c01af..500387fe7e 100644 --- a/drivers/event/sw/sw_evdev.c +++ b/drivers/event/sw/sw_evdev.c @@ -34,6 +34,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)) { @@ -46,6 +47,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) { @@ -310,6 +320,23 @@ cleanup: 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) @@ -327,24 +354,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 70042e5f52..615aeb0f4d 100644 --- a/test/test/test_eventdev.c +++ b/test/test/test_eventdev.c @@ -811,6 +811,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", @@ -871,9 +874,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.20.1