From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 0CF73A00C5;
	Mon,  6 Jul 2020 22:53:24 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 3E4E31DDAC;
	Mon,  6 Jul 2020 22:53:10 +0200 (CEST)
Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com
 [205.139.110.120]) by dpdk.org (Postfix) with ESMTP id 989A51DD75
 for <dev@dpdk.org>; Mon,  6 Jul 2020 22:53:08 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1594068788;
 h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
 to:to:cc:cc:mime-version:mime-version:content-type:content-type:
 content-transfer-encoding:content-transfer-encoding:
 in-reply-to:in-reply-to:references:references;
 bh=E54v03bb2+hpUh6ihJTcG8VCcAwR1+8C1shjD5kYdQA=;
 b=hCEOmRTR7jiFXrDa4tgQLCu6QX/JCbr/yzT7ovZ/ffdnSR7K6KhYG3dl8KpWrqWSpxYXAk
 t+0VoJfbnARORkAnrGV43pBqBqT1A0c6fmJPKJf306LKavD02215ohIAgzvExRU2avAI+a
 zr8Z1t3EeY0jRna8QWVtKzOBfkgVCc4=
Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com
 [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id
 us-mta-104-oopeFXeiNFCG8Y6bd4UmPA-1; Mon, 06 Jul 2020 16:53:04 -0400
X-MC-Unique: oopeFXeiNFCG8Y6bd4UmPA-1
Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com
 [10.5.11.22])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 53AAF19057A0;
 Mon,  6 Jul 2020 20:53:02 +0000 (UTC)
Received: from dmarchan.remote.csb (unknown [10.40.195.188])
 by smtp.corp.redhat.com (Postfix) with ESMTP id 837CE10013D7;
 Mon,  6 Jul 2020 20:52:57 +0000 (UTC)
From: David Marchand <david.marchand@redhat.com>
To: dev@dpdk.org
Cc: 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, olivier.matz@6wind.com,
 konstantin.ananyev@intel.com,
 Harini Ramakrishnan <harini.ramakrishnan@microsoft.com>,
 Omar Cardona <ocardona@microsoft.com>,
 Pallavi Kadam <pallavi.kadam@intel.com>,
 Ranjit Menon <ranjit.menon@intel.com>
Date: Mon,  6 Jul 2020 22:52:27 +0200
Message-Id: <20200706205234.8040-4-david.marchand@redhat.com>
In-Reply-To: <20200706205234.8040-1-david.marchand@redhat.com>
References: <20200610144506.30505-1-david.marchand@redhat.com>
 <20200706205234.8040-1-david.marchand@redhat.com>
MIME-Version: 1.0
X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22
Authentication-Results: relay.mimecast.com;
 auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david.marchand@redhat.com
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 8bit
Subject: [dpdk-dev] [PATCH v6 03/10] eal: introduce thread init helper
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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.

A side-effect of this patch is that control threads can now use
recursive locks (rte_gettid() was not called before).

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v4:
- renamed rte_thread_init as __rte_thread_init and moved to
  eal_private.h,

Changes since v1:
- rebased on master, removed Windows workarounds wrt gettid and traces
  support,

---
 lib/librte_eal/common/eal_common_thread.c | 50 ++++++++++++++---------
 lib/librte_eal/common/eal_private.h       | 10 +++++
 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 +----
 9 files changed, 71 insertions(+), 103 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index fd13453fee..fb06f8f802 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();
+
+	thread_update_affinity(cpuset);
+
+	__rte_trace_mem_per_thread_alloc();
+}
 
 struct rte_thread_ctrl_params {
 	void *(*start_routine)(void *);
@@ -154,7 +167,7 @@ struct rte_thread_ctrl_params {
 	pthread_barrier_t configured;
 };
 
-static void *rte_thread_init(void *arg)
+static void *ctrl_thread_init(void *arg)
 {
 	int ret;
 	struct internal_config *internal_conf =
@@ -164,8 +177,7 @@ static void *rte_thread_init(void *arg)
 	void *(*start_routine)(void *) = params->start_routine;
 	void *routine_arg = params->arg;
 
-	/* Store cpuset in TLS for quick access */
-	memmove(&RTE_PER_LCORE(_cpuset), cpuset, sizeof(rte_cpuset_t));
+	__rte_thread_init(rte_lcore_id(), cpuset);
 
 	ret = pthread_barrier_wait(&params->configured);
 	if (ret == PTHREAD_BARRIER_SERIAL_THREAD) {
@@ -173,8 +185,6 @@ static void *rte_thread_init(void *arg)
 		free(params);
 	}
 
-	__rte_trace_mem_per_thread_alloc();
-
 	return start_routine(routine_arg);
 }
 
@@ -198,7 +208,7 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
 
 	pthread_barrier_init(&params->configured, NULL, 2);
 
-	ret = pthread_create(thread, attr, rte_thread_init, (void *)params);
+	ret = pthread_create(thread, attr, ctrl_thread_init, (void *)params);
 	if (ret != 0) {
 		free(params);
 		return -ret;
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 46bcae9305..5d8b53882d 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -699,4 +699,14 @@ eal_get_internal_configuration(void);
 rte_usage_hook_t
 eal_get_application_usage_hook(void);
 
+/**
+ * Init per-lcore info in current thread.
+ *
+ * @param lcore_id
+ *   identifier of lcore.
+ * @param cpuset
+ *   CPU affinity for this thread.
+ */
+void __rte_thread_init(unsigned int lcore_id, rte_cpuset_t *cpuset);
+
 #endif /* _EAL_PRIVATE_H_ */
diff --git a/lib/librte_eal/common/eal_thread.h b/lib/librte_eal/common/eal_thread.h
index b40ed249ed..dc1fc6eb99 100644
--- a/lib/librte_eal/common/eal_thread.h
+++ b/lib/librte_eal/common/eal_thread.h
@@ -15,14 +15,6 @@
  */
 __rte_noreturn void *eal_thread_loop(void *arg);
 
-/**
- * Init per-lcore info for master thread
- *
- * @param lcore_id
- *   identifier of master lcore
- */
-void eal_thread_init_master(unsigned lcore_id);
-
 /**
  * Get the NUMA socket id from cpu id.
  * This function is private to EAL.
diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 8c75cba79a..fd577daf44 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -845,7 +845,14 @@ rte_eal_init(int argc, char **argv)
 
 	eal_check_mem_on_local_socket();
 
-	eal_thread_init_master(config->master_lcore);
+	if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
+			&lcore_config[config->master_lcore].cpuset) != 0) {
+		rte_eal_init_alert("Cannot set affinity");
+		rte_errno = EINVAL;
+		return -1;
+	}
+	__rte_thread_init(config->master_lcore,
+		&lcore_config[config->master_lcore].cpuset);
 
 	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
 
@@ -876,6 +883,11 @@ rte_eal_init(int argc, char **argv)
 		snprintf(thread_name, sizeof(thread_name),
 				"lcore-slave-%d", i);
 		rte_thread_setname(lcore_config[i].thread_id, thread_name);
+
+		ret = pthread_setaffinity_np(lcore_config[i].thread_id,
+			sizeof(rte_cpuset_t), &lcore_config[i].cpuset);
+		if (ret != 0)
+			rte_panic("Cannot set affinity\n");
 	}
 
 	/*
diff --git a/lib/librte_eal/freebsd/eal_thread.c b/lib/librte_eal/freebsd/eal_thread.c
index 40676d9ef5..0788a54fe6 100644
--- a/lib/librte_eal/freebsd/eal_thread.c
+++ b/lib/librte_eal/freebsd/eal_thread.c
@@ -66,29 +66,6 @@ rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned slave_id)
 	return rc;
 }
 
-/* set affinity for current thread */
-static int
-eal_thread_set_affinity(void)
-{
-	unsigned lcore_id = rte_lcore_id();
-
-	/* acquire system unique id  */
-	rte_gettid();
-
-	/* update EAL thread core affinity */
-	return rte_thread_set_affinity(&lcore_config[lcore_id].cpuset);
-}
-
-void eal_thread_init_master(unsigned lcore_id)
-{
-	/* set the lcore ID in per-lcore memory area */
-	RTE_PER_LCORE(_lcore_id) = lcore_id;
-
-	/* set CPU affinity */
-	if (eal_thread_set_affinity() < 0)
-		rte_panic("cannot set affinity\n");
-}
-
 /* main loop of threads */
 __rte_noreturn void *
 eal_thread_loop(__rte_unused void *arg)
@@ -113,19 +90,12 @@ eal_thread_loop(__rte_unused void *arg)
 	m2s = lcore_config[lcore_id].pipe_master2slave[0];
 	s2m = lcore_config[lcore_id].pipe_slave2master[1];
 
-	/* set the lcore ID in per-lcore memory area */
-	RTE_PER_LCORE(_lcore_id) = lcore_id;
-
-	/* set CPU affinity */
-	if (eal_thread_set_affinity() < 0)
-		rte_panic("cannot set affinity\n");
+	__rte_thread_init(lcore_id, &lcore_config[lcore_id].cpuset);
 
 	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
-
 	RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%p;cpuset=[%s%s])\n",
 		lcore_id, thread_id, cpuset, ret == 0 ? "" : "...");
 
-	__rte_trace_mem_per_thread_alloc();
 	rte_eal_trace_thread_lcore_ready(lcore_id, cpuset);
 
 	/* read on our pipe to get commands */
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 3b56d14da1..bd089cdd44 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1184,10 +1184,16 @@ rte_eal_init(int argc, char **argv)
 
 	eal_check_mem_on_local_socket();
 
-	eal_thread_init_master(config->master_lcore);
+	if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
+			&lcore_config[config->master_lcore].cpuset) != 0) {
+		rte_eal_init_alert("Cannot set affinity");
+		rte_errno = EINVAL;
+		return -1;
+	}
+	__rte_thread_init(config->master_lcore,
+		&lcore_config[config->master_lcore].cpuset);
 
 	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
-
 	RTE_LOG(DEBUG, EAL, "Master lcore %u is ready (tid=%zx;cpuset=[%s%s])\n",
 		config->master_lcore, (uintptr_t)thread_id, cpuset,
 		ret == 0 ? "" : "...");
@@ -1219,6 +1225,11 @@ rte_eal_init(int argc, char **argv)
 		if (ret != 0)
 			RTE_LOG(DEBUG, EAL,
 				"Cannot set name for lcore thread\n");
+
+		ret = pthread_setaffinity_np(lcore_config[i].thread_id,
+			sizeof(rte_cpuset_t), &lcore_config[i].cpuset);
+		if (ret != 0)
+			rte_panic("Cannot set affinity\n");
 	}
 
 	/*
diff --git a/lib/librte_eal/linux/eal_thread.c b/lib/librte_eal/linux/eal_thread.c
index a52ebef3a4..e0440c0000 100644
--- a/lib/librte_eal/linux/eal_thread.c
+++ b/lib/librte_eal/linux/eal_thread.c
@@ -66,29 +66,6 @@ rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned slave_id)
 	return rc;
 }
 
-/* set affinity for current EAL thread */
-static int
-eal_thread_set_affinity(void)
-{
-	unsigned lcore_id = rte_lcore_id();
-
-	/* acquire system unique id  */
-	rte_gettid();
-
-	/* update EAL thread core affinity */
-	return rte_thread_set_affinity(&lcore_config[lcore_id].cpuset);
-}
-
-void eal_thread_init_master(unsigned lcore_id)
-{
-	/* set the lcore ID in per-lcore memory area */
-	RTE_PER_LCORE(_lcore_id) = lcore_id;
-
-	/* set CPU affinity */
-	if (eal_thread_set_affinity() < 0)
-		rte_panic("cannot set affinity\n");
-}
-
 /* main loop of threads */
 __rte_noreturn void *
 eal_thread_loop(__rte_unused void *arg)
@@ -113,19 +90,12 @@ eal_thread_loop(__rte_unused void *arg)
 	m2s = lcore_config[lcore_id].pipe_master2slave[0];
 	s2m = lcore_config[lcore_id].pipe_slave2master[1];
 
-	/* set the lcore ID in per-lcore memory area */
-	RTE_PER_LCORE(_lcore_id) = lcore_id;
-
-	/* set CPU affinity */
-	if (eal_thread_set_affinity() < 0)
-		rte_panic("cannot set affinity\n");
+	__rte_thread_init(lcore_id, &lcore_config[lcore_id].cpuset);
 
 	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
-
 	RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%zx;cpuset=[%s%s])\n",
 		lcore_id, (uintptr_t)thread_id, cpuset, ret == 0 ? "" : "...");
 
-	__rte_trace_mem_per_thread_alloc();
 	rte_eal_trace_thread_lcore_ready(lcore_id, cpuset);
 
 	/* read on our pipe to get commands */
diff --git a/lib/librte_eal/windows/eal.c b/lib/librte_eal/windows/eal.c
index eb10b4ef96..9f5d019e64 100644
--- a/lib/librte_eal/windows/eal.c
+++ b/lib/librte_eal/windows/eal.c
@@ -333,7 +333,8 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
-	eal_thread_init_master(config->master_lcore);
+	__rte_thread_init(config->master_lcore,
+		&lcore_config[config->master_lcore].cpuset);
 
 	bscan = rte_bus_scan();
 	if (bscan < 0) {
diff --git a/lib/librte_eal/windows/eal_thread.c b/lib/librte_eal/windows/eal_thread.c
index f12a2ec6ad..20889b6196 100644
--- a/lib/librte_eal/windows/eal_thread.c
+++ b/lib/librte_eal/windows/eal_thread.c
@@ -53,13 +53,6 @@ rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int slave_id)
 	return 0;
 }
 
-void
-eal_thread_init_master(unsigned int lcore_id)
-{
-	/* set the lcore ID in per-lcore memory area */
-	RTE_PER_LCORE(_lcore_id) = lcore_id;
-}
-
 /* main loop of threads */
 void *
 eal_thread_loop(void *arg __rte_unused)
@@ -84,8 +77,7 @@ eal_thread_loop(void *arg __rte_unused)
 	m2s = lcore_config[lcore_id].pipe_master2slave[0];
 	s2m = lcore_config[lcore_id].pipe_slave2master[1];
 
-	/* set the lcore ID in per-lcore memory area */
-	RTE_PER_LCORE(_lcore_id) = lcore_id;
+	__rte_thread_init(lcore_id, &lcore_config[lcore_id].cpuset);
 
 	RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%zx;cpuset=[%s])\n",
 		lcore_id, (uintptr_t)thread_id, cpuset);
-- 
2.23.0