From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6D5FBA0350; Tue, 30 Jun 2020 12:07:22 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 223D71BEB3; Tue, 30 Jun 2020 12:07:21 +0200 (CEST) Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id ECBC31023 for ; Tue, 30 Jun 2020 12:07:18 +0200 (CEST) Received: by mail-wr1-f66.google.com with SMTP id k6so19513988wrn.3 for ; Tue, 30 Jun 2020 03:07:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=p8K+VXWhWFVgboH5VcyOZoNUpN214++Gu0Xa+Z+NuGQ=; b=fTW3UVr/NK00VpU1sVm4KXt3fqbsnAnJJ4iKP7B0EAdEpcD9+h9pMh8Ye9v9CROCru FuBHBoInyuSgvxgjxAaMmlZn8QE4DfyDib8nEcEFfu1bJsmZqe2q3ZHlFlG1kIzTZcTd Gc0NrK086Q48CtinHrpcXspuFBXjoW/T8CgyACKQRnIE3iAhyxrvXX23JBz4HqzovOSZ su1TOaFezvcMbu7OEP5NLHRVxOLI3wy44e+AhDinjVM73AusxDCJfxg4MHJJZc3B9aeS uplTc6kmqcrBa1LHG9s2RdHIlQWzq8HvPJnqfwZy/Gco0pht8Ste9l7RbWHooX6EIGpx 46jw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=p8K+VXWhWFVgboH5VcyOZoNUpN214++Gu0Xa+Z+NuGQ=; b=HCEMG0/LOt+TaKIIILxo0V5PbeVw5gTPtIlVKVoa/7Jy6p2uyjnJPeSA2J1KSW74gg P0Sd6jWY1gazFGJKtnGplN7+ALQlI69vj/icvDIdbL+P4oC2QpcWs4LMZrsmMNP890Io EO3J8Xc50zsQDvKOOCh+csEYyGtke4VbyVDOynuUoK9ZMk1/n2gwQuvywE9O7fmaboUy W6khKbYjrIadHc00KVRekmiDj948FeaYLYI3H4h78NBgymeFZeKWnBy/XKKQ4yvA1JBM ldgc46MOBJIjsjPpPjrvHbXmknLHsao7nDoqTo+ESuqbfcL1NGbn07/saIPDkW8dglju 1eyQ== X-Gm-Message-State: AOAM531yyOuYNDe0i+Xo/07ITdM0a7a4555BX3rpIy35+CQkUuih5EmH mpA4n7FMAjQTlo6Io8l6OPBJKQ== X-Google-Smtp-Source: ABdhPJziBZrybc6edYzh6WK69Qjyyr/swkBJi65ScLkKJV/NKt+NOprkM9NGkCikowXLJWew+zADBQ== X-Received: by 2002:adf:fa81:: with SMTP id h1mr21305710wrr.266.1593511638491; Tue, 30 Jun 2020 03:07:18 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id a22sm2846081wmb.4.2020.06.30.03.07.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Jun 2020 03:07:17 -0700 (PDT) Date: Tue, 30 Jun 2020 12:07:17 +0200 From: Olivier Matz To: David Marchand Cc: dev@dpdk.org, jerinjacobk@gmail.com, bruce.richardson@intel.com, mdr@ashroe.eu, thomas@monjalon.net, arybchenko@solarflare.com, ktraynor@redhat.com, ian.stokes@intel.com, i.maximets@ovn.org, John McNamara , Marko Kovacevic , Anatoly Burakov , Neil Horman Message-ID: <20200630100717.GF5869@platinum> References: <20200610144506.30505-1-david.marchand@redhat.com> <20200626144736.11011-1-david.marchand@redhat.com> <20200626144736.11011-7-david.marchand@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200626144736.11011-7-david.marchand@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH v4 6/9] eal: register non-EAL threads as lcores X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, Jun 26, 2020 at 04:47:33PM +0200, David Marchand wrote: > DPDK allows calling some part of its API from a non-EAL thread but this > has some limitations. > OVS (and other applications) has its own thread management but still > want to avoid such limitations by hacking RTE_PER_LCORE(_lcore_id) and > faking EAL threads potentially unknown of some DPDK component. > > Introduce a new API to register non-EAL thread and associate them to a > free lcore with a new NON_EAL role. > This role denotes lcores that do not run DPDK mainloop and as such > prevents use of rte_eal_wait_lcore() and consorts. > > Multiprocess is not supported as the need for cohabitation with this new > feature is unclear at the moment. > > Signed-off-by: David Marchand > Acked-by: Andrew Rybchenko > --- > Changes since v2: > - refused multiprocess init once rte_thread_register got called, and > vice versa, > - added warning on multiprocess in rte_thread_register doxygen, > > Changes since v1: > - moved cleanup on lcore role code in patch 5, > - added unit test, > - updated documentation, > - changed naming from "external thread" to "registered non-EAL thread" > > --- > MAINTAINERS | 1 + > app/test/Makefile | 1 + > app/test/autotest_data.py | 6 + > app/test/meson.build | 2 + > app/test/test_lcores.c | 139 ++++++++++++++++++ > doc/guides/howto/debug_troubleshoot.rst | 5 +- > .../prog_guide/env_abstraction_layer.rst | 22 +-- > doc/guides/prog_guide/mempool_lib.rst | 2 +- > lib/librte_eal/common/eal_common_lcore.c | 50 ++++++- > lib/librte_eal/common/eal_common_mcfg.c | 36 +++++ > lib/librte_eal/common/eal_common_thread.c | 33 +++++ > lib/librte_eal/common/eal_memcfg.h | 10 ++ > lib/librte_eal/common/eal_private.h | 18 +++ > lib/librte_eal/freebsd/eal.c | 4 + > lib/librte_eal/include/rte_lcore.h | 25 +++- > lib/librte_eal/linux/eal.c | 4 + > lib/librte_eal/rte_eal_version.map | 2 + > lib/librte_mempool/rte_mempool.h | 11 +- > 18 files changed, 349 insertions(+), 22 deletions(-) > create mode 100644 app/test/test_lcores.c > [...] > diff --git a/app/test/test_lcores.c b/app/test/test_lcores.c > new file mode 100644 > index 0000000000..864bcbade7 > --- /dev/null > +++ b/app/test/test_lcores.c > @@ -0,0 +1,139 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright (c) 2020 Red Hat, Inc. > + */ > + > +#include > +#include > + > +#include > + > +#include "test.h" > + > +struct thread_context { > + enum { INIT, ERROR, DONE } state; > + bool lcore_id_any; > + pthread_t id; > + unsigned int *registered_count; > +}; > +static void *thread_loop(void *arg) > +{ missing an empty line here > + struct thread_context *t = arg; > + unsigned int lcore_id; > + > + lcore_id = rte_lcore_id(); > + if (lcore_id != LCORE_ID_ANY) { > + printf("Incorrect lcore id for new thread %u\n", lcore_id); > + t->state = ERROR; > + } > + rte_thread_register(); > + lcore_id = rte_lcore_id(); > + if ((t->lcore_id_any && lcore_id != LCORE_ID_ANY) || > + (!t->lcore_id_any && lcore_id == LCORE_ID_ANY)) { > + printf("Could not register new thread, got %u while %sexpecting %u\n", > + lcore_id, t->lcore_id_any ? "" : "not ", LCORE_ID_ANY); > + t->state = ERROR; > + } To check if rte_thread_register() succedeed, we need to look at lcore_id. I wonder if rte_thread_register() shouldn't return the lcore id on success, and -1 on error (rte_errno could be set to give some info on the error). The same could be done for rte_thread_init() [...] > diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c > index a7ae0691bf..1cbddc4b5b 100644 > --- a/lib/librte_eal/common/eal_common_thread.c > +++ b/lib/librte_eal/common/eal_common_thread.c > @@ -236,3 +236,36 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name, > pthread_join(*thread, NULL); > return -ret; > } > + > +void > +rte_thread_register(void) > +{ > + unsigned int lcore_id; > + rte_cpuset_t cpuset; > + > + /* EAL init flushes all lcores, we can't register before. */ > + assert(internal_config.init_complete == 1); > + if (pthread_getaffinity_np(pthread_self(), sizeof(cpuset), > + &cpuset) != 0) > + CPU_ZERO(&cpuset); > + lcore_id = eal_lcore_non_eal_allocate(); > + if (lcore_id >= RTE_MAX_LCORE) > + lcore_id = LCORE_ID_ANY; > + rte_thread_init(lcore_id, &cpuset); > + if (lcore_id != LCORE_ID_ANY) > + RTE_LOG(DEBUG, EAL, "Registered non-EAL thread as lcore %u.\n", > + lcore_id); > +} So, in this case, the affinity of the pthread is kept and saved, in other words there is no link between the lcore id and the affinity. It means we are allowing an application to register lcores for dataplane with conflicting affinities. I wonder if it could be useful to have an API that automatically sets the affinity according to the lcore_id. Or a function that creates a pthread using the specified lcore id, and setting the correct affinity. I could simplify the work for applications that want to create/destroy dataplane threads dynamically. This could be done later however, just an idea. [...] > diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c > index 13e5de006f..32a3d999b8 100644 > --- a/lib/librte_eal/freebsd/eal.c > +++ b/lib/librte_eal/freebsd/eal.c > @@ -424,6 +424,10 @@ rte_config_init(void) > } > if (rte_eal_config_reattach() < 0) > return -1; > + if (!eal_mcfg_enable_multiprocess()) { > + RTE_LOG(ERR, EAL, "Primary process refused secondary attachment\n"); > + return -1; > + } > eal_mcfg_update_internal(); > break; > case RTE_PROC_AUTO: > diff --git a/lib/librte_eal/include/rte_lcore.h b/lib/librte_eal/include/rte_lcore.h > index 3968c40693..43747e88df 100644 > --- a/lib/librte_eal/include/rte_lcore.h > +++ b/lib/librte_eal/include/rte_lcore.h > @@ -31,6 +31,7 @@ enum rte_lcore_role_t { > ROLE_RTE, > ROLE_OFF, > ROLE_SERVICE, > + ROLE_NON_EAL, > }; If find the name ROLE_NON_EAL a bit heavy (this was also my impression when reading the doc part). I understand that there are several types of threads: - eal (pthread created by eal): ROLE_RTE and ROLE_SERVICE - unregistered (pthread not created by eal, and not registered): ROLE_OFF (note that ROLE_OFF also applies for unexistant threads) - dynamic: pthread not created by eal, but registered What about using ROLE_DYN ? I'm not sure about this name either, it's just to open the discussion :)