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 B3456A00C3; Wed, 7 Dec 2022 21:37:47 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 99A62410D7; Wed, 7 Dec 2022 21:37:47 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 06F8040156 for ; Wed, 7 Dec 2022 21:37:46 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 4E78D20B6C40; Wed, 7 Dec 2022 12:37:45 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 4E78D20B6C40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1670445465; bh=l80uceZcFMhB/BrokNwr76np62tkAsMmOz4sQLNhXQc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=n7oQuXFFDLMUhmVHthGCmaRjAeM012krIYbg+A4IBYniiAPfv7eJ4uryVhz8mgzHZ dpuvGHuDiyyxf5DtmfV0PZgaqIvQBEN74j70wKuQL18yMhgfp6OeUt4/Wpr9vY9AYy RbTKtP6VJqWC79fpJqBwQ+t6MEUs3IwcPAwoiM24= Date: Wed, 7 Dec 2022 12:37:45 -0800 From: Tyler Retzlaff To: Mattias =?iso-8859-1?Q?R=F6nnblom?= Cc: dev@dpdk.org, thomas@monjalon.net, david.marchand@redhat.com, stephen@networkplumber.org, olivier.matz@6wind.com, mb@smartsharesystems.com Subject: Re: [PATCH v2 1/3] eal: add rte control thread create API Message-ID: <20221207203745.GF14713@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1670271868-11364-1-git-send-email-roretzla@linux.microsoft.com> <1670347706-23802-1-git-send-email-roretzla@linux.microsoft.com> <1670347706-23802-2-git-send-email-roretzla@linux.microsoft.com> <5b080d72-f7a8-232d-86a1-9164a005fcfb@lysator.liu.se> <20221207163839.GB14713@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20221207163839.GB14713@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> 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 just following up. On Wed, Dec 07, 2022 at 08:38:39AM -0800, Tyler Retzlaff wrote: > On Wed, Dec 07, 2022 at 10:13:39AM +0100, Mattias Rönnblom wrote: > > > > To be consistent with function naming scheme, where "ctrl" is the > > old API, and "control" the new, you could call the start functions > > something with "ctrl" and "control" as well. > > i'll make this change in v3. > > > > > Maybe it's worth mentioning that fact somewhere in the beginning of > > the file, as a comment (i.e., that "ctrl" denotes the old API). > > i'll make this change in v3. didn't end up doing this, didn't think it was necessary once i looked at it the commit history and the rename you suggested above should be satisfactory. > > > > > >+ } u; > > > void *arg; > > > int ret; > > > > Why is 'ret' needed? (This question is unrelated to your patch.) > > i'm not the original author so difficult to answer authoritatively. but > if i have to speculate i'd say it might be to work around the windows > pthread_join stub being implemented as a noop. specifically it didn't > communicate the return value from the start_routine. > > the recently added rte_thread_join addresses this (which > rte_control_thread_create uses) and could remove ret parameter and to > avoid touching the new function implementation in the future it can not > use ret now. > > i'll make this change in v3. also did not do this, once looked at i was concerned about inter-mixing too much semantic change in the implementation of a couple of the functions to accomplish it. it is best revisited when the old api is removed. > > >+ * the error is ignored and a debug message is logged. > > >+ * > > >+ * @param thread > > >+ * Filled with the thread id of the new created thread. > > >+ * @param name > > >+ * The name of the control thread (max 16 characters including '\0'). > > > > Is there a constant for this limit? > > i have a series introducing rte_lcore_{set,get}_name api that introduces > a constant (probably i'll post it today). as of this series there is no > constant. change in v3, i forgot we were talking about the thread length limit and not the lcore name length limit and there was already a preprocessor definition for the limit. api documentation comment was updated in v3. thanks!