From: David Marchand <david.marchand@redhat.com>
To: Anatoly Burakov <anatoly.burakov@intel.com>
Cc: dev <dev@dpdk.org>, John McNamara <john.mcnamara@intel.com>,
Marko Kovacevic <marko.kovacevic@intel.com>,
Bruce Richardson <bruce.richardson@intel.com>,
Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v2 1/2] eal/freebsd: add support for base virtaddr option
Date: Thu, 24 Oct 2019 20:56:37 +0200 [thread overview]
Message-ID: <CAJFAV8wmSawATMokBZ_p05H6z7ANwjtmY3zgV48CPnu5FtdK9A@mail.gmail.com> (raw)
In-Reply-To: <c484c4f62e188f0f2524d26faabe8558b3ad3816.1571930271.git.anatoly.burakov@intel.com>
On Thu, Oct 24, 2019 at 5:18 PM Anatoly Burakov
<anatoly.burakov@intel.com> wrote:
>
> According to our docs, only Linuxapp supports base-virtaddr option.
> That is, strictly speaking, not true because most of the things
> that are attempting to respect base-virtaddr are in common files,
> so FreeBSD already *mostly* supports this option in practice.
>
> This commit fixes the remaining bits to explicitly support
> base-virtaddr option, and moves the arg parsing from EAL to common
> options parsing code. Documentation is also updated to reflect
> that all platforms now support base-virtaddr.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> ---
>
> Notes:
> v2:
> - Harmonize FreeBSD reattach implementation with Linux
>
> doc/guides/linux_gsg/eal_args.include.rst | 6 +++
> doc/guides/linux_gsg/linux_eal_parameters.rst | 6 ---
> doc/guides/rel_notes/release_19_11.rst | 5 +++
> lib/librte_eal/common/eal_common_options.c | 38 ++++++++++++++++++
I was about to apply it, and I realised that the usage() in Linux
still references the --base-virtaddr option but it was not moved to
the common code.
What do you think of this hunk?
diff --git a/lib/librte_eal/common/eal_common_options.c
b/lib/librte_eal/common/eal_common_options.c
index 1cdbd35..020f36e 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -1692,6 +1692,7 @@ static int xdigit2val(unsigned char c)
" --"OPT_NO_PCI" Disable PCI\n"
" --"OPT_NO_HPET" Disable HPET\n"
" --"OPT_NO_SHCONF" No shared config (mmap'd files)\n"
+ " --"OPT_BASE_VIRTADDR" Base virtual address\n"
"\n", RTE_MAX_LCORE);
rte_option_usage();
}
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index ef5dafa..c0aac21 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -539,7 +539,6 @@ enum rte_proc_type_t
" --"OPT_SOCKET_LIMIT" Limit memory allocation on
sockets (comma separated values)\n"
" --"OPT_HUGE_DIR" Directory where hugetlbfs
is mounted\n"
" --"OPT_FILE_PREFIX" Prefix for hugepage filenames\n"
- " --"OPT_BASE_VIRTADDR" Base virtual address\n"
" --"OPT_CREATE_UIO_DEV" Create /dev/uioX (usually
done by hotplug)\n"
" --"OPT_VFIO_INTR" Interrupt mode for VFIO
(legacy|msi|msix)\n"
" --"OPT_LEGACY_MEM" Legacy memory mode (no
dynamic allocation, contiguous segments)\n"
> lib/librte_eal/freebsd/eal/eal.c | 31 ++++++++++-----
> lib/librte_eal/linux/eal/eal.c | 39 -------------------
> 6 files changed, 70 insertions(+), 55 deletions(-)
>
> diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst
> index cf421a56eb..ed8b0e35b0 100644
> --- a/doc/guides/linux_gsg/eal_args.include.rst
> +++ b/doc/guides/linux_gsg/eal_args.include.rst
> @@ -86,6 +86,12 @@ Multiprocessing-related options
>
> Set the type of the current process.
>
> +* ``--base-virtaddr <address>``
> +
> + Attempt to use a different starting address for all memory maps of the
> + primary DPDK process. This can be helpful if secondary processes cannot
> + start due to conflicts in address map.
> +
> Memory-related options
> ~~~~~~~~~~~~~~~~~~~~~~
>
> diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
> index c63f0f49a0..b2cc60e447 100644
> --- a/doc/guides/linux_gsg/linux_eal_parameters.rst
> +++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
> @@ -49,12 +49,6 @@ Multiprocessing-related options
> allows running multiple independent DPDK primary/secondary processes under
> different prefixes.
>
> -* ``--base-virtaddr <address>``
> -
> - Attempt to use a different starting address for all memory maps of the
> - primary DPDK process. This can be helpful if secondary processes cannot
> - start due to conflicts in address map.
> -
> Memory-related options
> ~~~~~~~~~~~~~~~~~~~~~~
>
> diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> index 856088c5c7..95231d13dd 100644
> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -56,6 +56,11 @@ New Features
> Also, make sure to start the actual text at the margin.
> =========================================================
>
> +* **FreeBSD now supports `--base-virtaddr` EAL option.**
> +
> + FreeBSD version now also supports setting base virtual address for mapping
> + pages and resources into its address space.
> +
> * **Added Lock-free Stack for aarch64.**
>
> The lock-free stack implementation is enabled for aarch64 platforms.
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 05cae5f75c..1cdbd35655 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -20,6 +20,7 @@
> #include <rte_eal.h>
> #include <rte_log.h>
> #include <rte_lcore.h>
> +#include <rte_memory.h>
> #include <rte_tailq.h>
> #include <rte_version.h>
> #include <rte_devargs.h>
> @@ -1095,6 +1096,36 @@ eal_parse_iova_mode(const char *name)
> return 0;
> }
>
> +static int
> +eal_parse_base_virtaddr(const char *arg)
> +{
> + char *end;
> + uint64_t addr;
> +
> + errno = 0;
> + addr = strtoull(arg, &end, 16);
> +
> + /* check for errors */
> + if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0'))
> + return -1;
> +
> + /* make sure we don't exceed 32-bit boundary on 32-bit target */
> +#ifndef RTE_ARCH_64
> + if (addr >= UINTPTR_MAX)
> + return -1;
> +#endif
> +
> + /* align the addr on 16M boundary, 16MB is the minimum huge page
> + * size on IBM Power architecture. If the addr is aligned to 16MB,
> + * it can align to 2MB for x86. So this alignment can also be used
> + * on x86 and other architectures.
> + */
> + internal_config.base_virtaddr =
> + RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
> +
> + return 0;
> +}
> +
> /* caller is responsible for freeing the returned string */
> static char *
> available_cores(void)
> @@ -1408,6 +1439,13 @@ eal_parse_common_option(int opt, const char *optarg,
> return -1;
> }
> break;
> + case OPT_BASE_VIRTADDR_NUM:
> + if (eal_parse_base_virtaddr(optarg) < 0) {
> + RTE_LOG(ERR, EAL, "invalid parameter for --"
> + OPT_BASE_VIRTADDR "\n");
> + return -1;
> + }
> + break;
>
> /* don't know what to do, leave this to caller */
> default:
> diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
> index f86e9aa318..c7e1264205 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -226,6 +226,14 @@ rte_eal_config_create(void)
> if (internal_config.no_shconf)
> return 0;
>
> + /* map the config before base address so that we don't waste a page */
> + if (internal_config.base_virtaddr != 0)
> + rte_mem_cfg_addr = (void *)
> + RTE_ALIGN_FLOOR(internal_config.base_virtaddr -
> + sizeof(struct rte_mem_config), sysconf(_SC_PAGE_SIZE));
> + else
> + rte_mem_cfg_addr = NULL;
> +
> if (mem_cfg_fd < 0){
> mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0600);
> if (mem_cfg_fd < 0) {
> @@ -253,8 +261,9 @@ rte_eal_config_create(void)
> return -1;
> }
>
> - rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
> - PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
> + rte_mem_cfg_addr = mmap(rte_mem_cfg_addr,
> + sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE,
> + MAP_SHARED, mem_cfg_fd, 0);
>
> if (rte_mem_cfg_addr == MAP_FAILED){
> RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n");
> @@ -332,15 +341,17 @@ rte_eal_config_reattach(void)
> close(mem_cfg_fd);
> mem_cfg_fd = -1;
>
> - if (mem_config == MAP_FAILED) {
> + if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
> + if (mem_config != MAP_FAILED) {
> + /* errno is stale, don't use */
> + RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]"
> + " - please use '--base-virtaddr' option\n",
> + rte_mem_cfg_addr, mem_config);
> + munmap(mem_config, sizeof(struct rte_mem_config));
> + return -1;
> + }
> RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config! error %i (%s)\n",
> - errno, strerror(errno));
> - return -1;
> - } else if (mem_config != rte_mem_cfg_addr) {
> - /* errno is stale, don't use */
> - RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]\n",
> - rte_mem_cfg_addr, mem_config);
> - munmap(mem_config, sizeof(struct rte_mem_config));
> + errno, strerror(errno));
> return -1;
> }
>
> diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
> index f39720637f..ef5dafa94f 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -611,35 +611,6 @@ eal_parse_socket_arg(char *strval, volatile uint64_t *socket_arg)
> return 0;
> }
>
> -static int
> -eal_parse_base_virtaddr(const char *arg)
> -{
> - char *end;
> - uint64_t addr;
> -
> - errno = 0;
> - addr = strtoull(arg, &end, 16);
> -
> - /* check for errors */
> - if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0'))
> - return -1;
> -
> - /* make sure we don't exceed 32-bit boundary on 32-bit target */
> -#ifndef RTE_ARCH_64
> - if (addr >= UINTPTR_MAX)
> - return -1;
> -#endif
> -
> - /* align the addr on 16M boundary, 16MB is the minimum huge page
> - * size on IBM Power architecture. If the addr is aligned to 16MB,
> - * it can align to 2MB for x86. So this alignment can also be used
> - * on x86 */
> - internal_config.base_virtaddr =
> - RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
> -
> - return 0;
> -}
> -
> static int
> eal_parse_vfio_intr(const char *mode)
> {
> @@ -798,16 +769,6 @@ eal_parse_args(int argc, char **argv)
> internal_config.force_socket_limits = 1;
> break;
>
> - case OPT_BASE_VIRTADDR_NUM:
> - if (eal_parse_base_virtaddr(optarg) < 0) {
> - RTE_LOG(ERR, EAL, "invalid parameter for --"
> - OPT_BASE_VIRTADDR "\n");
> - eal_usage(prgname);
> - ret = -1;
> - goto out;
> - }
> - break;
> -
> case OPT_VFIO_INTR_NUM:
> if (eal_parse_vfio_intr(optarg) < 0) {
> RTE_LOG(ERR, EAL, "invalid parameters for --"
> --
> 2.17.1
next prev parent reply other threads:[~2019-10-24 18:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-16 11:25 [dpdk-dev] [PATCH] " Anatoly Burakov
2019-07-22 16:07 ` Thomas Monjalon
2019-08-12 10:19 ` David Marchand
2019-08-12 13:38 ` Burakov, Anatoly
2019-09-26 13:55 ` David Marchand
2019-10-24 15:17 ` [dpdk-dev] [PATCH v2 1/2] " Anatoly Burakov
2019-10-24 18:56 ` David Marchand [this message]
2019-10-25 9:05 ` Burakov, Anatoly
2019-10-25 9:36 ` David Marchand
2019-10-24 15:17 ` [dpdk-dev] [PATCH v2 2/2] eal: use define instead of raw flag name Anatoly Burakov
2019-10-24 18:59 ` David Marchand
2019-10-25 9:36 ` David Marchand
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=CAJFAV8wmSawATMokBZ_p05H6z7ANwjtmY3zgV48CPnu5FtdK9A@mail.gmail.com \
--to=david.marchand@redhat.com \
--cc=anatoly.burakov@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=john.mcnamara@intel.com \
--cc=marko.kovacevic@intel.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).