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 8D229A00C5; Mon, 6 Jul 2020 22:54:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5064C1DDDB; Mon, 6 Jul 2020 22:53:32 +0200 (CEST) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by dpdk.org (Postfix) with ESMTP id B92181DDD3 for ; Mon, 6 Jul 2020 22:53:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594068810; 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=FMrx449Dp+quLDBDMJF8y8dcyqGH2sxdRWZlr/F+WV0=; b=FTiNu3vH/jm9M6m5zQSGYhP9202pyOi+ftGpsYZTYJVJ6QmR3xthtGbBZbITZIqKjjdL3C AFg56l8LVAjpqL9HcBoT7DXsL14uNopgl7DN1Tq+LKFJ+uz9Xg4Ipb0ybp+9ucJYqkrNsP mBwtZewGQF4DaW3X3SYzwsRBMOXV3a0= 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-23-ELdl3-PyOSmmDV87xG-JWw-1; Mon, 06 Jul 2020 16:53:28 -0400 X-MC-Unique: ELdl3-PyOSmmDV87xG-JWw-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 2099F8014D4; Mon, 6 Jul 2020 20:53:26 +0000 (UTC) Received: from dmarchan.remote.csb (unknown [10.40.195.188]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1BEEC10013D7; Mon, 6 Jul 2020 20:53:21 +0000 (UTC) From: David Marchand 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, Neil Horman Date: Mon, 6 Jul 2020 22:52:31 +0200 Message-Id: <20200706205234.8040-8-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 07/10] eal: add lcore init callbacks 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" DPDK components and applications can have their say when a new lcore is initialized. For this, they can register a callback for initializing and releasing their private data. Signed-off-by: David Marchand --- Changes since v4: - fixed leak on callback register failure, - fixed nits from Konstantin and Olivier, - prefixed unit tests logs with Error: when applicable, Changes since v2: - added missing test, - fixed rollback on lcore register, Changes since v1: - added unit test (since missing some coverage, for v3), - preferred callback and removed mention of notification, --- app/test/test_lcores.c | 227 +++++++++++++++++++++++ lib/librte_eal/common/eal_common_lcore.c | 146 ++++++++++++++- lib/librte_eal/common/eal_private.h | 3 +- lib/librte_eal/include/rte_lcore.h | 70 +++++++ lib/librte_eal/rte_eal_version.map | 2 + 5 files changed, 442 insertions(+), 6 deletions(-) diff --git a/app/test/test_lcores.c b/app/test/test_lcores.c index afb9cdd444..7df827b4e8 100644 --- a/app/test/test_lcores.c +++ b/app/test/test_lcores.c @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -117,6 +118,226 @@ test_non_eal_lcores(unsigned int eal_threads_count) return ret; } +struct limit_lcore_context { + unsigned int init; + unsigned int max; + unsigned int uninit; +}; + +static int +limit_lcores_init(unsigned int lcore_id __rte_unused, void *arg) +{ + struct limit_lcore_context *l = arg; + + l->init++; + if (l->init > l->max) + return -1; + return 0; +} + +static void +limit_lcores_uninit(unsigned int lcore_id __rte_unused, void *arg) +{ + struct limit_lcore_context *l = arg; + + l->uninit++; +} + +static int +test_lcores_callback(unsigned int eal_threads_count) +{ + struct limit_lcore_context l; + void *handle; + + /* Refuse last lcore => callback register error. */ + memset(&l, 0, sizeof(l)); + l.max = eal_threads_count - 1; + handle = rte_lcore_callback_register("limit", limit_lcores_init, + limit_lcores_uninit, &l); + if (handle != NULL) { + printf("Error: lcore callback register should have failed\n"); + goto error; + } + /* Refusal happens at the n th call to the init callback. + * Besides, n - 1 were accepted, so we expect as many uninit calls when + * the rollback happens. + */ + if (l.init != eal_threads_count) { + printf("Error: lcore callback register failed but incorrect init calls, expected %u, got %u\n", + eal_threads_count, l.init); + goto error; + } + if (l.uninit != eal_threads_count - 1) { + printf("Error: lcore callback register failed but incorrect uninit calls, expected %u, got %u\n", + eal_threads_count - 1, l.uninit); + goto error; + } + + /* Accept all lcore and unregister. */ + memset(&l, 0, sizeof(l)); + l.max = eal_threads_count; + handle = rte_lcore_callback_register("limit", limit_lcores_init, + limit_lcores_uninit, &l); + if (handle == NULL) { + printf("Error: lcore callback register failed\n"); + goto error; + } + if (l.uninit != 0) { + printf("Error: lcore callback register succeeded but incorrect uninit calls, expected 0, got %u\n", + l.uninit); + goto error; + } + rte_lcore_callback_unregister(handle); + handle = NULL; + if (l.init != eal_threads_count) { + printf("Error: lcore callback unregister done but incorrect init calls, expected %u, got %u\n", + eal_threads_count, l.init); + goto error; + } + if (l.uninit != eal_threads_count) { + printf("Error: lcore callback unregister done but incorrect uninit calls, expected %u, got %u\n", + eal_threads_count, l.uninit); + goto error; + } + + return 0; + +error: + if (handle != NULL) + rte_lcore_callback_unregister(handle); + + return -1; +} + +static int +test_non_eal_lcores_callback(unsigned int eal_threads_count) +{ + struct thread_context thread_contexts[2]; + unsigned int non_eal_threads_count = 0; + struct limit_lcore_context l[2] = {}; + unsigned int registered_count = 0; + struct thread_context *t; + void *handle[2] = {}; + unsigned int i; + int ret; + + /* This test requires two empty slots to be sure lcore init refusal is + * because of callback execution. + */ + if (eal_threads_count + 2 >= RTE_MAX_LCORE) + return 0; + + /* Register two callbacks: + * - first one accepts any lcore, + * - second one accepts all EAL lcore + one more for the first non-EAL + * thread, then refuses the next lcore. + */ + l[0].max = UINT_MAX; + handle[0] = rte_lcore_callback_register("no_limit", limit_lcores_init, + limit_lcores_uninit, &l[0]); + if (handle[0] == NULL) { + printf("Error: lcore callback [0] register failed\n"); + goto error; + } + l[1].max = eal_threads_count + 1; + handle[1] = rte_lcore_callback_register("limit", limit_lcores_init, + limit_lcores_uninit, &l[1]); + if (handle[1] == NULL) { + printf("Error: lcore callback [1] register failed\n"); + goto error; + } + if (l[0].init != eal_threads_count || l[1].init != eal_threads_count) { + printf("Error: lcore callbacks register succeeded but incorrect init calls, expected %u, %u, got %u, %u\n", + eal_threads_count, eal_threads_count, + l[0].init, l[1].init); + goto error; + } + if (l[0].uninit != 0 || l[1].uninit != 0) { + printf("Error: lcore callbacks register succeeded but incorrect uninit calls, expected 0, 1, got %u, %u\n", + l[0].uninit, l[1].uninit); + goto error; + } + /* First thread that expects a valid lcore id. */ + t = &thread_contexts[0]; + t->state = INIT; + t->registered_count = ®istered_count; + t->lcore_id_any = false; + if (pthread_create(&t->id, NULL, thread_loop, t) != 0) + goto cleanup_threads; + non_eal_threads_count++; + while (__atomic_load_n(®istered_count, __ATOMIC_ACQUIRE) != + non_eal_threads_count) + ; + if (l[0].init != eal_threads_count + 1 || + l[1].init != eal_threads_count + 1) { + printf("Error: incorrect init calls, expected %u, %u, got %u, %u\n", + eal_threads_count + 1, eal_threads_count + 1, + l[0].init, l[1].init); + goto cleanup_threads; + } + if (l[0].uninit != 0 || l[1].uninit != 0) { + printf("Error: incorrect uninit calls, expected 0, 0, got %u, %u\n", + l[0].uninit, l[1].uninit); + goto cleanup_threads; + } + /* Second thread, that expects LCORE_ID_ANY because of init refusal. */ + t = &thread_contexts[1]; + t->state = INIT; + t->registered_count = ®istered_count; + t->lcore_id_any = true; + if (pthread_create(&t->id, NULL, thread_loop, t) != 0) + goto cleanup_threads; + non_eal_threads_count++; + while (__atomic_load_n(®istered_count, __ATOMIC_ACQUIRE) != + non_eal_threads_count) + ; + if (l[0].init != eal_threads_count + 2 || + l[1].init != eal_threads_count + 2) { + printf("Error: incorrect init calls, expected %u, %u, got %u, %u\n", + eal_threads_count + 2, eal_threads_count + 2, + l[0].init, l[1].init); + goto cleanup_threads; + } + if (l[0].uninit != 1 || l[1].uninit != 0) { + printf("Error: incorrect uninit calls, expected 1, 0, got %u, %u\n", + l[0].uninit, l[1].uninit); + goto cleanup_threads; + } + /* Release all threads, and check their states. */ + __atomic_store_n(®istered_count, 0, __ATOMIC_RELEASE); + ret = 0; + for (i = 0; i < non_eal_threads_count; i++) { + t = &thread_contexts[i]; + pthread_join(t->id, NULL); + if (t->state != DONE) + ret = -1; + } + if (ret < 0) + goto error; + if (l[0].uninit != 2 || l[1].uninit != 1) { + printf("Error: threads reported having successfully registered and unregistered, but incorrect uninit calls, expected 2, 1, got %u, %u\n", + l[0].uninit, l[1].uninit); + goto error; + } + rte_lcore_callback_unregister(handle[0]); + rte_lcore_callback_unregister(handle[1]); + return 0; + +cleanup_threads: + /* Release all threads */ + __atomic_store_n(®istered_count, 0, __ATOMIC_RELEASE); + for (i = 0; i < non_eal_threads_count; i++) { + t = &thread_contexts[i]; + pthread_join(t->id, NULL); + } +error: + if (handle[1] != NULL) + rte_lcore_callback_unregister(handle[1]); + if (handle[0] != NULL) + rte_lcore_callback_unregister(handle[0]); + return -1; +} + static int test_lcores(void) { @@ -137,6 +358,12 @@ test_lcores(void) if (test_non_eal_lcores(eal_threads_count) < 0) return TEST_FAILED; + if (test_lcores_callback(eal_threads_count) < 0) + return TEST_FAILED; + + if (test_non_eal_lcores_callback(eal_threads_count) < 0) + return TEST_FAILED; + return TEST_SUCCESS; } diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c index 2b7d262372..90139c77ff 100644 --- a/lib/librte_eal/common/eal_common_lcore.c +++ b/lib/librte_eal/common/eal_common_lcore.c @@ -224,11 +224,122 @@ rte_socket_id_by_idx(unsigned int idx) } static rte_spinlock_t lcore_lock = RTE_SPINLOCK_INITIALIZER; +struct lcore_callback { + TAILQ_ENTRY(lcore_callback) next; + char *name; + rte_lcore_init_cb init; + rte_lcore_uninit_cb uninit; + void *arg; +}; +static TAILQ_HEAD(lcore_callbacks_head, lcore_callback) lcore_callbacks = + TAILQ_HEAD_INITIALIZER(lcore_callbacks); + +static int +callback_init(struct lcore_callback *callback, unsigned int lcore_id) +{ + if (callback->init == NULL) + return 0; + RTE_LOG(DEBUG, EAL, "Call init for lcore callback %s, lcore_id %u\n", + callback->name, lcore_id); + return callback->init(lcore_id, callback->arg); +} + +static void +callback_uninit(struct lcore_callback *callback, unsigned int lcore_id) +{ + if (callback->uninit == NULL) + return; + RTE_LOG(DEBUG, EAL, "Call uninit for lcore callback %s, lcore_id %u\n", + callback->name, lcore_id); + callback->uninit(lcore_id, callback->arg); +} + +static void +free_callback(struct lcore_callback *callback) +{ + free(callback->name); + free(callback); +} + +void * +rte_lcore_callback_register(const char *name, rte_lcore_init_cb init, + rte_lcore_uninit_cb uninit, void *arg) +{ + struct rte_config *cfg = rte_eal_get_configuration(); + struct lcore_callback *callback; + unsigned int lcore_id; + + if (name == NULL) + return NULL; + callback = calloc(1, sizeof(*callback)); + if (callback == NULL) + return NULL; + if (asprintf(&callback->name, "%s-%p", name, arg) == -1) { + free(callback); + return NULL; + } + callback->init = init; + callback->uninit = uninit; + callback->arg = arg; + rte_spinlock_lock(&lcore_lock); + if (callback->init == NULL) + goto no_init; + for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) { + if (cfg->lcore_role[lcore_id] == ROLE_OFF) + continue; + if (callback_init(callback, lcore_id) == 0) + continue; + /* Callback refused init for this lcore, uninitialize all + * previous lcore. + */ + while (lcore_id-- != 0) { + if (cfg->lcore_role[lcore_id] == ROLE_OFF) + continue; + callback_uninit(callback, lcore_id); + } + free_callback(callback); + callback = NULL; + goto out; + } +no_init: + TAILQ_INSERT_TAIL(&lcore_callbacks, callback, next); + RTE_LOG(DEBUG, EAL, "Registered new lcore callback %s (%sinit, %suninit).\n", + callback->name, callback->init == NULL ? "NO " : "", + callback->uninit == NULL ? "NO " : ""); +out: + rte_spinlock_unlock(&lcore_lock); + return callback; +} + +void +rte_lcore_callback_unregister(void *handle) +{ + struct rte_config *cfg = rte_eal_get_configuration(); + struct lcore_callback *callback = handle; + unsigned int lcore_id; + + rte_spinlock_lock(&lcore_lock); + if (callback->uninit == NULL) + goto no_uninit; + for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) { + if (cfg->lcore_role[lcore_id] == ROLE_OFF) + continue; + callback_uninit(callback, lcore_id); + } +no_uninit: + TAILQ_REMOVE(&lcore_callbacks, callback, next); + rte_spinlock_unlock(&lcore_lock); + RTE_LOG(DEBUG, EAL, "Unregistered lcore callback %s-%p.\n", + callback->name, callback->arg); + free_callback(callback); +} unsigned int eal_lcore_non_eal_allocate(void) { struct rte_config *cfg = rte_eal_get_configuration(); + struct lcore_callback *callback; + struct lcore_callback *prev; unsigned int lcore_id; rte_spinlock_lock(&lcore_lock); @@ -239,8 +350,29 @@ eal_lcore_non_eal_allocate(void) cfg->lcore_count++; break; } - if (lcore_id == RTE_MAX_LCORE) + if (lcore_id == RTE_MAX_LCORE) { RTE_LOG(DEBUG, EAL, "No lcore available.\n"); + goto out; + } + TAILQ_FOREACH(callback, &lcore_callbacks, next) { + if (callback_init(callback, lcore_id) == 0) + continue; + /* Callback refused init for this lcore, call uninit for all + * previous callbacks. + */ + prev = TAILQ_PREV(callback, lcore_callbacks_head, next); + while (prev != NULL) { + callback_uninit(prev, lcore_id); + prev = TAILQ_PREV(prev, lcore_callbacks_head, next); + } + RTE_LOG(DEBUG, EAL, "Initialization refused for lcore %u.\n", + lcore_id); + cfg->lcore_role[lcore_id] = ROLE_OFF; + cfg->lcore_count--; + lcore_id = RTE_MAX_LCORE; + goto out; + } +out: rte_spinlock_unlock(&lcore_lock); return lcore_id; } @@ -249,11 +381,15 @@ void eal_lcore_non_eal_release(unsigned int lcore_id) { struct rte_config *cfg = rte_eal_get_configuration(); + struct lcore_callback *callback; rte_spinlock_lock(&lcore_lock); - if (cfg->lcore_role[lcore_id] == ROLE_NON_EAL) { - cfg->lcore_role[lcore_id] = ROLE_OFF; - cfg->lcore_count--; - } + if (cfg->lcore_role[lcore_id] != ROLE_NON_EAL) + goto out; + TAILQ_FOREACH(callback, &lcore_callbacks, next) + callback_uninit(callback, lcore_id); + cfg->lcore_role[lcore_id] = ROLE_OFF; + cfg->lcore_count--; +out: rte_spinlock_unlock(&lcore_lock); } diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h index e82fb80aa0..535e008474 100644 --- a/lib/librte_eal/common/eal_private.h +++ b/lib/librte_eal/common/eal_private.h @@ -401,7 +401,8 @@ uint64_t get_tsc_freq_arch(void); * * @return * - the id of a lcore with role ROLE_NON_EAL on success. - * - RTE_MAX_LCORE if none was available. + * - RTE_MAX_LCORE if none was available or initializing was refused (see + * rte_lcore_callback_register). */ unsigned int eal_lcore_non_eal_allocate(void); diff --git a/lib/librte_eal/include/rte_lcore.h b/lib/librte_eal/include/rte_lcore.h index 2fd1a03275..6e7206c79f 100644 --- a/lib/librte_eal/include/rte_lcore.h +++ b/lib/librte_eal/include/rte_lcore.h @@ -229,6 +229,76 @@ unsigned int rte_get_next_lcore(unsigned int i, int skip_master, int wrap); i