From: Yongseok Koh <yskoh@mellanox.com>
To: Anatoly Burakov <anatoly.burakov@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
Shahaf Shuler <shahafs@mellanox.com>,
Thomas Monjalon <thomas@monjalon.net>,
"shreyansh.jain@nxp.com" <shreyansh.jain@nxp.com>
Subject: Re: [dpdk-dev] [PATCH 2/4] malloc: separate destroying memseg list and heap data
Date: Fri, 14 Dec 2018 09:34:15 +0000 [thread overview]
Message-ID: <20181214093406.GB12221@mtidpdk.mti.labs.mlnx> (raw)
In-Reply-To: <54cb382e91119cfbef7c34db4406f8ea39428511.1543495935.git.anatoly.burakov@intel.com>
On Thu, Nov 29, 2018 at 01:48:33PM +0000, Anatoly Burakov wrote:
> Currently, destroying external heap chunk and its memseg list is
> part of one process. When we will gain the ability to unregister
> external memory from DPDK that doesn't have any heap structures
> associated with it, we need to be able to find and destroy
> memseg lists as well as heap data separately.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>
Thanks
> lib/librte_eal/common/malloc_heap.c | 70 +++++++++++++++----
> lib/librte_eal/common/malloc_heap.h | 6 ++
> lib/librte_eal/common/rte_malloc.c | 104 ++++++++++------------------
> 3 files changed, 102 insertions(+), 78 deletions(-)
>
> diff --git a/lib/librte_eal/common/malloc_heap.c b/lib/librte_eal/common/malloc_heap.c
> index 25693481f..fa0cb0799 100644
> --- a/lib/librte_eal/common/malloc_heap.c
> +++ b/lib/librte_eal/common/malloc_heap.c
> @@ -1067,12 +1067,9 @@ malloc_heap_dump(struct malloc_heap *heap, FILE *f)
> }
>
> static int
> -destroy_seg(struct malloc_elem *elem, size_t len)
> +destroy_elem(struct malloc_elem *elem, size_t len)
> {
> struct malloc_heap *heap = elem->heap;
> - struct rte_memseg_list *msl;
> -
> - msl = elem->msl;
>
> /* notify all subscribers that a memory area is going to be removed */
> eal_memalloc_mem_event_notify(RTE_MEM_EVENT_FREE, elem, len);
> @@ -1085,13 +1082,6 @@ destroy_seg(struct malloc_elem *elem, size_t len)
>
> memset(elem, 0, sizeof(*elem));
>
> - /* destroy the fbarray backing this memory */
> - if (rte_fbarray_destroy(&msl->memseg_arr) < 0)
> - return -1;
> -
> - /* reset the memseg list */
> - memset(msl, 0, sizeof(*msl));
> -
> return 0;
> }
>
> @@ -1158,6 +1148,62 @@ malloc_heap_create_external_seg(void *va_addr, rte_iova_t iova_addrs[],
> return msl;
> }
>
> +struct extseg_walk_arg {
> + void *va_addr;
> + size_t len;
> + struct rte_memseg_list *msl;
> +};
> +
> +static int
> +extseg_walk(const struct rte_memseg_list *msl, void *arg)
> +{
> + struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
> + struct extseg_walk_arg *wa = arg;
> +
> + if (msl->base_va == wa->va_addr && msl->len == wa->len) {
> + unsigned int found_idx;
> +
> + /* msl is const */
> + found_idx = msl - mcfg->memsegs;
> + wa->msl = &mcfg->memsegs[found_idx];
> + return 1;
> + }
> + return 0;
> +}
> +
> +struct rte_memseg_list *
> +malloc_heap_find_external_seg(void *va_addr, size_t len)
> +{
> + struct extseg_walk_arg wa;
> + int res;
> +
> + wa.va_addr = va_addr;
> + wa.len = len;
> +
> + res = rte_memseg_list_walk_thread_unsafe(extseg_walk, &wa);
> +
> + if (res != 1) {
> + /* 0 means nothing was found, -1 shouldn't happen */
> + if (res == 0)
> + rte_errno = ENOENT;
> + return NULL;
> + }
> + return wa.msl;
> +}
> +
> +int
> +malloc_heap_destroy_external_seg(struct rte_memseg_list *msl)
> +{
> + /* destroy the fbarray backing this memory */
> + if (rte_fbarray_destroy(&msl->memseg_arr) < 0)
> + return -1;
> +
> + /* reset the memseg list */
> + memset(msl, 0, sizeof(*msl));
> +
> + return 0;
> +}
> +
> int
> malloc_heap_add_external_memory(struct malloc_heap *heap,
> struct rte_memseg_list *msl)
> @@ -1206,7 +1252,7 @@ malloc_heap_remove_external_memory(struct malloc_heap *heap, void *va_addr,
> rte_errno = EBUSY;
> return -1;
> }
> - return destroy_seg(elem, len);
> + return destroy_elem(elem, len);
> }
>
> int
> diff --git a/lib/librte_eal/common/malloc_heap.h b/lib/librte_eal/common/malloc_heap.h
> index 255a315b8..ca9ff666f 100644
> --- a/lib/librte_eal/common/malloc_heap.h
> +++ b/lib/librte_eal/common/malloc_heap.h
> @@ -44,6 +44,12 @@ malloc_heap_create_external_seg(void *va_addr, rte_iova_t iova_addrs[],
> unsigned int n_pages, size_t page_sz, const char *seg_name,
> unsigned int socket_id);
>
> +struct rte_memseg_list *
> +malloc_heap_find_external_seg(void *va_addr, size_t len);
> +
> +int
> +malloc_heap_destroy_external_seg(struct rte_memseg_list *msl);
> +
> int
> malloc_heap_add_external_memory(struct malloc_heap *heap,
> struct rte_memseg_list *msl);
> diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
> index 66bfe63c3..9a82e3386 100644
> --- a/lib/librte_eal/common/rte_malloc.c
> +++ b/lib/librte_eal/common/rte_malloc.c
> @@ -396,6 +396,7 @@ rte_malloc_heap_memory_remove(const char *heap_name, void *va_addr, size_t len)
> {
> struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
> struct malloc_heap *heap = NULL;
> + struct rte_memseg_list *msl;
> int ret;
>
> if (heap_name == NULL || va_addr == NULL || len == 0 ||
> @@ -420,9 +421,19 @@ rte_malloc_heap_memory_remove(const char *heap_name, void *va_addr, size_t len)
> goto unlock;
> }
>
> + msl = malloc_heap_find_external_seg(va_addr, len);
> + if (msl == NULL) {
> + ret = -1;
> + goto unlock;
> + }
> +
> rte_spinlock_lock(&heap->lock);
> ret = malloc_heap_remove_external_memory(heap, va_addr, len);
> rte_spinlock_unlock(&heap->lock);
> + if (ret != 0)
> + goto unlock;
> +
> + ret = malloc_heap_destroy_external_seg(msl);
>
> unlock:
> rte_rwlock_write_unlock(&mcfg->memory_hotplug_lock);
> @@ -430,63 +441,12 @@ rte_malloc_heap_memory_remove(const char *heap_name, void *va_addr, size_t len)
> return ret;
> }
>
> -struct sync_mem_walk_arg {
> - void *va_addr;
> - size_t len;
> - int result;
> - bool attach;
> -};
> -
> -static int
> -sync_mem_walk(const struct rte_memseg_list *msl, void *arg)
> -{
> - struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
> - struct sync_mem_walk_arg *wa = arg;
> - size_t len = msl->page_sz * msl->memseg_arr.len;
> -
> - if (msl->base_va == wa->va_addr &&
> - len == wa->len) {
> - struct rte_memseg_list *found_msl;
> - int msl_idx, ret;
> -
> - /* msl is const */
> - msl_idx = msl - mcfg->memsegs;
> - found_msl = &mcfg->memsegs[msl_idx];
> -
> - if (wa->attach) {
> - ret = rte_fbarray_attach(&found_msl->memseg_arr);
> - } else {
> - /* notify all subscribers that a memory area is about to
> - * be removed
> - */
> - eal_memalloc_mem_event_notify(RTE_MEM_EVENT_FREE,
> - msl->base_va, msl->len);
> - ret = rte_fbarray_detach(&found_msl->memseg_arr);
> - }
> -
> - if (ret < 0) {
> - wa->result = -rte_errno;
> - } else {
> - /* notify all subscribers that a new memory area was
> - * added
> - */
> - if (wa->attach)
> - eal_memalloc_mem_event_notify(
> - RTE_MEM_EVENT_ALLOC,
> - msl->base_va, msl->len);
> - wa->result = 0;
> - }
> - return 1;
> - }
> - return 0;
> -}
> -
> static int
> sync_memory(const char *heap_name, void *va_addr, size_t len, bool attach)
> {
> struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
> struct malloc_heap *heap = NULL;
> - struct sync_mem_walk_arg wa;
> + struct rte_memseg_list *msl;
> int ret;
>
> if (heap_name == NULL || va_addr == NULL || len == 0 ||
> @@ -513,23 +473,35 @@ sync_memory(const char *heap_name, void *va_addr, size_t len, bool attach)
> }
>
> /* find corresponding memseg list to sync to */
> - wa.va_addr = va_addr;
> - wa.len = len;
> - wa.result = -ENOENT; /* fail unless explicitly told to succeed */
> - wa.attach = attach;
> -
> - /* we're already holding a read lock */
> - rte_memseg_list_walk_thread_unsafe(sync_mem_walk, &wa);
> -
> - if (wa.result < 0) {
> - rte_errno = -wa.result;
> + msl = malloc_heap_find_external_seg(va_addr, len);
> + if (msl == NULL) {
> ret = -1;
> - } else {
> - /* notify all subscribers that a new memory area was added */
> - if (attach)
> + goto unlock;
> + }
> +
> + if (attach) {
> + ret = rte_fbarray_attach(&msl->memseg_arr);
> + if (ret == 0) {
> + /* notify all subscribers that a new memory area was
> + * added.
> + */
> eal_memalloc_mem_event_notify(RTE_MEM_EVENT_ALLOC,
> va_addr, len);
> - ret = 0;
> + } else {
> + ret = -1;
> + goto unlock;
> + }
> + } else {
> + /* notify all subscribers that a memory area is about to
> + * be removed.
> + */
> + eal_memalloc_mem_event_notify(RTE_MEM_EVENT_FREE,
> + msl->base_va, msl->len);
> + ret = rte_fbarray_detach(&msl->memseg_arr);
> + if (ret < 0) {
> + ret = -1;
> + goto unlock;
> + }
> }
> unlock:
> rte_rwlock_read_unlock(&mcfg->memory_hotplug_lock);
> --
> 2.17.1
next prev parent reply other threads:[~2018-12-14 9:34 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-29 13:48 [dpdk-dev] [PATCH 0/4] Allow using external memory without malloc Anatoly Burakov
2018-11-29 13:48 ` [dpdk-dev] [PATCH 1/4] malloc: separate creating memseg list and malloc heap Anatoly Burakov
2018-12-14 9:33 ` Yongseok Koh
2018-11-29 13:48 ` [dpdk-dev] [PATCH 2/4] malloc: separate destroying memseg list and heap data Anatoly Burakov
2018-12-14 9:34 ` Yongseok Koh [this message]
2018-11-29 13:48 ` [dpdk-dev] [PATCH 3/4] mem: allow registering external memory areas Anatoly Burakov
2018-12-14 9:55 ` Yongseok Koh
2018-12-14 11:03 ` Burakov, Anatoly
2018-11-29 13:48 ` [dpdk-dev] [PATCH 4/4] mem: allow usage of non-heap external memory in multiprocess Anatoly Burakov
2018-12-14 9:56 ` Yongseok Koh
2018-12-02 5:48 ` [dpdk-dev] [PATCH 0/4] Allow using external memory without malloc Shahaf Shuler
2018-12-02 23:28 ` Yongseok Koh
2018-12-03 10:23 ` Burakov, Anatoly
2018-12-12 12:55 ` Yongseok Koh
2018-12-12 13:17 ` Burakov, Anatoly
2018-12-14 11:50 ` [dpdk-dev] [PATCH v2 " Anatoly Burakov
2018-12-20 15:32 ` [dpdk-dev] [PATCH v3 " Anatoly Burakov
2018-12-20 16:16 ` Stephen Hemminger
2018-12-20 17:18 ` Thomas Monjalon
2018-12-21 9:17 ` Burakov, Anatoly
2018-12-20 17:17 ` Thomas Monjalon
2018-12-20 15:32 ` [dpdk-dev] [PATCH v3 1/4] malloc: separate creating memseg list and malloc heap Anatoly Burakov
2018-12-20 15:32 ` [dpdk-dev] [PATCH v3 2/4] malloc: separate destroying memseg list and heap data Anatoly Burakov
2018-12-20 15:32 ` [dpdk-dev] [PATCH v3 3/4] mem: allow registering external memory areas Anatoly Burakov
2018-12-20 15:32 ` [dpdk-dev] [PATCH v3 4/4] mem: allow usage of non-heap external memory in multiprocess Anatoly Burakov
2018-12-14 11:50 ` [dpdk-dev] [PATCH v2 1/4] malloc: separate creating memseg list and malloc heap Anatoly Burakov
2018-12-14 11:50 ` [dpdk-dev] [PATCH v2 2/4] malloc: separate destroying memseg list and heap data Anatoly Burakov
2018-12-14 11:50 ` [dpdk-dev] [PATCH v2 3/4] mem: allow registering external memory areas Anatoly Burakov
2018-12-14 11:50 ` [dpdk-dev] [PATCH v2 4/4] mem: allow usage of non-heap external memory in multiprocess Anatoly Burakov
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=20181214093406.GB12221@mtidpdk.mti.labs.mlnx \
--to=yskoh@mellanox.com \
--cc=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=shahafs@mellanox.com \
--cc=shreyansh.jain@nxp.com \
--cc=thomas@monjalon.net \
/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).