From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id AB73C46075; Tue, 14 Jan 2025 17:26:28 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 998E7402E4; Tue, 14 Jan 2025 17:26:28 +0100 (CET) Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) by mails.dpdk.org (Postfix) with ESMTP id D527C40298 for ; Tue, 14 Jan 2025 17:26:26 +0100 (CET) Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-2156e078563so85285665ad.2 for ; Tue, 14 Jan 2025 08:26:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1736871986; x=1737476786; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=A6pgSNtrxRGS4orQU5SGkAW/fyYUR4yIoknbvidHOhM=; b=1GpNPtIsBm4l2FrTvptgddcGwQ9/Q12bwrs3bFH2Cl7XEsPY/YHH1yRdQTqrrxD2Si LEbTT1Yc7Vk3YgzGLsfn2FSqzmN7KgMqY7CihrGBzDO39mhtNGTk2WIg2sWMp7PPvggr 4zRHSHrEwD1ZVT8PQ5tV/EqIoHS+KTjODn/agyOr+eoFkjVBKe1E9sZlTqVVmMtOoPB8 EFfbSLy79BZIZQ/9l0LInGGPde1LqHNuXUcYDcnJAM78Ilo8ngRvsw40y0KDshgG9LGp ZUGewhvEkKmFSXnghWntWSYV4cpU5y9MgUeVERbdQH0cN/JFjcq2AGmADCj6mvogFPvc FzUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736871986; x=1737476786; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=A6pgSNtrxRGS4orQU5SGkAW/fyYUR4yIoknbvidHOhM=; b=amxToyotEz8vWcIn0+0Lu885jxHf8GGW+kHRHv1Fu0tNwYsvhqdh1cUsFNi5LKTj7G JTLb1pYbP4/PTvxtDccJIg6mRgTtVZnjnxGDsS+WOirlXiWjf0Ttzr7HD+wFplECsTdK b/H84w4jb9ol+kgpYTELLlJhua6Rdb+ln7H5eQIy+en6l4sI8SJ/y4yGEFB0rd49gcm/ X87nNS1Ol3Tq0zj2pf5ldpyk+buwMc+M0NXRfOgV0AYpxVUJd/1hCZ4v08Du+zbHp1Tw kYrYupEw1TcntEilKAPNvFvppxs9ysECv2oe4SG+QK3ukfUUCHfAGP2LR0rbpb9eBLKM LaRQ== X-Gm-Message-State: AOJu0YwWGh4m8x2OyfAAM9c8vb67hGpDkPgBNv+u1UKUeoTgBGU98fSs 7vgpt3qoyK6cFOWnEx/Yu/Dny8PC5Yy+7oIGA6jtaZJHRmxdf9xbhWTJgRirV7ytu3wC0W1PC0E x X-Gm-Gg: ASbGncttgBHgGUSHCIm5SnMLH3WS8TpNgmlj9+TX03Ygww9TTpf29JDgO+c5b+GsXRA RBXTTkZHOZZJJ4aBDwKXj0AjwWSDvEaK/17b/Ola2VI02/32KhW1kg++RZ6BA+mVPeWi2GSgXAk M51kJKPhux9atZZgAfjFZlGCM4hZaxXWjsplUNSSpIloT0OUqL+2S0pLln64waEB35PzOnB1Ntr roqwUO8Dfyi113R1fyI/KVH4dSvAJFVARTM4F3T42HNKK02UxQigwt+zbRWW+SAa0ogYYgJjmzA D80LVAXEXjzuC3Kx9QaOH3Ic7aWGN1kDtw== X-Google-Smtp-Source: AGHT+IFLuMhnQZuKG/wixnt2l/wwJVrVBYwWx10QwqyMM580VBnh+loIuUe5+/xNX2mByagp5hCYag== X-Received: by 2002:a17:902:d488:b0:216:2f9d:32c with SMTP id d9443c01a7336-21a83fd36d0mr395132625ad.53.1736871985970; Tue, 14 Jan 2025 08:26:25 -0800 (PST) Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-21a9f1329dcsm69979435ad.73.2025.01.14.08.26.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jan 2025 08:26:25 -0800 (PST) Date: Tue, 14 Jan 2025 08:26:23 -0800 From: Stephen Hemminger To: Gagandeep Singh Cc: "dev@dpdk.org" Subject: Re: [PATCH] eal: add worker threads cleanup in rte_eal_cleanup() Message-ID: <20250114082623.1f72b412@hermes.local> In-Reply-To: References: <20250110064717.1372216-1-g.singh@nxp.com> <20250110091902.5139f8b2@hermes.local> <20250113084017.6fe7edc3@hermes.local> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 On Tue, 14 Jan 2025 04:53:51 +0000 Gagandeep Singh wrote: > Hi, > > > -----Original Message----- > > From: Stephen Hemminger > > Sent: Monday, January 13, 2025 10:10 PM > > To: Gagandeep Singh > > Cc: dev@dpdk.org > > Subject: Re: [PATCH] eal: add worker threads cleanup in rte_eal_cleanup() > > > > On Mon, 13 Jan 2025 05:13:01 +0000 > > Gagandeep Singh wrote: > > > > > Hi, > > > > > > > -----Original Message----- > > > > From: Stephen Hemminger > > > > Sent: Friday, January 10, 2025 10:49 PM > > > > To: Gagandeep Singh > > > > Cc: dev@dpdk.org > > > > Subject: Re: [PATCH] eal: add worker threads cleanup in > > > > rte_eal_cleanup() > > > > > > > > On Fri, 10 Jan 2025 12:17:17 +0530 > > > > Gagandeep Singh wrote: > > > > > > > > > This patch introduces a worker thread cleanup function in the EAL > > > > > library, ensuring proper termination of created pthreads and > > > > > invocation of registered pthread destructors. > > > > > This guarantees the correct cleanup of thread-specific resources, > > > > > used by drivers or applications. > > > > > > > > > > Signed-off-by: Gagandeep Singh > > > > > --- > > > > > > > > What problem is this trying to solve? > > > > > > > > Canceling threads sends signals and can be problematic. > > > > Many of the operations done in drivers are not signal safe. > > > > > > To ensure the proper cleanup of thread-specific resources, the DPAA driver > > initializes pthread-specific destructors using pthread_key_create(). These > > destructors are executed only when a thread terminates or the key is deleted. > > However, since threads are not terminated when the application is killed, these > > destructors are not executed, resulting in resource leaks. > > > To address this issue, we propose adding thread termination code to > > > rte_eal_cleanup() to ensure that threads are properly terminated, > > > thereby triggering the execution of pthread-specific destructors > > > > > > Any alternate suggestion in case pthread_cancel is not a better > > > solution? We can add pthread join timeout to avoid blocking on thread stuck or > > May be any way to call pthread_exit? > > > > The DPAA driver is the problem here. It should not be using pthread_key. > > Other drivers don't do this. Other drivers do setup on probe and cleanup on close. > > > > An application doing a clean shutdown should do what existing testpmd, l3fwd, do > > - wait for worker threads to go idle > > - stop all ports > > - close all ports > > - call eal cleanup > > > > If DPAA driver needs pthread_key it should handling that in the close. > > But it really should be using DPDK thread local storage for this. > > > This is fine for graceful application termination, but what about non-graceful application termination? > I have a DPAA bus cleanup patch ready, and will submit it soon, But we still need > destructor in case application exit without calling the rte_eal_cleanup(). If application crashes, the code isn't going to be called so the pthread destructor won't help. You need to build any driver so that if application ungracefully exits, the hardware can be reset on restart. Trying to rely on application doing anything while crashing is not going to work. Alternatively, having a distinct separate watcher process (or using systemd) that can do a forceful cleanup on application crash can work. The point is you can't handle non-graceful shutdown cleanup in a DPDK driver and have it work reliably. > > I can see DPDK is giving an API rte_thread_key_create() which is defined only in UNIX and windows. > Why not for Linux? I was thinking of the recent per-thread allocator. > > I know DPAA driver is using pthread_key which it should not use but instead use > rte_thread_key_create() provided by DPDK which we can replace. > Having pthread destructor is still a valid case in my opinion.