From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 7D006A823 for ; Wed, 17 Jan 2018 17:37:49 +0100 (CET) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 6AB19480099; Wed, 17 Jan 2018 16:37:47 +0000 (UTC) Received: from [192.168.239.128] (188.242.181.57) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Wed, 17 Jan 2018 16:37:41 +0000 To: santosh , Olivier MATZ CC: , "Artem V. Andreev" References: <1511539591-20966-1-git-send-email-arybchenko@solarflare.com> <1511539591-20966-3-git-send-email-arybchenko@solarflare.com> <20171214133721.b7z2i7yccwpath37@platinum> <69c425a0-7339-3e7a-a0d2-e2987a644cf2@caviumnetworks.com> From: Andrew Rybchenko Message-ID: <00c21922-bd7a-07cc-55c9-2dc881942ab5@solarflare.com> Date: Wed, 17 Jan 2018 19:37:34 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <69c425a0-7339-3e7a-a0d2-e2987a644cf2@caviumnetworks.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [188.242.181.57] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23600.003 X-TM-AS-Result: No--15.049500-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1516207068-km2N84BNiTeu Subject: Re: [dpdk-dev] [RFC PATCH 2/6] mempool: implement clustered object allocation 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: Wed, 17 Jan 2018 16:37:49 -0000 On 01/17/2018 06:55 PM, santosh wrote: > On Wednesday 17 January 2018 08:33 PM, Andrew Rybchenko wrote: >> On 12/14/2017 04:37 PM, Olivier MATZ wrote: >>> On Fri, Nov 24, 2017 at 04:06:27PM +0000, Andrew Rybchenko wrote: >>>> From: "Artem V. Andreev" >>>> >>>> Clustered allocation is required to simplify packaging objects into >>>> buckets and search of the bucket control structure by an object. >>>> >>>> Signed-off-by: Artem V. Andreev >>>> Signed-off-by: Andrew Rybchenko >>>> --- >>>>   lib/librte_mempool/rte_mempool.c | 39 +++++++++++++++++++++++++++++++++++---- >>>>   lib/librte_mempool/rte_mempool.h | 23 +++++++++++++++++++++-- >>>>   test/test/test_mempool.c         |  2 +- >>>>   3 files changed, 57 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c >>>> index d50dba4..43455a3 100644 >>>> --- a/lib/librte_mempool/rte_mempool.c >>>> +++ b/lib/librte_mempool/rte_mempool.c >>>> @@ -239,7 +239,8 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags, >>>>    */ >>>>   size_t >>>>   rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift, >>>> -              unsigned int flags) >>>> +              unsigned int flags, >>>> +              const struct rte_mempool_info *info) >>>>   { >>>>       size_t obj_per_page, pg_num, pg_sz; >>>>       unsigned int mask; >>>> @@ -252,6 +253,17 @@ rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift, >>>>       if (total_elt_sz == 0) >>>>           return 0; >>>>   +    if (flags & MEMPOOL_F_CAPA_ALLOCATE_IN_CLUSTERS) { >>>> +        unsigned int align_shift = >>>> +            rte_bsf32( >>>> +                rte_align32pow2(total_elt_sz * >>>> +                        info->cluster_size)); >>>> +        if (pg_shift < align_shift) { >>>> +            return ((elt_num / info->cluster_size) + 2) >>>> +                << align_shift; >>>> +        } >>>> +    } >>>> + >>> +Cc Santosh for this >>> >>> To be honnest, that was my fear when introducing >>> MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS and MEMPOOL_F_CAPA_PHYS_CONTIG to see more >>> and more specific flags in generic code. >>> >>> I feel that the hidden meaning of these flags is more "if driver == foo", >>> which shows that something is wrong is the current design. >>> >>> We have to think about another way to do. Let me try to propose >>> something (to be deepen). >>> >>> The standard way to create a mempool is: >>> >>>    mp = create_empty(...) >>>    set_ops_by_name(mp, "my-driver")    // optional >>>    populate_default(mp)                // or populate_*() >>>    obj_iter(mp, callback, arg)         // optional, to init objects >>>    // and optional local func to init mempool priv >>> >>> First, we can consider deprecating some APIs like: >>>   - rte_mempool_xmem_create() >>>   - rte_mempool_xmem_size() >>>   - rte_mempool_xmem_usage() >>>   - rte_mempool_populate_iova_tab() >>> >>> These functions were introduced for xen, which was recently >>> removed. They are complex to use, and are not used anywhere else in >>> DPDK. >>> >>> Then, instead of having flags (quite hard to understand without knowing >>> the underlying driver), we can let the mempool drivers do the >>> populate_default() operation. For that we can add a populate_default >>> field in mempool ops. Same for populate_virt(), populate_anon(), and >>> populate_phys() which can return -ENOTSUP if this is not >>> implemented/implementable on a specific driver, or if flags >>> (NO_CACHE_ALIGN, NO_SPREAD, ...) are not supported. If the function >>> pointer is NULL, use the generic function. >>> >>> Thanks to this, the generic code would remain understandable and won't >>> have to care about how memory should be allocated for a specific driver. >>> >>> Thoughts? >> Yes, I agree. This week we'll provide updated version of the RFC which >> covers it including transition of the mempool/octeontx. I think it is sufficient >> to introduce two new ops: >>  1. To calculate memory space required to store specified number of objects >>  2. To populate objects in the provided memory chunk (the op will be called >>      from rte_mempool_populate_iova() which is a leaf function for all >>      rte_mempool_populate_*() calls. >> It will allow to avoid duplication and keep memchunks housekeeping inside >> mempool library. >> > There is also a downside of letting mempool driver to populate, which was raised in other thread. > http://dpdk.org/dev/patchwork/patch/31943/ I've seen the note about code duplication. Let's discuss it when v2 is sent. I think our approach minimizes it and allows to have only specific code in the driver callback.