DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: dev <dev@dpdk.org>, "Zhang, AlvinX" <alvinx.zhang@intel.com>,
	 "Burakov, Anatoly" <anatoly.burakov@intel.com>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	 Bruce Richardson <bruce.richardson@intel.com>,
	dpdk stable <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 3/3] [20.05] mempool: return 0 if area is too small on populate
Date: Fri, 17 Jan 2020 21:32:36 +0100	[thread overview]
Message-ID: <CAJFAV8w-a-2Zrd7n5V-hckLWYZ70wWNXvWHxrYdqrorEO+wecw@mail.gmail.com> (raw)
In-Reply-To: <20200117145754.11682-4-olivier.matz@6wind.com>

On Fri, Jan 17, 2020 at 3:58 PM Olivier Matz <olivier.matz@6wind.com> wrote:
>
> Change rte_mempool_populate_iova() and rte_mempool_populate_iova() to
> 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 <olivier.matz@6wind.com>
> ---
>  examples/ntb/ntb_fwd.c                     |  2 +-
>  lib/librte_mempool/meson.build             |  1 +
>  lib/librte_mempool/rte_mempool.c           | 76 ++++++++++++++++++----
>  lib/librte_mempool/rte_mempool.h           | 14 ++--
>  lib/librte_mempool/rte_mempool_version.map |  7 ++
>  5 files changed, 84 insertions(+), 16 deletions(-)
>
> diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c
> index c914256dd..99a43688c 100644
> --- a/examples/ntb/ntb_fwd.c
> +++ b/examples/ntb/ntb_fwd.c
> @@ -1313,7 +1313,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 f8710b61b..4fe267cd8 100644
> --- a/lib/librte_mempool/meson.build
> +++ b/lib/librte_mempool/meson.build
> @@ -18,3 +18,4 @@ deps += ['ring']
>
>  # memseg walk is not yet part of stable API
>  allow_experimental_apis = true
> +use_function_versioning = true
> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> index b434d9f17..0ebb21eec 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -31,6 +31,7 @@
>  #include <rte_string_fns.h>
>  #include <rte_spinlock.h>
>  #include <rte_tailq.h>
> +#include <rte_function_versioning.h>
>
>  #include "rte_mempool.h"
>
> @@ -293,12 +294,17 @@ mempool_ops_alloc_once(struct rte_mempool *mp)
>         return 0;
>  }
>
> +__vsym int
> +rte_mempool_populate_iova_v20_0_1(struct rte_mempool *mp, char *vaddr,
> +       rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb,
> +       void *opaque);
> +

For 20.05, this should be 20.0.2.



>  /* 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_1(struct rte_mempool *mp, char *vaddr,
>         rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb,
>         void *opaque)
>  {
> @@ -355,21 +361,34 @@ __rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr,
>         rte_free(memhdr);
>         return ret;
>  }
> +BIND_DEFAULT_SYMBOL(rte_mempool_populate_iova, _v20_0_1, 20.0.1);
> +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_1);
> +
> +__vsym int
> +rte_mempool_populate_iova_v20_0(struct rte_mempool *mp, char *vaddr,
> +       rte_iova_t iova, size_t len, rte_mempool_memchunk_free_cb_t *free_cb,
> +       void *opaque);
>
> -int
> -rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr,
> +__vsym int
> +rte_mempool_populate_iova_v20_0(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_1(mp, vaddr, iova, len, free_cb,
>                                         opaque);
>         if (ret == 0)
>                 ret = -EINVAL;
>
>         return ret;
>  }
> +VERSION_SYMBOL(rte_mempool_populate_iova, _v20_0, 20.0);
>
>  static rte_iova_t
>  get_iova(void *addr)
> @@ -384,11 +403,16 @@ get_iova(void *addr)
>         return ms->iova + RTE_PTR_DIFF(addr, ms->addr);
>  }
>
> +__vsym int
> +rte_mempool_populate_virt_v20_0_1(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_1(struct rte_mempool *mp, char *addr,
>         size_t len, size_t pg_sz, rte_mempool_memchunk_free_cb_t *free_cb,
>         void *opaque)
>  {
> @@ -421,7 +445,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_1(mp, addr + off, iova,
>                         phys_len, free_cb, opaque);
>                 if (ret == 0)
>                         continue;
> @@ -432,15 +456,41 @@ rte_mempool_populate_virt(struct rte_mempool *mp, char *addr,
>                 cnt += ret;
>         }
>
> -       if (cnt == 0)
> -               return -EINVAL;
> -
>         return cnt;
>
>   fail:
>         rte_mempool_free_memchunks(mp);
>         return ret;
>  }
> +BIND_DEFAULT_SYMBOL(rte_mempool_populate_virt, _v20_0_1, 20.0.1);
> +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_1);
> +
> +__vsym int
> +rte_mempool_populate_virt_v20_0(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_0(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_1(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);
>
>  /* Get the minimal page size used in a mempool before populating it. */
>  int
> @@ -601,6 +651,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;
> @@ -692,6 +744,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 0a1dc6059..5a106a432 100644
> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -1106,9 +1106,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,
> @@ -1133,9 +1136,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 d002dfc46..34d977e47 100644
> --- a/lib/librte_mempool/rte_mempool_version.map
> +++ b/lib/librte_mempool/rte_mempool_version.map
> @@ -35,6 +35,13 @@ DPDK_20.0 {
>         local: *;
>  };
>
> +DPDK_20.0.1 {
> +       global:
> +
> +       rte_mempool_populate_iova;
> +       rte_mempool_populate_virt;
> +} DPDK_20.0;
> +
>  EXPERIMENTAL {
>         global:
>
> --
> 2.20.1
>


  reply	other threads:[~2020-01-17 20:32 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-09 13:27 [dpdk-dev] [PATCH] mempool: fix mempool virt populate with small chunks Olivier Matz
2020-01-09 13:40 ` David Marchand
2020-01-09 13:46   ` Olivier Matz
2020-01-09 13:52 ` Burakov, Anatoly
2020-01-09 14:23   ` Olivier Matz
2020-01-09 14:29     ` Burakov, Anatoly
2020-01-09 14:58       ` Bruce Richardson
2020-01-17 14:57 ` [dpdk-dev] [PATCH v2 0/3] " Olivier Matz
2020-01-17 14:57   ` [dpdk-dev] [PATCH v2 1/3] " Olivier Matz
2020-01-17 14:57   ` [dpdk-dev] [PATCH v2 2/3] doc: announce API change for mempool IOVA populate Olivier Matz
2020-01-17 20:32     ` David Marchand
2020-01-17 14:57   ` [dpdk-dev] [PATCH v2 3/3] [20.05] mempool: return 0 if area is too small on populate Olivier Matz
2020-01-17 20:32     ` David Marchand [this message]
2020-04-25 22:23     ` [dpdk-dev] [PATCH v3] " Thomas Monjalon
2020-04-26 16:52       ` [dpdk-dev] [PATCH v4] " Thomas Monjalon
2020-05-04 12:49         ` [dpdk-dev] [PATCH v5] " Olivier Matz
2020-05-04 12:54           ` Andrew Rybchenko
2020-05-04 15:47             ` Lukasz Wojciechowski
2020-05-04 22:30               ` Thomas Monjalon
2020-04-27 11:44       ` [dpdk-dev] [PATCH v3] " Ray Kinsella
2020-04-27 18:02         ` Lukasz Wojciechowski
2020-01-20 12:02   ` [dpdk-dev] [PATCH v2 0/3] mempool: fix mempool virt populate with small chunks Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJFAV8w-a-2Zrd7n5V-hckLWYZ70wWNXvWHxrYdqrorEO+wecw@mail.gmail.com \
    --to=david.marchand@redhat.com \
    --cc=alvinx.zhang@intel.com \
    --cc=anatoly.burakov@intel.com \
    --cc=arybchenko@solarflare.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).