From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 66EB6A00BE; Mon, 27 Apr 2020 20:03:03 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0E7AE1D502; Mon, 27 Apr 2020 20:02:58 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 0175D1D424 for ; Mon, 27 Apr 2020 20:02:55 +0200 (CEST) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20200427180255euoutp027502fb972f13a8f1df34e000d0d58359~JvzRkydp72207322073euoutp02U for ; Mon, 27 Apr 2020 18:02:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20200427180255euoutp027502fb972f13a8f1df34e000d0d58359~JvzRkydp72207322073euoutp02U DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1588010575; bh=2NY2tADA9ts/wfshHWc4/cCEYJPXfhIpix44mpCftr4=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=dsFDqeLEYDesoUjwOHxt0KBr19NpNyloymnRVcIMIDhWAp/LErrlb511nIvTCf3tx c1MS+3m5/7N3FHlMC47SXJFwsuqdX4VcOUksPntLhVPtRupTsim0SQHE+lpYInCgEO N/1/5JVl5EZJPhaqBTfAlKYG+B3ifkfEM+mqC74U= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20200427180255eucas1p19b699b0530b32b28e582cae9a59918dc~JvzRelE1U1115011150eucas1p15; Mon, 27 Apr 2020 18:02:55 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id 71.0F.60679.F4E17AE5; Mon, 27 Apr 2020 19:02:55 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20200427180254eucas1p27d129c7850a3df09554fb790e3b0565f~JvzRME0530927209272eucas1p2V; Mon, 27 Apr 2020 18:02:54 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20200427180254eusmtrp2de832c1a85f3b508d8d0e7e6a65024c2~JvzRLW5Mw0169401694eusmtrp2j; Mon, 27 Apr 2020 18:02:54 +0000 (GMT) X-AuditID: cbfec7f4-0cbff7000001ed07-7c-5ea71e4f44ac Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 32.A6.07950.E4E17AE5; Mon, 27 Apr 2020 19:02:54 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20200427180253eusmtip2743c6ed9d80f8477fc71e1771394d1a2~JvzQDySDu1868718687eusmtip2J; Mon, 27 Apr 2020 18:02:53 +0000 (GMT) To: Ray Kinsella , Thomas Monjalon , dev@dpdk.org Cc: david.marchand@redhat.com, Olivier Matz , Neil Horman , John McNamara , Marko Kovacevic , Xiaoyun Li , Jingjing Wu , Andrew Rybchenko From: Lukasz Wojciechowski Message-ID: Date: Mon, 27 Apr 2020 20:02:51 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <6bd397c6-3012-ae10-af9a-eabee96d5c92@ashroe.eu> Content-Transfer-Encoding: 8bit Content-Language: pl X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0hTYRjG+XYuO9Mmx6n5olk4MChIuwidsKKg7ISE/hFEkZfZDirqlE1N C8zUJMZsKpY3KilropUpbqZkitdsXrIyp6VOMcpEAy+VF7RtR8n/ft/7PO/lgY/CJD2EGxWl SOCUClmMlLTDDR1LvfsCd+pC9g/m2DPm/BGSqStXk8zsXJ2AKSqdR0zfAz3B9FVnE4xW344z wxldJKNPrxYwc+a3OJPbWYVO2LP92q+I1fVqCXa59AnBPn49JWA7hu4J2V9vBki2cvIvyX76 U4mCqEt2R+VcTFQSp/Q5HmYXWdZZguKfyZOXe+bwNGQMUiMRBbQvaL6rMTWyoyR0OYIxQzPO PxYQZA7MbyjzCMYrhrHNltp0k5AXdAiql6oE/GMGQaHRYHM50RfgtylfaGVnC5d9HLaZMLpR AC+7R0mrQNLHoK1okbCymPaHtcxpS52icNoLRpZwK7rQwVDw+TzvcISuokncyiJL59hirm08 Ru+CDH0JxrMzmCYykXUV0FNCaL7fZ5sD9CmYHdkI4AQ/O2uFPO+A9fqHAt5vQDCwsrTR3IRg MLt8w+UHrWsrttsweg9UNfjwM0/CfLmARwcwzTjyJzhAnqEA48tiuJ0l4Wd4wzfNXbS5dfX5 JJ6DpMVbghVvCVO8JUzx/7WlCK9ArlyiKjaCUx1UcFe9VbJYVaIiwvtKXGwNsnwy41rnwivU sBregmgKSbeJzU1PQySELEmVEtuCgMKkzuKJSEtJLJelXOOUcaHKxBhO1YLcKVzqKj70aCpY QkfIErhojovnlJuqgBK5pSFgLxMQtN3FPfBWva9Lo+ZDn4fR60zIerhDtrevT15zxe7DHljp xbyhc413PJMf3Qzzo/pFCY4JP3rlAy7NGg97B23N6Ug40kqIGLZ1TGIaKnvR1F41WuA/tzx+ I1ULZ4fUoY5t0wE9Zh1ZGJ2S9cWMy/WpGs9u13dB768HSHFVpOzAXkypkv0DM4G8d2ADAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrAIsWRmVeSWpSXmKPExsVy+t/xe7p+csvjDF7tMLZ4MOUum8X2FV1s Fu8+bWeymLngM6PF+XlbWS3Ob+xltejfepTF4lbzSTaLrU0bmSw+PTjBYjHx+HpGB26Pi/13 GD2Wn+tn9fi1YCmrx+I9L5k8jt2cxu7xft9VNo/VT36weVz5vpoxgCNKz6Yov7QkVSEjv7jE Vina0MJIz9DSQs/IxFLP0Ng81srIVEnfziYlNSezLLVI3y5BL2PJ8dmMBWtSKn6d/cTSwHg6 oIuRk0NCwERiS9MN9i5GLg4hgaWMEj82n2PuYuQASshIfLgkAFEjLPHnWhcbRM1rRolvHZdZ QRLCAuES325MYQexRYDsZ7tuMYEUMQvsZZL40nqVGaLjM6PEuruPWECq2ARsJY7M/ArWzSvg JvGv5TUbyDYWAVWJuz9ZQExRgViJlouaEBWCEidnPgHr5ATqvP91ItguZgEziXmbHzJD2PIS zVtnQ9kiEjcetTBOYBSahaR9FpKWWUhaZiFpWcDIsopRJLW0ODc9t9hIrzgxt7g0L10vOT93 EyMwdrcd+7llB2PXu+BDjAIcjEo8vA/2L4sTYk0sK67MPcQowcGsJML7KAMoxJuSWFmVWpQf X1Sak1p8iNEU6LWJzFKiyfnAtJJXEm9oamhuYWlobmxubGahJM7bIXAwRkggPbEkNTs1tSC1 CKaPiYNTqoFxpW912Auv+eJWm5RC7/cVLg44NeO8UX9kkP05tW+xAZ/299nv+F3w8NXc0psz JiaUMUs1SVscVc3z+7pY325qa5oM0yG7B4qu/ZbrUvgPPFR+9T0vT5j7++tAyx8Rgd3sVxal bj613zVCyEBz/a+nvndTD139PXGXi9mx1LQPk1cv/rckuqtQiaU4I9FQi7moOBEARt5PrPMC AAA= X-CMS-MailID: 20200427180254eucas1p27d129c7850a3df09554fb790e3b0565f X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20200427114503eucas1p216417dd6a0d7c0560671e7b16eabe26d X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20200427114503eucas1p216417dd6a0d7c0560671e7b16eabe26d References: <20200117145754.11682-4-olivier.matz@6wind.com> <20200425222338.2787083-1-thomas@monjalon.net> <6bd397c6-3012-ae10-af9a-eabee96d5c92@ashroe.eu> Subject: Re: [dpdk-dev] [PATCH v3] mempool: return 0 if area is too small on populate 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" W dniu 27.04.2020 o 13:44, Ray Kinsella pisze: > > On 25/04/2020 23:23, Thomas Monjalon wrote: >> From: Olivier Matz >> >> Change rte_mempool_populate_iova() and rte_mempool_populate_iova() to ... and rte_mempool_populate_virt() >> return 0 instead of -EINVAL when there is not enough room to store one >> object, as it can be helpful for applications to distinguish this >> specific case. >> >> As this is an ABI change, use symbol versioning to preserve old >> behavior for binary applications. >> >> Signed-off-by: Olivier Matz >> --- >> >> changes in v3: >> - rebase >> - remove deprecation notice >> - notify API change in release notes >> - fix ABI version from 20.0.1 to 20.0.2 (should be 21 maybe) > yes, should be v21. > >> This v3 cannot be merged because of a false positive ABI check: >> >> 2 Removed functions: >> 'function int rte_mempool_populate_iova(rte_mempool*, char*, rte_iova_t, size_t, rte_mempool_memchunk_free_cb_t*, void*)' {rte_mempool_populate_iova@@DPDK_20.0} >> 'function int rte_mempool_populate_virt(rte_mempool*, char*, size_t, size_t, rte_mempool_memchunk_free_cb_t*, void*)' {rte_mempool_populate_virt@@DPDK_20.0} > Well it's not really a false positive, as the v20 symbols are now missing. > See notes on VERSION_SYMBOL. > >> --- >> doc/guides/rel_notes/deprecation.rst | 5 -- >> doc/guides/rel_notes/release_20_05.rst | 4 ++ >> examples/ntb/ntb_fwd.c | 2 +- >> lib/librte_mempool/meson.build | 2 + >> lib/librte_mempool/rte_mempool.c | 77 ++++++++++++++++++---- >> lib/librte_mempool/rte_mempool.h | 14 ++-- >> lib/librte_mempool/rte_mempool_version.map | 7 ++ >> 7 files changed, 90 insertions(+), 21 deletions(-) >> >> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst >> index 1339f54f5f..20aa745b77 100644 >> --- a/doc/guides/rel_notes/deprecation.rst >> +++ b/doc/guides/rel_notes/deprecation.rst >> @@ -65,11 +65,6 @@ Deprecation Notices >> structure would be made internal (or removed if all dependencies are cleared) >> in future releases. >> >> -* mempool: starting from v20.05, the API of rte_mempool_populate_iova() >> - and rte_mempool_populate_virt() will change to return 0 instead >> - of -EINVAL when there is not enough room to store one object. The ABI >> - will be preserved until 20.11. >> - >> * ethdev: the legacy filter API, including >> ``rte_eth_dev_filter_supported()``, ``rte_eth_dev_filter_ctrl()`` as well >> as filter types MACVLAN, ETHERTYPE, FLEXIBLE, SYN, NTUPLE, TUNNEL, FDIR, >> diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst >> index b124c3f287..ab20a7d021 100644 >> --- a/doc/guides/rel_notes/release_20_05.rst >> +++ b/doc/guides/rel_notes/release_20_05.rst >> @@ -241,6 +241,10 @@ API Changes >> Also, make sure to start the actual text at the margin. >> ========================================================= >> >> +* mempool: The API of ``rte_mempool_populate_iova()`` and >> + ``rte_mempool_populate_virt()`` changed to return 0 instead of -EINVAL >> + when there is not enough room to store one object. >> + >> >> ABI Changes >> ----------- >> diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c >> index d49189e175..eba8ebf9fa 100644 >> --- a/examples/ntb/ntb_fwd.c >> +++ b/examples/ntb/ntb_fwd.c >> @@ -1319,7 +1319,7 @@ ntb_mbuf_pool_create(uint16_t mbuf_seg_size, uint32_t nb_mbuf, >> mz->len - ntb_info.ntb_hdr_size, >> ntb_mempool_mz_free, >> (void *)(uintptr_t)mz); >> - if (ret < 0) { >> + if (ret <= 0) { >> rte_memzone_free(mz); >> rte_mempool_free(mp); >> return NULL; >> diff --git a/lib/librte_mempool/meson.build b/lib/librte_mempool/meson.build >> index a6e861cbfc..7dbe6b9bea 100644 >> --- a/lib/librte_mempool/meson.build >> +++ b/lib/librte_mempool/meson.build >> @@ -9,6 +9,8 @@ foreach flag: extra_flags >> endif >> endforeach >> >> +use_function_versioning = true >> + >> sources = files('rte_mempool.c', 'rte_mempool_ops.c', >> 'rte_mempool_ops_default.c', 'mempool_trace_points.c') >> headers = files('rte_mempool.h', 'rte_mempool_trace.h', >> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c >> index 0be8f9f59d..edbdafaafb 100644 >> --- a/lib/librte_mempool/rte_mempool.c >> +++ b/lib/librte_mempool/rte_mempool.c >> @@ -31,6 +31,7 @@ >> #include >> #include >> #include >> +#include >> >> #include "rte_mempool.h" >> #include "rte_mempool_trace.h" >> @@ -303,12 +304,17 @@ mempool_ops_alloc_once(struct rte_mempool *mp) >> return 0; >> } >> >> +__vsym int >> +rte_mempool_populate_iova_v20_0_2(struct rte_mempool *mp, char *vaddr, >> + rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb, >> + void *opaque); >> + >> /* Add objects in the pool, using a physically contiguous memory >> * zone. Return the number of objects added, or a negative value >> * on error. >> */ >> -static int >> -__rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, >> +__vsym int >> +rte_mempool_populate_iova_v20_0_2(struct rte_mempool *mp, char *vaddr, >> rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb, >> void *opaque) >> { >> @@ -366,14 +372,27 @@ __rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, >> return ret; >> } >> >> -int >> -rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, >> +BIND_DEFAULT_SYMBOL(rte_mempool_populate_iova, _v20_0_2, 20.0.2); > as discussed v21. > >> +MAP_STATIC_SYMBOL( >> + int rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, >> + rte_iova_t iova, size_t len, >> + rte_mempool_memchunk_free_cb_t *free_cb, >> + void *opaque), >> + rte_mempool_populate_iova_v20_0_2); >> + >> +__vsym int >> +rte_mempool_populate_iova_v20(struct rte_mempool *mp, char *vaddr, >> + rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb, >> + void *opaque); >> + >> +__vsym int >> +rte_mempool_populate_iova_v20(struct rte_mempool *mp, char *vaddr, >> rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb, >> void *opaque) >> { >> int ret; >> >> - ret = __rte_mempool_populate_iova(mp, vaddr, iova, len, free_cb, >> + ret = rte_mempool_populate_iova_v20_0_2(mp, vaddr, iova, len, free_cb, >> opaque); >> if (ret == 0) >> ret = -EINVAL; >> @@ -381,6 +400,7 @@ rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, >> rte_mempool_trace_populate_iova(mp, vaddr, iova, len, free_cb, opaque); >> return ret; >> } >> +VERSION_SYMBOL(rte_mempool_populate_iova, _v20_0, 20.0); > this should be > > VERSION_SYMBOL(rte_mempool_populate_iova, _v20, 20.0); > >> >> static rte_iova_t >> get_iova(void *addr) >> @@ -395,11 +415,16 @@ get_iova(void *addr) >> return ms->iova + RTE_PTR_DIFF(addr, ms->addr); >> } >> >> +__vsym int >> +rte_mempool_populate_virt_v20_0_2(struct rte_mempool *mp, char *addr, >> + size_t len, size_t pg_sz, rte_mempool_memchunk_free_cb_t *free_cb, >> + void *opaque); >> + >> /* Populate the mempool with a virtual area. Return the number of >> * objects added, or a negative value on error. >> */ >> -int >> -rte_mempool_populate_virt(struct rte_mempool *mp, char *addr, >> +__vsym int >> +rte_mempool_populate_virt_v20_0_2(struct rte_mempool *mp, char *addr, >> size_t len, size_t pg_sz, rte_mempool_memchunk_free_cb_t *free_cb, >> void *opaque) >> { >> @@ -432,7 +457,7 @@ rte_mempool_populate_virt(struct rte_mempool *mp, char *addr, >> break; >> } >> >> - ret = __rte_mempool_populate_iova(mp, addr + off, iova, >> + ret = rte_mempool_populate_iova_v20_0_2(mp, addr + off, iova, >> phys_len, free_cb, opaque); >> if (ret == 0) >> continue; >> @@ -443,9 +468,6 @@ rte_mempool_populate_virt(struct rte_mempool *mp, char *addr, >> cnt += ret; >> } >> >> - if (cnt == 0) >> - return -EINVAL; >> - >> rte_mempool_trace_populate_virt(mp, addr, len, pg_sz, free_cb, opaque); >> return cnt; >> >> @@ -453,6 +475,35 @@ rte_mempool_populate_virt(struct rte_mempool *mp, char *addr, >> rte_mempool_free_memchunks(mp); >> return ret; >> } >> +BIND_DEFAULT_SYMBOL(rte_mempool_populate_virt, _v20_0_2, 20.0.2); > as discussed v21. > >> +MAP_STATIC_SYMBOL( >> + int rte_mempool_populate_virt(struct rte_mempool *mp, >> + char *addr, size_t len, size_t pg_sz, >> + rte_mempool_memchunk_free_cb_t *free_cb, >> + void *opaque), >> + rte_mempool_populate_virt_v20_0_2); >> + >> +__vsym int >> +rte_mempool_populate_virt_v20(struct rte_mempool *mp, char *addr, >> + size_t len, size_t pg_sz, rte_mempool_memchunk_free_cb_t *free_cb, >> + void *opaque); >> + >> +__vsym int >> +rte_mempool_populate_virt_v20(struct rte_mempool *mp, char *addr, >> + size_t len, size_t pg_sz, rte_mempool_memchunk_free_cb_t *free_cb, >> + void *opaque) >> +{ >> + int ret; >> + >> + ret = rte_mempool_populate_virt_v20_0_2(mp, addr, len, pg_sz, >> + free_cb, opaque); >> + >> + if (ret == 0) >> + ret = -EINVAL; >> + >> + return ret; >> +} >> +VERSION_SYMBOL(rte_mempool_populate_virt, _v20_0, 20.0); > this should be > VERSION_SYMBOL(rte_mempool_populate_virt, _v20, 20.0); > > >> >> /* Get the minimal page size used in a mempool before populating it. */ >> int >> @@ -609,6 +660,8 @@ rte_mempool_populate_default(struct rte_mempool *mp) >> mz->len, pg_sz, >> rte_mempool_memchunk_mz_free, >> (void *)(uintptr_t)mz); >> + if (ret == 0) /* should not happen */ >> + ret = -ENOBUFS; >> if (ret < 0) { >> rte_memzone_free(mz); >> goto fail; >> @@ -701,6 +754,8 @@ rte_mempool_populate_anon(struct rte_mempool *mp) >> >> ret = rte_mempool_populate_virt(mp, addr, size, getpagesize(), >> rte_mempool_memchunk_anon_free, addr); >> + if (ret == 0) /* should not happen */ >> + ret = -ENOBUFS; >> if (ret < 0) { >> rte_errno = -ret; >> goto fail; >> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h >> index 6e0573ea42..652d19f9f1 100644 >> --- a/lib/librte_mempool/rte_mempool.h >> +++ b/lib/librte_mempool/rte_mempool.h >> @@ -1112,9 +1112,12 @@ rte_mempool_free(struct rte_mempool *mp); >> * @param opaque >> * An opaque argument passed to free_cb. >> * @return >> - * The number of objects added on success. >> + * The number of objects added on success (strictly positive). >> * On error, the chunk is not added in the memory list of the >> - * mempool and a negative errno is returned. >> + * mempool the following code is returned: >> + * (0): not enough room in chunk for one object. >> + * (-ENOSPC): mempool is already populated. >> + * (-ENOMEM): allocation failure. >> */ >> int rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, >> rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb, >> @@ -1139,9 +1142,12 @@ int rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr, >> * @param opaque >> * An opaque argument passed to free_cb. >> * @return >> - * The number of objects added on success. >> + * The number of objects added on success (strictly positive). >> * On error, the chunk is not added in the memory list of the >> - * mempool and a negative errno is returned. >> + * mempool the following code is returned: >> + * (0): not enough room in chunk for one object. >> + * (-ENOSPC): mempool is already populated. >> + * (-ENOMEM): allocation failure. >> */ >> int >> rte_mempool_populate_virt(struct rte_mempool *mp, char *addr, >> diff --git a/lib/librte_mempool/rte_mempool_version.map b/lib/librte_mempool/rte_mempool_version.map >> index 695dd6e04f..9e58093665 100644 >> --- a/lib/librte_mempool/rte_mempool_version.map >> +++ b/lib/librte_mempool/rte_mempool_version.map >> @@ -31,6 +31,13 @@ DPDK_20.0 { >> local: *; >> }; >> >> +DPDK_20.0.2 { > as discussed DPDK_21 > >> + global: >> + >> + rte_mempool_populate_iova; >> + rte_mempool_populate_virt; >> +} DPDK_20.0; >> + >> EXPERIMENTAL { >> global: >> >> -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com