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 D64FEA00C5; Tue, 21 Jun 2022 21:44:24 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 77E6C4069C; Tue, 21 Jun 2022 21:44:24 +0200 (CEST) Received: from mail-lf1-f43.google.com (mail-lf1-f43.google.com [209.85.167.43]) by mails.dpdk.org (Postfix) with ESMTP id E9CB940151 for ; Tue, 21 Jun 2022 21:44:23 +0200 (CEST) Received: by mail-lf1-f43.google.com with SMTP id c2so24146241lfk.0 for ; Tue, 21 Jun 2022 12:44:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Nriog/zuBXSiAhryUwc1iCisJO/RXG5ZhdIlfqGu+lA=; b=CJYxAAfcSoZZTK5JjYd+fh92Tif+lmilm/8D0cTuQGiK6pg9NIz/UWILM0bf15nP8m qVug66v19ea2CRyDXNnt/vYXQ5ilhvClYjt2I0Xwue4M/0azAwNKi5BNVFdpXzEhhB8i tCuFE/sJ6wChVvBTyZBst1lSG/P1eJNTICcNS7E9+0xQl9WwWaZvi1c/xOZA/ZwhNt5p wN1Sa2qJFzxPd/0UZaL/FWniP+jSoIHOJsAlfidSMn8Bs+6GWZda8DG7snLGZNwSaNlS cXJ6pQFHIThqYxjZH88yMCX4mZeahdugawF4NsBM1Z02ckoCARep/F4ANVPgZdmZnOks lABQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Nriog/zuBXSiAhryUwc1iCisJO/RXG5ZhdIlfqGu+lA=; b=fcrnfmoJ6AjVh/7ryEbblJ2REFccxALjXEd5sBmQ42GyIew6cmMaRg5kj7P8cIMMGy Q2zM7OOXMtkYjfbdTN1VjbEMbI/OgW9otwoSI9Ct8wpqP1yq7ZPT0SP8KI2/Wf1Nyywu LI3nmXtU05CFbfVX6rRtmd2GtDQYopkI48xN9H1w/IRukfph3AMf2HPtMu9LZ4cJ5eXH 4Z9hgPWYOlqbkH01PL3P0QDZB6CMgsL12h2Ec8jO0+wkbqi2OFu96Ut6tCngH9RebrxX w50Jzrk2Jq0wCRUmcerADY7HgqgM/WDZb26UUpxS3faBdZmwbIH9cITfYfZB5Zjc2alW ZOmg== X-Gm-Message-State: AJIora/d1y8iu7hNgqA8mBHCrQ7Sc4RFei5FM1P2pMJgEj1VQlUeXXB8 cvlnCLCoNhQTOgNp4Q58e6c= X-Google-Smtp-Source: AGRyM1sAOjBlEtdiaLTlNd6o7MUNJr/nT9t85maFV+A+vWWeVSxDuQ1SQpQtyrrSNCrqCsc+J3BXQQ== X-Received: by 2002:a05:6512:23a4:b0:47f:79df:2ea9 with SMTP id c36-20020a05651223a400b0047f79df2ea9mr4021886lfv.498.1655840663307; Tue, 21 Jun 2022 12:44:23 -0700 (PDT) Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id a7-20020a194f47000000b0047f77cc3287sm665291lfk.274.2022.06.21.12.44.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Jun 2022 12:44:22 -0700 (PDT) Date: Tue, 21 Jun 2022 22:44:21 +0300 From: Dmitry Kozlyuk To: Tyler Retzlaff Cc: dev@dpdk.org, thomas@monjalon.net, anatoly.burakov@intel.com, Narcisa Vasile Subject: Re: [PATCH v2 2/6] eal: add thread lifetime management Message-ID: <20220621224421.244a772f@sovereign> In-Reply-To: <20220621185103.GC18214@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1654783134-13303-1-git-send-email-roretzla@linux.microsoft.com> <1655250438-18044-1-git-send-email-roretzla@linux.microsoft.com> <1655250438-18044-3-git-send-email-roretzla@linux.microsoft.com> <20220618155908.70e822af@sovereign> <20220621185103.GC18214@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) 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 2022-06-21 11:51 (UTC-0700), Tyler Retzlaff: > > > +int > > > +rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr) > > > +{ > > > + int ret = 0; > > > + void *res = NULL; > > > + void **pres = NULL; > > > + > > > + if (value_ptr != NULL) > > > + pres = &res; > > > + > > > + ret = pthread_join((pthread_t)thread_id.opaque_id, pres); > > > + if (ret != 0) { > > > + RTE_LOG(DEBUG, EAL, "pthread_join failed\n"); > > > + return ret; > > > + } > > > + > > > + if (value_ptr != NULL && *pres != NULL) > > > + *value_ptr = *(unsigned long *)(*pres); > > > + > > > + return 0; > > > +} > > > > What makes *pres == NULL special? > > it's not clear what you mean, can you explain? maybe there is some > context i am missing from the original patch series? There's no previous context. After ptread_join(), *pres holds the return value of the thread routine. You only assign *value_ptr if value_ptr is not NULL (obviously correct) and if *pres != NULL, that is, if the thread returned a non-NULL value. But this value is opaque, why do you filter NULL? Perhaps you meant if (pres != NULL), no dereference? > > > +int > > > +rte_thread_create(rte_thread_t *thread_id, > > > + const rte_thread_attr_t *thread_attr, > > > + rte_thread_func thread_func, void *args) > > > +{ > > > + int ret = 0; > > > + DWORD tid; > > > + HANDLE thread_handle = NULL; > > > + GROUP_AFFINITY thread_affinity; > > > + struct thread_routine_ctx *ctx = NULL; > > > + > > > + ctx = calloc(1, sizeof(*ctx)); > > > + if (ctx == NULL) { > > > + RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n"); > > > + ret = ENOMEM; > > > + goto cleanup; > > > + } > > > + ctx->routine_args = args; > > > + ctx->thread_func = thread_func; > > > + > > > + thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx, > > > + CREATE_SUSPENDED, &tid); > > > + if (thread_handle == NULL) { > > > + ret = thread_log_last_error("CreateThread()"); > > > + free(ctx); > > > + goto cleanup; > > > > Missing `free(ctx)` from other error paths below. > > beyond this point free(ctx) will happen in thread_func_wrapper. i will > add a comment to make it clear. Not if you exit before ResumeThread() and thread_func_wrapper() will never execute to call free().