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 E2788A0C58; Mon, 23 Aug 2021 22:25:57 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6343440151; Mon, 23 Aug 2021 22:25:57 +0200 (CEST) Received: from mail-lj1-f181.google.com (mail-lj1-f181.google.com [209.85.208.181]) by mails.dpdk.org (Postfix) with ESMTP id D9C054014D for ; Mon, 23 Aug 2021 22:25:55 +0200 (CEST) Received: by mail-lj1-f181.google.com with SMTP id h9so33633277ljq.8 for ; Mon, 23 Aug 2021 13:25:55 -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=5SP9oR8KdbQ2zhhkT+kGI6ki/eUWjZpVAg6nFxIWt/4=; b=HMRlXsRx/k/rXN8oOsAQ6vsRAeVA78JQTASsj0Mc5DCabrAnPZwVMDVqAaFO0CCg/N nuXMctyYUKS6iCiKDNcFPNzp4mnQO/U1gh4glTAgZLPQOXgShMYNZ4X1UUcmL0CxpR5H VuuaPJWHBBnO8gRDWDNTmTwoqYJ0MdVlx7L22gd6izPnWKEPnL0Cu8eU4h7zw9q2yzBV M5krvRntmnImHDFCTH4ex0tRnzPwQ4vYKR2+ZfwOJxM2Q3Qn6UMqzbJYa702PzT1iw63 xBtCP2dUs1o3AcAkKtqVG/esv3+AjjOFv67bwx4A0pKPZyaZhrSzbbz3i0OKdrAbDtzx G5JQ== 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=5SP9oR8KdbQ2zhhkT+kGI6ki/eUWjZpVAg6nFxIWt/4=; b=aEb5NEVlBmORVWA/UJkvHmuPB4c2alxCyg2yXCzezVV+YmM2POJyRwOMSsliL/UqdJ kwpdOpe2WPwOvkz5XXLLiMBPdIFeMQQ9zKSeGDqOq7bTq99pH8oV+4+luAAfslGQiKVz rHGC5P2fm4jjalWkJEWZ8gzkUfllQB43LV5lVxcrwLcgGr069WXbzmIZKcXwtLRAHk/T 3lnt5GNPrIhz6Q3hNzDJkMUTGbKrlYiGTbeKHmm+4CWzdd9ouQvx4EMQK8kJiYiH/sE5 5cUAT8ri2ZpB07VLSc2LhqzFq5sQuChPNGdNugahmQLtTAAOCm1dS1IXy8HYqnN6/GDe zGHg== X-Gm-Message-State: AOAM533tg9yfq+ts5na0gQncOWePSfyeAQLZz/aXE4Ve18W79XxzfpmS sOxkR5/+lh4iMxhqZ2NtPbE= X-Google-Smtp-Source: ABdhPJyK/fpDkAcb6ghpuPwv7LSAt6jWjovUA21Vn10DupwQIiyNkBXJpAPUCKgfHhTJDM+4HQStVQ== X-Received: by 2002:a2e:a316:: with SMTP id l22mr26978597lje.104.1629750355327; Mon, 23 Aug 2021 13:25:55 -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 y6sm1541435lfg.225.2021.08.23.13.25.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Aug 2021 13:25:54 -0700 (PDT) Date: Mon, 23 Aug 2021 23:25:52 +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: <20210823232552.1a24ae1b@sovereign> In-Reply-To: <1629408694-31803-10-git-send-email-navasile@linux.microsoft.com> References: <1628017291-3756-1-git-send-email-navasile@linux.microsoft.com> <1629408694-31803-1-git-send-email-navasile@linux.microsoft.com> <1629408694-31803-10-git-send-email-navasile@linux.microsoft.com> X-Mailer: Claws Mail 3.17.8 (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 v14 9/9] Add unit tests for thread API 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-08-19 14:31 (UTC-0700), Narcisa Ana Maria Vasile: > From: Narcisa Vasile > > As a new API for threading is introduced, > a set of unit tests have been added to test the new interface. There are three substantial issues with this tests: 1. They use pthread API to verify the results, but the whole point of introducing this new API is to abstract pthread or Win32 interfaces and eventually to remove pthread shim from Windows EAL. Furthermore, new new API actually has functions to do the checks, e.g. rte_thread_get_affinity(). 2. They don't really check the contracts: 2.1. Mutex test could pass even if it used no locking at all. 2.2. Barrier test does not verify that threads are blocked/unblocked. 2.3. No test for static initialization. See also comments inline. [...] > +#define TEST_THREADS_LOG(func) \ > + printf("Error at line %d. %s failed!\n", __LINE__, func) rte_test.h contains some useful macros like this one. [...] > +static int > +test_thread_self(void) > +{ > + rte_thread_t threads_ids[THREADS_COUNT]; > + rte_thread_t self_ids[THREADS_COUNT] = {}; > + size_t i; > + size_t j; > + int ret = 0; > + > + for (i = 0; i < THREADS_COUNT; ++i) { > + if (rte_thread_create(&threads_ids[i], NULL, thread_loop_self, > + &self_ids[i]) != 0) { > + printf("Error, Only %zu threads created.\n", i); Typo: "only" (in other copies too). This test could use RTE_LOG_REGISTER() and log levels if needed. > + break; > + } > + } > + > + for (j = 0; j < i; ++j) { > + ret = rte_thread_join(threads_ids[j], NULL); > + if (ret != 0) { > + TEST_THREADS_LOG("rte_thread_join()"); > + return -1; > + } > + > + if (rte_thread_equal(threads_ids[j], self_ids[j]) == 0) > + ret = -1; Tests should indicate what exactly went wrong. Suggesting RTE_TEST_ASSERT_EQUAL() here. > + } > + > + return ret; > +} > + > +struct thread_context { > + rte_thread_barrier *barrier; > + size_t *thread_count; > +}; > + > +static void * > +thread_loop_barrier(void *arg) > +{ > + Unnecessary empty line. > + struct thread_context *ctx = arg; > + > + (void)__atomic_add_fetch(ctx->thread_count, 1, __ATOMIC_RELAXED); > + > + if (rte_thread_barrier_wait(ctx->barrier) > 0) > + TEST_THREADS_LOG("rte_thread_barrier_wait()"); > + > + return NULL; > +} [...]