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 D04B04297D; Tue, 18 Apr 2023 17:04:43 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A2465410EA; Tue, 18 Apr 2023 17:04:43 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 1375E4014F for ; Tue, 18 Apr 2023 17:04:42 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 60E6B21C2029; Tue, 18 Apr 2023 08:04:41 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 60E6B21C2029 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1681830281; bh=wcxzZlqrn3i7i3dym5GDG7HROQuzNDW5tglxMl33LNQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=qMScb7pNPiUjPTwrLj2ICGRg0HUrtfpbss0Z/L0LIHtFTU6QsyJHOiOcXFN9R+q7G ih3abGA4xPWN8i7Oyp9YiEBVkEXbxjtqC87YxJGbcOWfvCUzkeu8EpZSypuno0AOrm 8lKasJOrNb03lNuNjN350qj+VgfRkDaXdLsfDtUY= Date: Tue, 18 Apr 2023 08:04:41 -0700 From: Tyler Retzlaff To: Bruce Richardson Cc: dev@dpdk.org, thomas@monjalon.net Subject: Re: [PATCH 1/6] dma/skeleton: use rte thread API Message-ID: <20230418150441.GA32568@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1679092460-9930-1-git-send-email-roretzla@linux.microsoft.com> <1679092460-9930-2-git-send-email-roretzla@linux.microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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, Apr 18, 2023 at 10:19:15AM +0100, Bruce Richardson wrote: > On Fri, Mar 17, 2023 at 03:34:15PM -0700, Tyler Retzlaff wrote: > > Update driver to use rte thread API where available instead of pthread > > as a prerequisite to removing pthread stubs on Windows. > > > > Signed-off-by: Tyler Retzlaff > > --- > > drivers/dma/skeleton/skeleton_dmadev.c | 15 ++++++++------- > > drivers/dma/skeleton/skeleton_dmadev.h | 4 ++-- > > 2 files changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/dma/skeleton/skeleton_dmadev.c b/drivers/dma/skeleton/skeleton_dmadev.c > > index daf35ec..2ec10db 100644 > > --- a/drivers/dma/skeleton/skeleton_dmadev.c > > +++ b/drivers/dma/skeleton/skeleton_dmadev.c > > @@ -5,6 +5,8 @@ > > #include > > #include > > > > +#include > > + > > Curious as to why this is needed here. Does rte_thread.h not include all > needed threading dependencies? > [Self-answer] I assume this is for pthread_cancel below, right? yes, pthread_cancel has no equivalent in the EAL. windows can't easily provide anything that would behave the same, but pthread_cancel is kind of awful anyway. it's something that i'm thinking about but haven't got a good solution for. so for now any code that uses it remains. > > > > #include > > #include > > #include > > @@ -53,7 +55,7 @@ > > return 0; > > } > > > > -static void * > > +static uint32_t > > cpucopy_thread(void *param) > > { > > #define SLEEP_THRESHOLD 10000 > > @@ -81,7 +83,7 @@ > > (void)rte_ring_enqueue(hw->desc_completed, (void *)desc); > > } > > > > - return NULL; > > + return 0; > > } > > > > static void > > @@ -126,7 +128,7 @@ > > rte_mb(); > > > > snprintf(name, sizeof(name), "dma_skel_%d", dev->data->dev_id); > > - ret = rte_ctrl_thread_create(&hw->thread, name, NULL, > > + ret = rte_thread_create_control(&hw->thread, name, NULL, > > cpucopy_thread, dev); > > if (ret) { > > SKELDMA_LOG(ERR, "Start cpucopy thread fail!"); > > @@ -135,8 +137,7 @@ > > > > if (hw->lcore_id != -1) { > > cpuset = rte_lcore_cpuset(hw->lcore_id); > > - ret = pthread_setaffinity_np(hw->thread, sizeof(cpuset), > > - &cpuset); > > + ret = rte_thread_get_affinity_by_id(hw->thread, &cpuset); > > if (ret) > > SKELDMA_LOG(WARNING, > > "Set thread affinity lcore = %d fail!", > > @@ -154,8 +155,8 @@ > > hw->exit_flag = true; > > rte_delay_ms(1); > > > > - (void)pthread_cancel(hw->thread); > > - pthread_join(hw->thread, NULL); > > + (void)pthread_cancel((pthread_t)hw->thread.opaque_id); > > + rte_thread_join(hw->thread, NULL); > > > > Is there no rte_* equivalent to pthread_cancel? Will that cause issues > later? there isn't because windows can't really do what it is doing sensibly. in part it's because of how it is influences how other posix calls behave. > > > return 0; > > } > > diff --git a/drivers/dma/skeleton/skeleton_dmadev.h b/drivers/dma/skeleton/skeleton_dmadev.h > > index 6f89400..8670a68 100644 > > --- a/drivers/dma/skeleton/skeleton_dmadev.h > > +++ b/drivers/dma/skeleton/skeleton_dmadev.h > > @@ -5,9 +5,9 @@ > > #ifndef SKELETON_DMADEV_H > > #define SKELETON_DMADEV_H > > > > -#include > > > > #include > > +#include > > > > #define SKELDMA_ARG_LCORE "lcore" > > > > @@ -21,7 +21,7 @@ struct skeldma_desc { > > struct skeldma_hw { > > int lcore_id; /* cpucopy task affinity core */ > > int socket_id; > > - pthread_t thread; /* cpucopy task thread */ > > + rte_thread_t thread; /* cpucopy task thread */ > > volatile int exit_flag; /* cpucopy task exit flag */ > > > > struct skeldma_desc *desc_mem; > > -- > > 1.8.3.1 > >