From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-we0-f176.google.com (mail-we0-f176.google.com [74.125.82.176]) by dpdk.org (Postfix) with ESMTP id 539715A8E for ; Thu, 19 Mar 2015 09:41:25 +0100 (CET) Received: by webcq43 with SMTP id cq43so51376608web.2 for ; Thu, 19 Mar 2015 01:41:25 -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=PqV5K1QU34zYOQbdPjt68kE31PQujxPA4LUmWXPwS4A=; b=LfEUtVzO5M/QQ6hq/G1P9gkEdA5uoq229Oc7e206YDTGivouuDtvmLQ7pZ6pXqxnlB gD4s52NZsJdzvU6hnfgEeqGcL8nCxXzTwygZ6zGdGsgSQKw/qksP3MGQcALbEh/5plKx 0fQKFxGYxFFxaye6oz9YEAlV3lKdevx+G9LXntk2gfh5m0n0uVUZj+FoWiC26xMGooTQ dozkimEUvKLdKvxHfhh9lQDnRbpA3uE6nGgPOcKgMD+t7rA8rrVIpEZdTbxNhbZXfKgs P65qS0rQPfhrwE4Qe3IribbUIjxPg9SMSpZ1TeU0KBUyXJ9GJL9ENKSKbSb1snBxdL0u saZw== X-Gm-Message-State: ALoCoQlIxFgkMJ20JNXJT4g+MjKEof2q1Honk126ZCXxmdeH5RDQxUD2hyOmUTQbe3h5jHwLmRpc X-Received: by 10.180.76.147 with SMTP id k19mr13784981wiw.92.1426754485172; Thu, 19 Mar 2015 01:41:25 -0700 (PDT) Received: from [10.16.0.195] (6wind.net2.nerim.net. [213.41.180.237]) by mx.google.com with ESMTPSA id a6sm384243wiy.17.2015.03.19.01.41.24 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Mar 2015 01:41:24 -0700 (PDT) Message-ID: <550A8BB5.9040104@6wind.com> Date: Thu, 19 Mar 2015 09:41:25 +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: Neil Horman , "vadim.suraev@gmail.com" References: <1426710078-11230-1-git-send-email-vadim.suraev@gmail.com> <20150318205856.GA5151@neilslaptop.think-freely.org> In-Reply-To: <20150318205856.GA5151@neilslaptop.think-freely.org> Content-Type: text/plain; charset=windows-1252; 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:41:25 -0000 Hi Neil, On 03/18/2015 09:58 PM, Neil Horman wrote: >> +/** >> + * Free a bulk of mbufs into its original mempool. >> + * This function assumes: >> + * - refcnt equals 1 >> + * - mbufs are direct >> + * - all mbufs must belong to the same mempool >> + * >> + * @param mbufs >> + * Array of pointers to mbuf >> + * @param count >> + * Array size >> + */ >> +static inline void rte_pktmbuf_bulk_free(struct rte_mbuf **mbufs, >> + unsigned count) >> +{ >> + unsigned idx; >> + >> + RTE_MBUF_ASSERT(count > 0); >> + >> + for (idx = 0; idx < count; idx++) { >> + RTE_MBUF_ASSERT(mbufs[idx]->pool == mbufs[0]->pool); >> + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 1); >> + rte_mbuf_refcnt_set(mbufs[idx], 0); > This is really a misuse of the API. The entire point of reference counting is > to know when an mbuf has no more references and can be freed. By forcing all > the reference counts to zero here, you allow the refcnt infrastructure to be > circumvented, causing memory leaks. > > I think what you need to do here is enhance the underlying pktmbuf interface > such that an rte_mbuf structure has a destructor method association with it > which is called when its refcnt reaches zero. That way the > rte_pktmbuf_bulk_free function can just decrement the refcnt on each > mbuf_structure, and the pool as a whole can be returned when the destructor > function discovers that all mbufs in that bulk pool are freed. I don't really understand what's the problem here. The API explicitly describes the conditions for calling this functions: the segments are directs, they belong to the same mempool, and their refcnt is 1. This function could be useful in a driver which knows that the mbuf it allocated matches this conditions. I think an application that only uses direct mbufs and one mempool could also use this function. Regards, Olivier