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 24A71A0350; Tue, 30 Jun 2020 11:37:44 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id F33191BEA8; Tue, 30 Jun 2020 11:37:42 +0200 (CEST) Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by dpdk.org (Postfix) with ESMTP id 0BEF81BEA3 for ; Tue, 30 Jun 2020 11:37:41 +0200 (CEST) Received: by mail-wr1-f67.google.com with SMTP id f18so11354733wrs.0 for ; Tue, 30 Jun 2020 02:37:41 -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=0ovZqW9O5dLyiYVn97jJHCXO/L4aV7jj3PW/Syod10I=; b=X2HDJu5GtjNWdkN4tfFby82m+dCcti4851+BVfJ9imD+0I6DhopWCmKZmkwJKvh01b MHY0OLnQ5SviGlwmTNmiXjlC1dG6d+4JeTY3YpjLhW+da4QubOdYayyTXhGL9fkssfm7 ejlDwbpVElkKRf9mfWn4H8/2g9Dwf4jynPgPaalZs1l8PhJbsC9rKRXK8gMJ0+QYtPBy bgc9HMfrsUhi1FsHtt1cZkGea+FQ4+Eo9e0yy4YJHGMqbwcQG+9WWnSbmIO1QhvZh/kH Rwp42upgyA2r4N8GnErNdVoIsybACr2nZBk3152+ch06DA9to+spx5Swhf+PHE/YrwRO EfRg== 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=0ovZqW9O5dLyiYVn97jJHCXO/L4aV7jj3PW/Syod10I=; b=JR/k0/tUMQMVoCOJaFfSjhn6Z0TzKPSNAnM5SwZ3wSGDJloC/bYglh1GfC/GAZCRHB Jay2praeK0FgW0bw0gIZeZJ9JjNl0I4/4HbT6R87RoK0gdhRNQZcM/foBXel+Q2Hry6A CG0TNZmXABmxDuOMSbbFkiHT1h707LawBsBjZAPgnKvvil70OTG9sa9gRc7dg7lkpQUC jmTerFtPkjPEmhnUaZbHeTIN7bzK+2EnCdSWZV7e0ApIG37r69l4PcEzbG+SaAhkxQ5t Yk40wAMyhAy5rd+QhQRIndJHKAXEKFZRLTZRDN+CMB5lf6ZryPCY0E2wQeBSAWZjg87D oG9g== X-Gm-Message-State: AOAM531dvTq4a1jzL+y6Vtc0gb60ysCjjtkZHXJSynLQShudjlSZRBFc Lp+X42WsgU13YLQxs0/wlXghHw== X-Google-Smtp-Source: ABdhPJzTjNxgDll4ORP6X2Xhr/Eeo0SaMp2CqvQiYjopVLVQTpYzXFNreOzTJKNdlDSzcf/BxE/RPg== X-Received: by 2002:adf:e3c5:: with SMTP id k5mr21146168wrm.121.1593509860757; Tue, 30 Jun 2020 02:37:40 -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 b17sm2930385wrp.32.2020.06.30.02.37.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Jun 2020 02:37:40 -0700 (PDT) Date: Tue, 30 Jun 2020 11:37:39 +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, Harini Ramakrishnan , Omar Cardona , Pallavi Kadam , Ranjit Menon Message-ID: <20200630093739.GC5869@platinum> References: <20200610144506.30505-1-david.marchand@redhat.com> <20200626144736.11011-1-david.marchand@redhat.com> <20200626144736.11011-4-david.marchand@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200626144736.11011-4-david.marchand@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH v4 3/9] eal: introduce thread init helper 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" Hi David, one minor comment below On Fri, Jun 26, 2020 at 04:47:30PM +0200, David Marchand wrote: > Introduce a helper responsible for initialising the per thread context. > We can then have a unified context for EAL and non-EAL threads and > remove copy/paste'd OS-specific helpers. > > Per EAL thread CPU affinity setting is separated from the thread init. > It is to accommodate with Windows EAL where CPU affinity is not set at > the moment. > Besides, having affinity set by the master lcore in FreeBSD and Linux > will make it possible to detect errors rather than panic in the child > thread. But the cleanup when such an event happens is left for later. > > Signed-off-by: David Marchand > --- > Changes since v1: > - rebased on master, removed Windows workarounds wrt gettid and traces > support, > > --- > lib/librte_eal/common/eal_common_thread.c | 51 +++++++++++++---------- > lib/librte_eal/common/eal_thread.h | 8 ++-- > lib/librte_eal/freebsd/eal.c | 14 ++++++- > lib/librte_eal/freebsd/eal_thread.c | 32 +------------- > lib/librte_eal/linux/eal.c | 15 ++++++- > lib/librte_eal/linux/eal_thread.c | 32 +------------- > lib/librte_eal/windows/eal.c | 3 +- > lib/librte_eal/windows/eal_thread.c | 10 +---- > 8 files changed, 66 insertions(+), 99 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c > index 280c64bb76..afb30236c5 100644 > --- a/lib/librte_eal/common/eal_common_thread.c > +++ b/lib/librte_eal/common/eal_common_thread.c > @@ -71,20 +71,10 @@ eal_cpuset_socket_id(rte_cpuset_t *cpusetp) > return socket_id; > } > > -int > -rte_thread_set_affinity(rte_cpuset_t *cpusetp) > +static void > +thread_update_affinity(rte_cpuset_t *cpusetp) > { > - int s; > - unsigned lcore_id; > - pthread_t tid; > - > - tid = pthread_self(); > - > - s = pthread_setaffinity_np(tid, sizeof(rte_cpuset_t), cpusetp); > - if (s != 0) { > - RTE_LOG(ERR, EAL, "pthread_setaffinity_np failed\n"); > - return -1; > - } > + unsigned int lcore_id = rte_lcore_id(); > > /* store socket_id in TLS for quick access */ > RTE_PER_LCORE(_socket_id) = > @@ -94,14 +84,24 @@ rte_thread_set_affinity(rte_cpuset_t *cpusetp) > memmove(&RTE_PER_LCORE(_cpuset), cpusetp, > sizeof(rte_cpuset_t)); > > - lcore_id = rte_lcore_id(); > if (lcore_id != (unsigned)LCORE_ID_ANY) { > /* EAL thread will update lcore_config */ > lcore_config[lcore_id].socket_id = RTE_PER_LCORE(_socket_id); > memmove(&lcore_config[lcore_id].cpuset, cpusetp, > sizeof(rte_cpuset_t)); > } > +} > > +int > +rte_thread_set_affinity(rte_cpuset_t *cpusetp) > +{ > + if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t), > + cpusetp) != 0) { > + RTE_LOG(ERR, EAL, "pthread_setaffinity_np failed\n"); > + return -1; > + } > + > + thread_update_affinity(cpusetp); > return 0; > } > > @@ -147,6 +147,19 @@ eal_thread_dump_affinity(char *str, unsigned size) > return ret; > } > > +void > +rte_thread_init(unsigned int lcore_id, rte_cpuset_t *cpuset) > +{ > + /* set the lcore ID in per-lcore memory area */ > + RTE_PER_LCORE(_lcore_id) = lcore_id; > + > + /* acquire system unique id */ > + rte_gettid(); If I understand properly, rte_gettid() is now also called for control thread. I don't think this behavior change can break anything, but it may be good to highlight it in the commit log. also, there are 2 spaces before "*/"