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 6E4C6A0A0E; Thu, 29 Apr 2021 02:50:40 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5599A412C2; Thu, 29 Apr 2021 02:50:40 +0200 (CEST) Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) by mails.dpdk.org (Postfix) with ESMTP id A232B412C1 for ; Thu, 29 Apr 2021 02:50:36 +0200 (CEST) Received: by mail-lf1-f52.google.com with SMTP id y4so61950862lfl.10 for ; Wed, 28 Apr 2021 17:50:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=75y9Z6TxQtBOBd4JWFXjC7BGnOXnTWq5PqvIg7Txm7M=; b=emLPksp7xIzrNrDHOGf/iOR4lCcWKxQcMZG4uthuJgM5xzBOXV2It6ugjkdFtFSDej ah8hkXDbLcCPweBE599iWGk06PmkVt7oPUF+PwT+rIjObfqwhyLrLLlnhOJaxDxi1FtN 1vLfhKqp+y1N+Iy3SHkoJUwqK4loluC5iA/bntIaM6U7OXqhi+VPUVI1bDVHhqYo77ax WhewLT8ETEBZpwM/3nXNVdvsAW3CwJfSDmiQuHG1tQy5Vf4QBIX2ShccSNFrwY19HncE LoEDTqibCoyeBSjnHjVoo7swgfmaUryUtLSHYzNuVGIcqPjawbL4wr+FbI7ZHPRi1Xux tL4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=75y9Z6TxQtBOBd4JWFXjC7BGnOXnTWq5PqvIg7Txm7M=; b=sRHpi1xrj83jrWR7Oj5Pvd7NV7EDiSaaXg3dXow2AIo7drUCbWQ3fmj4f+sgVm+oiq Y0V0VowF2qaLq7twDVSIEMedEjOTz7nuKEFlAqXdCGKfqsqluOXKueMawA55Xir/ir7P bs8EVi3xgctngSdjit4q6SUxzDBF3HVkzBUveypMkRub17W2XOLgnnhkAXOuERHjS1Hy dGGeeCuj4+q4XKJ4oU90BX1gStiS8U1oN/9Tas90u3OZKv46NA1uf0WjrOCcVHDX8cEE e12VF1/izozkDUOYJokxaUoW1C6nD2t646DS9110OI55AZjl07M1SX1YvGUQ6gb+DPuV 3gSQ== X-Gm-Message-State: AOAM531xv8zjhPd/kdldIkmlHKv24NpbQDBbLFk65JR0dlBffAbMUrCN m5kmLUXxgctwAsfPVTyLeR0= X-Google-Smtp-Source: ABdhPJz12WUPuL5X5oHwk8IuXXiC76+m5+NXl00xBQNMjpK30o/ADRNw9Z4QaxkMfvLhYpp9qJZmog== X-Received: by 2002:a19:614c:: with SMTP id m12mr21823691lfk.606.1619657436218; Wed, 28 Apr 2021 17:50:36 -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 a4sm330897lfs.130.2021.04.28.17.50.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Apr 2021 17:50:35 -0700 (PDT) Date: Thu, 29 Apr 2021 03:50:34 +0300 From: Dmitry Kozlyuk To: Narcisa Ana Maria Vasile Cc: dev@dpdk.org, thomas@monjalon.net, khot@microsoft.com, navasile@microsoft.com, dmitrym@microsoft.com, roretzla@microsoft.com, talshn@nvidia.com, ocardona@microsoft.com, bruce.richardson@intel.com, david.marchand@redhat.com, pallavi.kadam@intel.com Message-ID: <20210429035034.570f33e3@sovereign> In-Reply-To: <1617413948-10504-3-git-send-email-navasile@linux.microsoft.com> References: <1617057640-24301-2-git-send-email-navasile@linux.microsoft.com> <1617413948-10504-1-git-send-email-navasile@linux.microsoft.com> <1617413948-10504-3-git-send-email-navasile@linux.microsoft.com> X-Mailer: Claws Mail 3.17.6 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v6 02/10] eal: add thread attributes 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 Sender: "dev" 2021-04-02 18:39 (UTC-0700), Narcisa Ana Maria Vasile: > From: Narcisa Vasile > > Implement thread attributes for: > * thread affinity > * thread priority > > Implement functions for managing thread attributes. > > Signed-off-by: Narcisa Vasile > --- > lib/librte_eal/common/rte_thread.c | 53 ++++++++++++ > lib/librte_eal/include/rte_thread.h | 82 +++++++++++++++++++ > lib/librte_eal/include/rte_thread_types.h | 3 + > .../include/rte_windows_thread_types.h | 3 + > lib/librte_eal/windows/rte_thread.c | 56 +++++++++++++ > 5 files changed, 197 insertions(+) > > diff --git a/lib/librte_eal/common/rte_thread.c b/lib/librte_eal/common/rte_thread.c > index 5ec382949..0bd1b115d 100644 > --- a/lib/librte_eal/common/rte_thread.c > +++ b/lib/librte_eal/common/rte_thread.c Please see a comment to Windows counterpart of this file. [...] > diff --git a/lib/librte_eal/include/rte_thread.h b/lib/librte_eal/include/rte_thread.h > index cbc07f739..bfdd8e1b1 100644 > --- a/lib/librte_eal/include/rte_thread.h > +++ b/lib/librte_eal/include/rte_thread.h > @@ -28,6 +28,19 @@ extern "C" { > #include > #endif > > +enum rte_thread_priority { > + RTE_THREAD_PRIORITY_NORMAL = EAL_THREAD_PRIORITY_NORMAL, > + RTE_THREAD_PRIORITY_REALTIME_CRITICAL = EAL_THREAD_PRIORITY_REALTIME_CIRTICAL, Please add Doxygen comments for each new public item in the API. > + /* > + * This enum can be extended to allow more priority levels. > + */ This comment is useless: any enum can be extended. (FYI, recently DPDK eliminated xxx_MAX enum members just so that apps don't rely on enums not being extended.) > +}; > + > +typedef struct { > + enum rte_thread_priority priority; > + rte_cpuset_t cpuset; > +} rte_thread_attr_t; > + > /** > * TLS key type, an opaque pointer. > */ > @@ -60,6 +73,75 @@ rte_thread_t rte_thread_self(void); > __rte_experimental > int rte_thread_equal(rte_thread_t t1, rte_thread_t t2); > > +/** > + * Initialize the attributes of a thread. > + * These attributes can be passed to the rte_thread_create() function > + * that will create a new thread and set its attributes according to attr; Typo: ";" -> "." > + * > + * @param attr > + * Thread attributes to initialize. > + * > + * @return > + * On success, return 0. > + * On failure, return a positive errno-style error number. > + */ > +__rte_experimental > +int rte_thread_attr_init(rte_thread_attr_t *attr); > + > +/** > + * Set the CPU affinity value in the thread attributes pointed to > + * by 'thread_attr'. > + * > + * @param thread_attr > + * Points to the thread attributes in which affinity will be updated. > + * > + * @param cpuset > + * Points to the value of the affinity to be set. > + * > + * @return > + * On success, return 0. > + * On failure, return a positive errno-style error number. > + */ > +__rte_experimental > +int rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr, > + rte_cpuset_t *cpuset); This description belongs to another function, doesn't it? Same for other added functions below. Please double-check comments. [...] > diff --git a/lib/librte_eal/include/rte_thread_types.h b/lib/librte_eal/include/rte_thread_types.h > index 19fb85e38..a884daf17 100644 > --- a/lib/librte_eal/include/rte_thread_types.h > +++ b/lib/librte_eal/include/rte_thread_types.h > @@ -7,6 +7,9 @@ > > #include > > +#define EAL_THREAD_PRIORITY_NORMAL 0 > +#define EAL_THREAD_PRIORITY_REALTIME_CIRTICAL 99 Typo: "cirtical" -> "critical" Are these values chosen arbitrarily? What do you think of making "enum rte_thread_priority" members just 0 and 1, then mapping them to OS-specific values in getters/setters? > + > typedef pthread_t rte_thread_t; > > #endif /* _RTE_THREAD_TYPES_H_ */ > diff --git a/lib/librte_eal/windows/include/rte_windows_thread_types.h b/lib/librte_eal/windows/include/rte_windows_thread_types.h > index ebd3d9e8f..8cb4b3856 100644 > --- a/lib/librte_eal/windows/include/rte_windows_thread_types.h > +++ b/lib/librte_eal/windows/include/rte_windows_thread_types.h > @@ -7,6 +7,9 @@ > > #include > > +#define EAL_THREAD_PRIORITY_NORMAL THREAD_PRIORITY_NORMAL > +#define EAL_THREAD_PRIORITY_REALTIME_CIRTICAL THREAD_PRIORITY_TIME_CRITICAL > + > typedef DWORD rte_thread_t; > > #endif /* _RTE_THREAD_TYPES_H_ */ > diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c > index 940d9c653..b29336cbd 100644 > --- a/lib/librte_eal/windows/rte_thread.c > +++ b/lib/librte_eal/windows/rte_thread.c > @@ -24,6 +24,62 @@ rte_thread_equal(rte_thread_t t1, rte_thread_t t2) > return t1 == t2 ? 1 : 0; > } > > +int > +rte_thread_attr_init(rte_thread_attr_t *attr) > +{ > + if (attr == NULL) { > + RTE_LOG(DEBUG, EAL, > + "Unable to init thread attributes, invalid parameter\n"); > + return EINVAL; > + } This message doesn't add value for debugging: caller already knows that attribute initialization failed (that's what function attempts to do) and that the parameter is invalid (EINVAL). I'd remove it (same applies below). If you find it useful to keep, an extra indent missing (also more below). [...]