From: David Marchand <david.marchand@redhat.com>
To: "Burakov, Anatoly" <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>
Subject: Re: [dpdk-dev] [PATCH] eal/freebsd: add support for base virtaddr option
Date: Thu, 26 Sep 2019 15:55:10 +0200 [thread overview]
Message-ID: <CAJFAV8xh-9p5C3o4fN502eFjkY3a8kCQ-a+yu+JaGT2vW2Dwsw@mail.gmail.com> (raw)
In-Reply-To: <9e3271a8-8f86-a2ea-35ee-2ed3d98c5f0f@intel.com>
Hello Anatoly,
On Mon, Aug 12, 2019 at 3:38 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 12-Aug-19 11:19 AM, David Marchand wrote:
> > On Tue, Jul 16, 2019 at 1:25 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>
> >> ---
> >> doc/guides/linux_gsg/eal_args.include.rst | 6 +++
> >> doc/guides/linux_gsg/linux_eal_parameters.rst | 6 ---
> >> doc/guides/rel_notes/release_19_08.rst | 5 +++
> >> lib/librte_eal/common/eal_common_options.c | 38 ++++++++++++++++++
> >> lib/librte_eal/freebsd/eal/eal.c | 13 ++++++-
> >> lib/librte_eal/linux/eal/eal.c | 39 -------------------
> >> 6 files changed, 60 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst
> >> index cf421a56e..ed8b0e35b 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.
> >> +
> >
> > doc/guides/freebsd_gsg/freebsd_eal_parameters.rst:.. include::
> > ../linux_gsg/eal_args.include.rst
> >
> > Ok, a bit misleading to put in linux_gsg/, so writing this here if
> > someone else looks at this :-)
>
> This was agreed upon when this file was first introduced. We don't have
> a "common" section and there's no real way to create it without
> triggering a bunch of errors in doxygen, so it was decided that putting
> this in a Linux GSG is the best way to do this.
>
> >
> >
> >> Memory-related options
> >> ~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
> >> index c63f0f49a..b2cc60e44 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_08.rst b/doc/guides/rel_notes/release_19_08.rst
> >> index 4a1fd8dd8..1b58d9282 100644
> >> --- a/doc/guides/rel_notes/release_19_08.rst
> >> +++ b/doc/guides/rel_notes/release_19_08.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 MCS lock.**
> >>
> >> MCS lock provides scalability by spinning on a CPU/thread local variable
> >
> > Well, obviously, this needs some rebase on 19.11-rc0 :-)
>
> Yes, will do.
>
> >
> >> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> >> index 512d5088e..156e48e19 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>
>
> <snip>
>
> >> - 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");
> >
> > Nit: when compared to Linux implementation, the reattach step does not
> > recommend using the --base-virtaddr in case the remmapping failed.
> >
>
> Good catch, will fix.
Can you prepare a v2?
Thanks.
--
David Marchand
next prev parent reply other threads:[~2019-09-26 13:55 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-16 11:25 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 [this message]
2019-10-24 15:17 ` [dpdk-dev] [PATCH v2 1/2] " Anatoly Burakov
2019-10-24 18:56 ` David Marchand
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=CAJFAV8xh-9p5C3o4fN502eFjkY3a8kCQ-a+yu+JaGT2vW2Dwsw@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 \
/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).