From patchwork Thu Oct 20 19:00:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Carrillo, Erik G" X-Patchwork-Id: 118837 X-Patchwork-Delegate: david.marchand@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4F31CA0552; Thu, 20 Oct 2022 21:01:10 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 38454410D1; Thu, 20 Oct 2022 21:01:10 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id 2849740FAE; Thu, 20 Oct 2022 21:01:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1666292468; x=1697828468; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=zNLx4Q78hpCALD4j6Zw9fxYx6rwakyjLKkHx1j6XjAw=; b=fU3EjYjPPwZqBOtTbJVUgprv+0MKm0uu425ty71hB/vzxZBh6ZoB1zGF BGMmEDI2tiGTBsV7mc5gG8ZXco8ImgyuitAujOZz7auZgx6HmbICu5gZy oXyRbijxazwa+SxKYqR5ZjdtUKeo35xsY1o2dCF3JZNIRzTRZqrww2e1h X8HBmlZ8+MDbGO0n7RFJXQlG/24PjqWRk4+8IcyQCGLO8oJ7GaKDbYSqu yom3joi00N3Dbn6AUt9gvRxzFN9SH7bXhUZMChzyC3SMNwbPsRi7AC1Me BZQ7P124a5NVxRFZMJAanZtf7+PIB6lCTuLe8q45gpy7eLgtfAP4LaSvv A==; X-IronPort-AV: E=McAfee;i="6500,9779,10506"; a="307908703" X-IronPort-AV: E=Sophos;i="5.95,199,1661842800"; d="scan'208";a="307908703" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Oct 2022 12:00:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10506"; a="607866648" X-IronPort-AV: E=Sophos;i="5.95,199,1661842800"; d="scan'208";a="607866648" Received: from txandevlnx321.an.intel.com ([10.123.117.43]) by orsmga006.jf.intel.com with ESMTP; 20 Oct 2022 12:00:52 -0700 From: Erik Gabriel Carrillo To: harry.van.haaren@intel.com Cc: s.v.naga.harish.k@intel.com, dev@dpdk.org, stable@dpdk.org Subject: [PATCH] service: fix early move to inactive status Date: Thu, 20 Oct 2022 14:00:41 -0500 Message-Id: <20221020190041.2350842-1-erik.g.carrillo@intel.com> X-Mailer: git-send-email 2.23.0 MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Assume thread T2 is a service lcore that is in the middle of executing a service function. Also, assume thread T1 concurrently calls rte_service_lcore_stop(), which will set the "service_active_on_lcore" state to false. If thread T1 then calls rte_service_may_be_active(), it can return zero even though T2 is still running the service function. If T1 then proceeds to free data being used by T2, a crash can ensue. Move the logic that clears the "service_active_on_lcore" state from the rte_service_lcore_stop() function to the service_runner_func() to ensure that we: - don't let the "service_active_on_lcore" state linger as 1 - don't clear the state early Fixes: 6550113be62d ("service: fix lingering active status") Cc: stable@dpdk.org Signed-off-by: Erik Gabriel Carrillo Acked-by: Harry van Haaren --- lib/eal/common/rte_service.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/eal/common/rte_service.c b/lib/eal/common/rte_service.c index 81c9514149..bcc2e19077 100644 --- a/lib/eal/common/rte_service.c +++ b/lib/eal/common/rte_service.c @@ -479,6 +479,7 @@ static int32_t service_runner_func(void *arg) { RTE_SET_USED(arg); + uint8_t i; const int lcore = rte_lcore_id(); struct core_state *cs = &lcore_states[lcore]; @@ -494,7 +495,6 @@ service_runner_func(void *arg) const uint64_t service_mask = cs->service_mask; uint8_t start_id; uint8_t end_id; - uint8_t i; if (service_mask == 0) continue; @@ -510,6 +510,12 @@ service_runner_func(void *arg) __atomic_store_n(&cs->loops, cs->loops + 1, __ATOMIC_RELAXED); } + /* Switch off this core for all services, to ensure that future + * calls to may_be_active() know this core is switched off. + */ + for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) + cs->service_active_on_lcore[i] = 0; + /* Use SEQ CST memory ordering to avoid any re-ordering around * this store, ensuring that once this store is visible, the service * lcore thread really is done in service cores code. @@ -806,11 +812,6 @@ rte_service_lcore_stop(uint32_t lcore) __atomic_load_n(&rte_services[i].num_mapped_cores, __ATOMIC_RELAXED)); - /* Switch off this core for all services, to ensure that future - * calls to may_be_active() know this core is switched off. - */ - cs->service_active_on_lcore[i] = 0; - /* if the core is mapped, and the service is running, and this * is the only core that is mapped, the service would cease to * run if this core stopped, so fail instead.