From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Lilijun <jerry.lilijun@huawei.com>, dev@dpdk.org
Cc: jerry.zhang@intel.com, ian.stokes@intel.com
Subject: Re: [dpdk-dev] [PATCH] eal: unmap unneed dpdk VA spaces for legacy mem
Date: Fri, 8 Mar 2019 09:37:50 +0000 [thread overview]
Message-ID: <5d2df2f7-76a7-4fb3-2911-af6a431a80a1@intel.com> (raw)
In-Reply-To: <20190308053859.19980-1-jerry.lilijun@huawei.com>
On 08-Mar-19 5:38 AM, Lilijun wrote:
> Comparing dpdk VA spaces to dpdk 16.11, the dpdk app process's VA spaces increase to above 30G.
> Here we can unmap the unneed VA spaces in rte_memseg_list.
>
> Signed-off-by: Lilijun <jerry.lilijun@huawei.com>
> ---
> lib/librte_eal/linuxapp/eal/eal_memory.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index 32feb41..56abdd2 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -1626,8 +1626,19 @@ void numa_error(char *where)
> if (msl->base_va == NULL)
> continue;
> /* skip lists where there is at least one page allocated */
> - if (msl->memseg_arr.count > 0)
> + if (msl->memseg_arr.count > 0) {
> + if (internal_config.legacy_mem) {
> + struct rte_fbarray *arr = &msl->memseg_arr;
> + int idx = rte_fbarray_find_next_free(arr, 0);
> +
> + while (idx >= 0) {
> + void *va = (void*)((char*)msl->base_va + idx * msl->page_sz);
> + munmap(va, msl->page_sz);
> + idx = rte_fbarray_find_next_free(arr, idx + 1);
> + }
I am not entirely convinced this change is safe to do. Technically, this
space is marked as free, so correctly written code should not attempt to
access it, however it is still potentially dangerous to have memory area
that is supposed to be allocated (according to data structures'
parameters), but isn't.
If you are deallocating the VA space, ideally you should also resize the
memseg list (as in, change its length), because that leftover memory
area is no longer valid. However, this then presents us with a mismatch
between (va_start + len) and (va_start + page_sz * memseg_arr.len),
which may break things further.
May i ask what is the purpose of this change? I mean, i understand the
part about unused VA space sitting there, but what is the consequence of
that? This isn't 32-bit codepath, and in 64-bit there's plenty of
address space to go around, and this memory doesn't take up any system
resources anyway because it is read-only anonymous memory, and is
therefore backed by zero page instead of real pages. So, what's wrong
with just leaving it there?
I don't see any advantage of this change, and i see plenty of
disadvantages, so for now i'm inclined to NACK this particular patch.
_However_, i should note that if you feel this is very important feature
to have and would still like to implement it, my advise would be to look
at how 32-bit code works, and model the 64-bit implementation after
that, because 32-bit codepath does exactly what you propose, and doesn't
leave unused address space.
> + } > continue;
> + }
> /* this is an unused list, deallocate it */
> mem_sz = msl->len;
> munmap(msl->base_va, mem_sz);
>
--
Thanks,
Anatoly
next prev parent reply other threads:[~2019-03-08 9:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-08 5:38 Lilijun
2019-03-08 9:37 ` Burakov, Anatoly [this message]
2019-03-12 1:47 ` Lilijun (Jerry, Cloud Networking)
2019-03-12 11:02 ` Burakov, Anatoly
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=5d2df2f7-76a7-4fb3-2911-af6a431a80a1@intel.com \
--to=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=ian.stokes@intel.com \
--cc=jerry.lilijun@huawei.com \
--cc=jerry.zhang@intel.com \
/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).