patches for DPDK stable branches
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: David Marchand <david.marchand@redhat.com>, dev@dpdk.org
Cc: olivier.matz@6wind.com, ktraynor@redhat.com, stable@dpdk.org
Subject: Re: [dpdk-stable] [PATCH v2 2/2] eal: restrict ctrl threads to startup cpu affinity
Date: Tue, 19 Feb 2019 11:38:29 +0000	[thread overview]
Message-ID: <6d3960bc-9798-fa79-2538-339d63caf81b@intel.com> (raw)
In-Reply-To: <1550151042-9764-2-git-send-email-david.marchand@redhat.com>

On 14-Feb-19 1:30 PM, David Marchand wrote:
> Spawning the ctrl threads on anything that is not part of the eal
> coremask is not that polite to the rest of the system, especially
> when you took good care to pin your processes on cpu resources with
> tools like taskset (linux) / cpuset (freebsd).
> 
> Rather than introduce yet another eal options to control on which cpu
> those ctrl threads are created, let's take the startup cpu affinity
> as a reference and remove the eal coremask from it.
> If no cpu is left, then we default to the master core.
> 
> The cpuset is computed once at init before the original cpu affinity
> is lost.
> 
> Introduced a RTE_CPU_AND macro to abstract the differences between linux
> and freebsd respective macros.
> 
> Examples in a 4 cores FreeBSD vm:
> 
> $ ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
>   -- -i --total-num-mbufs=2048
> 
> $ procstat -S 1057
>    PID    TID COMM                TDNAME              CPU CSID CPU MASK
>   1057 100131 testpmd             -                     2    1 2
>   1057 100140 testpmd             eal-intr-thread       1    1 0-1
>   1057 100141 testpmd             rte_mp_handle         1    1 0-1
>   1057 100142 testpmd             lcore-slave-3         3    1 3
> 
> $ cpuset -l 1,2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
>   -- -i --total-num-mbufs=2048
> 
> $ procstat -S 1061
>    PID    TID COMM                TDNAME              CPU CSID CPU MASK
>   1061 100131 testpmd             -                     2    2 2
>   1061 100144 testpmd             eal-intr-thread       1    2 1
>   1061 100145 testpmd             rte_mp_handle         1    2 1
>   1061 100147 testpmd             lcore-slave-3         3    2 3
> 
> $ cpuset -l 2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
>   -- -i --total-num-mbufs=2048
> 
> $ procstat -S 1065
>    PID    TID COMM                TDNAME              CPU CSID CPU MASK
>   1065 100131 testpmd             -                     2    2 2
>   1065 100148 testpmd             eal-intr-thread       2    2 2
>   1065 100149 testpmd             rte_mp_handle         2    2 2
>   1065 100150 testpmd             lcore-slave-3         3    2 3
> 
> Fixes: d651ee4919cd ("eal: set affinity for control threads")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> 
> Changes since v1:
> - added some description in the prog guide
> - fixed FreeBSD build
> 
> ---
>   doc/guides/prog_guide/env_abstraction_layer.rst | 14 +++++++++++++
>   lib/librte_eal/common/eal_common_options.c      | 28 +++++++++++++++++++++++++
>   lib/librte_eal/common/eal_common_thread.c       | 21 ++++---------------
>   lib/librte_eal/common/eal_internal_cfg.h        |  3 +++
>   lib/librte_eal/common/include/rte_lcore.h       | 17 +++++++++++----
>   5 files changed, 62 insertions(+), 21 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst b/doc/guides/prog_guide/env_abstraction_layer.rst
> index 929d76d..bfc66d9 100644
> --- a/doc/guides/prog_guide/env_abstraction_layer.rst
> +++ b/doc/guides/prog_guide/env_abstraction_layer.rst
> @@ -498,6 +498,20 @@ Those TLS include *_cpuset* and *_socket_id*:
>   *	*_socket_id* stores the NUMA node of the CPU set. If the CPUs in CPU set belong to different NUMA node, the *_socket_id* will be set to SOCKET_ID_ANY.
>   
>   
> +Control Thread API
> +~~~~~~~~~~~~~~~~~~
> +
> +It is possible to create Control Threads using the public API ``rte_ctrl_thread_create()``.
> +Those threads can be used for management/infrastructure tasks and are used internally by DPDK for multi process support and interrupt handling.
> +
> +Those threads will be scheduled on cpus part of the original process cpu affinity from which the dataplane and service lcores are excluded.
> +
> +For example, on a 8 cpus system, starting a dpdk application with -l 2,3 (dataplane cores), then depending on the affinity configuration which can be controlled with tools like taskset (Linux) or cpuset (FreeBSD),
> +
> +- with no affinity configuration, the Control Threads will end up on 0-1,4-7 cpus.
> +- with affinity restricted to 2-4, the Control Threads will end up on cpu 4.
> +- with affinity restricted to 2-3, the Control Threads will end up on cpu 2 (master lcore, which is the default when no cpu is available).

You're not winning anything by foregoing the 80 char limit on 
documentation (doxygen will still generate this correctly), but you're 
losing in readability when working in terminal. I would prefer if you 
didn't do those long lines :)

Thomas, do we want checkpatch to warn about this?

> +
>   .. _known_issue_label:
>   
>   Known Issues
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 1f45f82..fca3f83 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -217,6 +217,7 @@ struct device_option {
>   	internal_cfg->create_uio_dev = 0;
>   	internal_cfg->iova_mode = RTE_IOVA_DC;
>   	internal_cfg->user_mbuf_pool_ops_name = NULL;
> +	CPU_ZERO(&internal_cfg->ctrl_cpuset);
>   	internal_cfg->init_complete = 0;
>   }
>   
> @@ -1359,6 +1360,31 @@ static int xdigit2val(unsigned char c)
>   	cfg->lcore_count -= removed;
>   }
>   
> +static void
> +compute_ctrl_threads_cpuset(struct internal_config *internal_cfg)
> +{
> +	rte_cpuset_t *cpuset = &internal_cfg->ctrl_cpuset;
> +	rte_cpuset_t default_set;
> +	unsigned int lcore_id;
> +
> +	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
> +		if (eal_cpu_detected(lcore_id) &&
> +				rte_lcore_has_role(lcore_id, ROLE_OFF)) {
> +			CPU_SET(lcore_id, cpuset);
> +		}
> +	}
> +
> +	if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> +				&default_set) < 0)

Shouldn't this be != 0? Manpage doesn't say the error values will be 
negative.

-- 
Thanks,
Anatoly

  reply	other threads:[~2019-02-19 11:38 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-13 16:13 [dpdk-stable] [PATCH] " David Marchand
2019-02-13 20:21 ` [dpdk-stable] [dpdk-dev] " David Marchand
2019-02-14  9:39 ` [dpdk-stable] " Burakov, Anatoly
2019-02-14  9:53   ` David Marchand
2019-02-14 10:04     ` Burakov, Anatoly
2019-02-14 10:16       ` David Marchand
2019-02-14 11:05 ` [dpdk-stable] [dpdk-dev] " David Marchand
2019-02-14 13:30 ` [dpdk-stable] [PATCH v2 1/2] eal: fix potential incorrect pinning for ctrl threads David Marchand
2019-02-14 13:30   ` [dpdk-stable] [PATCH v2 2/2] eal: restrict ctrl threads to startup cpu affinity David Marchand
2019-02-19 11:38     ` Burakov, Anatoly [this message]
2019-02-19 11:51       ` David Marchand
2019-02-19 16:03         ` Thomas Monjalon
2019-02-14 16:12   ` [dpdk-stable] [PATCH v2 1/2] eal: fix potential incorrect pinning for ctrl threads Burakov, Anatoly
2019-02-14 17:45     ` David Marchand
2019-02-19 20:41   ` [dpdk-stable] [PATCH v3 " David Marchand
2019-02-19 20:41     ` [dpdk-stable] [PATCH v3 2/2] eal: restrict ctrl threads to startup cpu affinity David Marchand
2019-02-20 16:01       ` Burakov, Anatoly
2019-02-25  8:33         ` Olivier Matz
2019-03-07 18:23           ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
2019-02-20 16:01     ` [dpdk-stable] [PATCH v3 1/2] eal: fix potential incorrect pinning for ctrl threads Burakov, Anatoly
2019-02-25  8:33       ` Olivier Matz

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=6d3960bc-9798-fa79-2538-339d63caf81b@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ktraynor@redhat.com \
    --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).