From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id A6FDB322C; Fri, 23 Nov 2018 15:39:06 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DD11181F10; Fri, 23 Nov 2018 14:39:05 +0000 (UTC) Received: from [10.36.112.54] (ovpn-112-54.ams2.redhat.com [10.36.112.54]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 97C605C236; Fri, 23 Nov 2018 14:39:03 +0000 (UTC) From: Maxime Coquelin To: Hari Kumar Vemula , dev@dpdk.org Cc: byron.marohn@intel.com, reshma.pattan@intel.com, pablo.de.lara.guarch@intel.com, stable@dpdk.org References: <1542109533-14283-1-git-send-email-hari.kumarx.vemula@intel.com> <1542194265-16156-1-git-send-email-hari.kumarx.vemula@intel.com> Message-ID: <1266315a-3ba9-5f9e-b9c8-d17c491b2547@redhat.com> Date: Fri, 23 Nov 2018 15:39:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Fri, 23 Nov 2018 14:39:05 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2] lib/efd: fix to free tail queue entry after use 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: , X-List-Received-Date: Fri, 23 Nov 2018 14:39:07 -0000 On 11/23/18 2:30 PM, Maxime Coquelin wrote: > > > On 11/14/18 12:17 PM, Hari Kumar Vemula wrote: >> In rte_efd_create() allocated memory for tail queue entry but >> not freed. >> Added freeing the tail queue entry. >> >> Fixes: 56b6ef874f80 ("efd: new Elastic Flow Distributor library") >> Cc: stable@dpdk.org >> >> Signed-off-by: Hari Kumar Vemula >> Acked-by: Reshma Pattan >> >> --- >> v2: Updated commit message. >> --- >> >>   lib/librte_efd/rte_efd.c | 21 +++++++++++++++++++++ >>   1 file changed, 21 insertions(+) >> >> diff --git a/lib/librte_efd/rte_efd.c b/lib/librte_efd/rte_efd.c >> index a780e2fe8..f8c6c447f 100644 >> --- a/lib/librte_efd/rte_efd.c >> +++ b/lib/librte_efd/rte_efd.c >> @@ -739,17 +739,38 @@ void >>   rte_efd_free(struct rte_efd_table *table) >>   { >>       uint8_t socket_id; >> +    struct rte_efd_list *efd_list = NULL; > > NULL init seems useless here. > >> +    struct rte_tailq_entry *te; >>       if (table == NULL) >>           return; >> +    efd_list = RTE_TAILQ_CAST(rte_efd_tailq.head, rte_efd_list); >> + >>       for (socket_id = 0; socket_id < RTE_MAX_NUMA_NODES; socket_id++) >>           rte_free(table->chunks[socket_id]); >> +    rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK); >> + >> +    /* find our tailq entry */ >> +    TAILQ_FOREACH(te, efd_list, next) { >> +        if (te->data == (void *) table) >> +            break; >> +    } >> + >> +    if (te == NULL) { >> +        rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); >> +        return; And in this case, I think there is a leak, as all table stuffs don't get freed. >> +    } >> + >> +    TAILQ_REMOVE(efd_list, te, next); >> +    rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); > > Wouldn't this be simpler by using TAILQ_FOREACH_SAFE() instead? > The element could removed from the list and freed within the loop. > >> + >>       rte_ring_free(table->free_slots); >>       rte_free(table->offline_chunks); >>       rte_free(table->keys); >>       rte_free(table); >> +    rte_free(te); >>   } >>   /** >>