From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qg0-f53.google.com (mail-qg0-f53.google.com [209.85.192.53]) by dpdk.org (Postfix) with ESMTP id 41E772BF2 for ; Fri, 11 Mar 2016 16:02:39 +0100 (CET) Received: by mail-qg0-f53.google.com with SMTP id y89so99927960qge.2 for ; Fri, 11 Mar 2016 07:02:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=c/8pDQF3FqBnq4IOfTEOI7cxuSl5DQgcyofpVwb71/Q=; b=Cf4P+KScdzder/pvWwX5lCpYrtPLXT/WaB2Pk5vJTHlyZi1CJ/LBlBoLk5UIXA82wU OgsmU14N1LLphDJGXZukaRG8EtjNx/DorVAiq1Wo4LnkdTlU1QbbBNdb3ep+pdsEtzoe +C6HmtdwlL8gM+7UtXmE/tNUnvdftTlcpdFjAUkf0nsnPthKlwNldyB1ZHSApXhLliqI o0EBlI1e9uZqNflaHZUHh5d7/rps/oZeU59JN3Y9O7SBy5KL7Qb/VXvXXMfUj3emBkvf fZO8h5o+9oez9br+Qs3hFUPTPrBsvvOgq87W/XFDq7p4p+r/99HImCeXVOHqLo3W/boc Xq/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=c/8pDQF3FqBnq4IOfTEOI7cxuSl5DQgcyofpVwb71/Q=; b=AJzy8X+zJEkDTfy+1p0DxClN+Jzetiyt1zIpX+hHC7hyfnd4KXLvX7UHKuZ6fH6rXL j4uINJEdfErX93mhQJdkhoKSAJtzDei0ocW20FblmTvR4avJsXA/bfz2f/hKQBlRZCVd Kl6VpxqZjfwnA1uhOeEU6iK2xP8xahut+gFWA/Mdifo6nd6OkpkI0846/eXXTblAe60Z q6H9UWC9frxb+Tp0cjD2bC/8nl3x5PBTbUoBwJdPK9aXtf+GekIu7CCEVkp/SZqIPjyu GJm0TMObxNAWipeRT7KfujEVIkn/hxvmYtooBC2+12yKxFQt73wjbZ3nHvJTkKb2jC1k ve4A== X-Gm-Message-State: AD7BkJL0446hqVGqAVkpHnrkCCCKV/HgoqQSK8UJA/1KzbIj02FDCorSeMSpvGg+vUOQWbj+f87enKFqCuNhkqbD X-Received: by 10.140.109.100 with SMTP id k91mr12566799qgf.54.1457708558726; Fri, 11 Mar 2016 07:02:38 -0800 (PST) MIME-Version: 1.0 Received: by 10.55.207.20 with HTTP; Fri, 11 Mar 2016 07:02:19 -0800 (PST) In-Reply-To: <20160311140011.GA11396@bricha3-MOBL3> References: <1457098145-13663-1-git-send-email-christian.ehrhardt@canonical.com> <20160311140011.GA11396@bricha3-MOBL3> From: Christian Ehrhardt Date: Fri, 11 Mar 2016 16:02:19 +0100 Message-ID: To: Bruce Richardson , Christian Ehrhardt Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: dev Subject: Re: [dpdk-dev] [PATCH] lpm/lpm6: fix missing free of rules_tbl and lpm X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Mar 2016 15:02:39 -0000 Hi Bruce, thanks I'll split into two patches and ensure there is no sign-off missing. While doing so I'll also rebase to latest master to get rid of the offset a patch would report. I'll send all three lpm related patches as one series then adding your acks to the respective patches. Thanks, Christian Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd On Fri, Mar 11, 2016 at 3:00 PM, Bruce Richardson < bruce.richardson@intel.com> wrote: > On Tue, Mar 08, 2016 at 05:38:28PM +0100, Christian Ehrhardt wrote: > > Hi, > > Stephen acked the other LPM patch I had last week (thanks). > > There was no feedback to this one so far and none of the two patches is > > committed yet. > > > > So I wanted to give this another "ping" for feedback or acceptance. > > > > Thanks in advance, > > Christian > > > > Christian Ehrhardt > > Software Engineer, Ubuntu Server > > Canonical Ltd > > > Hi Christian, > > sorry for the delayed comments. This fix looks ok to me, but the patch > appears > to be missing your signoff. > > Two minor comments > * This is probably better as two patches, one for lpm6, other for > lpm > * Coding standards state that we don't use "{}" for blocks with > only > a single line. > > Otherwise: > > Acked-by: Bruce Richardson > > Regards, > /Bruce > > > On Fri, Mar 4, 2016 at 2:29 PM, Christian Ehrhardt < > > christian.ehrhardt@canonical.com> wrote: > > > > > lpm6 autotests failed with the default alloc of 512M Memory. > > > While >=2500M was a workaround it became clear while debugging that it > > > had a leak. > > > One could see a lot of output like: > > > LPM Test tests6[i]: FAIL > > > LPM: LPM memory allocation failed > > > > > > It turned out that in rte_lpm6_free > > > - lpm might not be freed if it didn't find a te (early return) > > > - lpm->rules_tbl was not freed ever > > > > > > The first of the two also applies to lpm. > > > --- > > > lib/librte_lpm/rte_lpm.c | 7 ++----- > > > lib/librte_lpm/rte_lpm6.c | 9 ++++----- > > > 2 files changed, 6 insertions(+), 10 deletions(-) > > > > > > diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c > > > index cfdf959..941720d 100644 > > > --- a/lib/librte_lpm/rte_lpm.c > > > +++ b/lib/librte_lpm/rte_lpm.c > > > @@ -236,13 +236,10 @@ rte_lpm_free(struct rte_lpm *lpm) > > > if (te->data == (void *) lpm) > > > break; > > > } > > > - if (te == NULL) { > > > - rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); > > > - return; > > > + if (te != NULL) { > > > + TAILQ_REMOVE(lpm_list, te, next); > > > } > > > > > > - TAILQ_REMOVE(lpm_list, te, next); > > > - > > > rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); > > > > > > rte_free(lpm); > > > diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c > > > index 48931cc..5abfc78 100644 > > > --- a/lib/librte_lpm/rte_lpm6.c > > > +++ b/lib/librte_lpm/rte_lpm6.c > > > @@ -278,15 +278,14 @@ rte_lpm6_free(struct rte_lpm6 *lpm) > > > if (te->data == (void *) lpm) > > > break; > > > } > > > - if (te == NULL) { > > > - rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); > > > - return; > > > - } > > > > > > - TAILQ_REMOVE(lpm_list, te, next); > > > + if (te != NULL) { > > > + TAILQ_REMOVE(lpm_list, te, next); > > > + } > > > > > > rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); > > > > > > + rte_free(lpm->rules_tbl); > > > rte_free(lpm); > > > rte_free(te); > > > } > > > -- > > > 2.7.0 > > > > > > >