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 A18595F11 for ; Thu, 20 Sep 2018 11:34:12 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us4.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 1865E4C0063; Thu, 20 Sep 2018 09:31:39 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Thu, 20 Sep 2018 10:31:26 +0100 To: Anatoly Burakov , CC: Neil Horman , John McNamara , Marko Kovacevic , Hemant Agrawal , Shreyansh Jain , Matan Azrad , Shahaf Shuler , Yongseok Koh , Maxime Coquelin , Tiwei Bie , Zhihong Wang , Bruce Richardson , Olivier Matz , , , , , , , , , , , , References: From: Andrew Rybchenko Message-ID: <25b7ffe7-2e94-a326-e00e-11b2f3303147@solarflare.com> Date: Thu, 20 Sep 2018 12:30:40 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24106.003 X-TM-AS-Result: No-14.945400-8.000000-10 X-TMASE-MatchedRID: 9zTThWtzImsOwH4pD14DsPHkpkyUphL9Kx5ICGp/WtH4JyR+b5tvoLiN d7CXjOinydDjPKDFg0LTZ/rvxn7nAl+Txlf7MijxHcQQBuf4ZFsGwd8wUY9uM4fAYSb4KlgZ/Py 9qOi5zpegrGzZoZCW0wG2ORx9EyapJa+FAG1BTBNDmVmiQbM5qiyZvBThPVi1HRspwjeLvSVFbd E9dNZSXefOVcxjDhcwAYt5KiTiutkLbigRnpKlKSPzRlrdFGDwTa0V3TyOzFCmybPsjXyRcnNrE 4aMLNpdEADiqDc6rLrlmMoMwjg9Zw== X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--14.945400-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24106.003 X-MDID: 1537435901-hQO3bYVGwsuQ Subject: Re: [dpdk-dev] [PATCH v2 02/20] mem: allow memseg lists to be marked as external 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: Thu, 20 Sep 2018 09:34:13 -0000 On 9/19/18 4:56 PM, Anatoly Burakov wrote: > When we allocate and use DPDK memory, we need to be able to > differentiate between DPDK hugepage segments and segments that > were made part of DPDK but are externally allocated. Add such > a property to memseg lists. > > This breaks the ABI, so bump the EAL library ABI version and > document the change in release notes. > > All current calls for memseg walk functions were adjusted to > ignore external segments where it made sense. > > Mempools is a special case, because we may be asked to allocate > a mempool on a specific socket, and we need to ignore all page > sizes on other heaps or other sockets. Previously, this > assumption of knowing all page sizes was not a problem, but it > will be now, so we have to match socket ID with page size when > calculating minimum page size for a mempool. > > Signed-off-by: Anatoly Burakov A couple of minor questions/suggestions below, but it is OK to go as is even if rejected. Acked-by: Andrew Rybchenko <...> > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c > index 03e6b5f73..d61c77da3 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -99,25 +99,40 @@ static unsigned optimize_object_size(unsigned obj_size) > return new_obj_size * RTE_MEMPOOL_ALIGN; > } > > +struct pagesz_walk_arg { > + int socket_id; > + size_t min; > +}; > + > static int > find_min_pagesz(const struct rte_memseg_list *msl, void *arg) > { > - size_t *min = arg; > + struct pagesz_walk_arg *wa = arg; > + bool valid; > > - if (msl->page_sz < *min) > - *min = msl->page_sz; > + valid = msl->socket_id == wa->socket_id; Is it intended that we accept externally allocated segment if it is on requested socket? If so, it would be good to add comment to explain why. > + valid |= wa->socket_id == SOCKET_ID_ANY && msl->external == 0; > + > + if (!valid) > + return 0; > + > + if (msl->page_sz < wa->min) > + wa->min = msl->page_sz; I'd suggest to keep single return (it is just a bit shorter) if (valid && msl->page_sz < wa->min)          wa->min = msl->page_sz; <...>