From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f172.google.com (mail-wi0-f172.google.com [209.85.212.172]) by dpdk.org (Postfix) with ESMTP id CA00D11A2 for ; Thu, 19 Mar 2015 09:13:01 +0100 (CET) Received: by wixw10 with SMTP id w10so61801595wix.0 for ; Thu, 19 Mar 2015 01:13:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=junrl0vhXD094xXn7VghiEhAY1laxkfl8h3r0SAsnrI=; b=Ev2SSacyS82qvqKbKjllFhQaQZSf1lxcoUXoiSEP+g0QGulcU02bhZnoWTcdRHyMKs eykc8b+gov6hv71NFiYKNTw9e7FNKIWZvwvqmyoeCZnV5MWCIwo1rpVYW2hPMVeslgip Wdyww/hp791McHKaCIb0YMn4A3hTUSGjSqK8R/OqL4sEtkEhc4eBBuNzrReU3GD/jLC3 57okOYNKP8P8cA6Okw2PPsWP0r6X7/iI5yQZUldzsEtZPlZnSOJ/lNFuVDcL2M7kQXSr k55svYtyeSxPEYrlVMxvmabKPI//7SyAOEkiWyxszwtJUb156Kcv3yZuU/v+UyYL+Kif bWpw== X-Gm-Message-State: ALoCoQki8jPGu9DplowYffunmmfDBSMOSA/O3AyZ24G2QcfGuDXdtGoIbM1W7kO1JL/LgwNnHKdz X-Received: by 10.180.104.200 with SMTP id gg8mr14038387wib.8.1426752781715; Thu, 19 Mar 2015 01:13:01 -0700 (PDT) Received: from [10.16.0.195] (6wind.net2.nerim.net. [213.41.180.237]) by mx.google.com with ESMTPSA id v13sm6428343wij.10.2015.03.19.01.13.00 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Mar 2015 01:13:01 -0700 (PDT) Message-ID: <550A850D.9010309@6wind.com> Date: Thu, 19 Mar 2015 09:13:01 +0100 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.3.0 MIME-Version: 1.0 To: "Ananyev, Konstantin" , "vadim.suraev@gmail.com" References: <1426628169-1735-1-git-send-email-vadim.suraev@gmail.com> <2601191342CEEE43887BDE71AB977258213F6F10@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258213F7053@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258213F706D@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258213F7136@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258213F7188@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB977258213F7188@irsmsx105.ger.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2] rte_mbuf: mbuf bulk alloc/free functions added + unittest 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: Thu, 19 Mar 2015 08:13:01 -0000 Hi Konstantin, On 03/18/2015 04:13 PM, Ananyev, Konstantin wrote: > >> From: Vadim Suraev [mailto:vadim.suraev@gmail.com] >> Sent: Wednesday, March 18, 2015 10:41 AM >> To: Ananyev, Konstantin >> Cc: dev@dpdk.org >> Subject: Re: [PATCH v2] rte_mbuf: mbuf bulk alloc/free functions added + unittest >> >> Hi, Konstantin, >> >> Got it. To make the same, nulling the next should be inside of the block as you said. >> One question raises here: If a segment in the chain has refcnt > 1 (so its next is not assigned NULL), and the next segment has refcnt >> == 1 (so it is freed), do you think this scenario is real/should be considered? If so, the former can be safely freed only by calling >> rte_pktmbuf_free_seg which does not iterate. So why to keep next pointing to something? > > I think we need it, not just to keep things the same with rte_pktmbuf_free(), but because it is a right thing to do. > Let say you have a packet in 2 mbufs chained together, both mbufs have refcnt==2. > Then: > rte_pktmbuf_free(firs_mbuf); > rte_pktmbuf_free(firs_mbuf); > > Would work correctly and free both mbufs back to the mempool. > > While after: > rte_pktmbuf_free_chain(first_mbuf); > rte_pktmbuf_free_chain(first_mbuf); > > We would have first_mbuf freed back into the mempool, while second would get lost(memory leaking). > Basically free() shouldn't modify any filed inside mbuf, except refcnt if rte_mbuf_refcnt_update(m, -1) > 0 > > About your case, when: first_mbuf->refcnt==2 and second_mbuf->refcnt==1. > Right now, rte_pktmbuf_free() can't handle such cases properly, > and, as I know, such situation is not considered as valid one. I'm not sure I understand what you are saying. To me, the case you are describing is similar to the case below, and it should work properly: /* allocate a packet and clone it. After that, m1 has a * refcnt of 2 */ m1 = rte_pktmbuf_alloc(); clone1 = rte_pktmbuf_clone(m1); /* allocate another packet */ m2 = rte_pktmbuf_alloc(); /* chain m2 after m1, updating fields like total length. * After that, m1 has 2 segments, the first one has a refcnt * of 1 and the second has a refcnt of 2 */ mbuf_concat(m1, m2); /* This will decrement the refcnt on the first segment and * free the second segment */ rte_pktmbuf_free(m1); /* free the indirect mbuf, and as the refcnt is 1 on the * direct mbuf (m1), also release it */ rte_pktmbuf_free(clone1); Am I missing something? Thanks, Olivier