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 A35A8A09E4; Fri, 29 Jan 2021 05:58:58 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 69BCD4067B; Fri, 29 Jan 2021 05:58:58 +0100 (CET) Received: from mail-pg1-f176.google.com (mail-pg1-f176.google.com [209.85.215.176]) by mails.dpdk.org (Postfix) with ESMTP id 4D26840395 for ; Fri, 29 Jan 2021 05:58:57 +0100 (CET) Received: by mail-pg1-f176.google.com with SMTP id t25so5860493pga.2 for ; Thu, 28 Jan 2021 20:58:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Okd3bI5c+x/edz0uCgHD9SmPtV+HD1rSAoiiXktDWk4=; b=VDDVPU09mEOlZq24uAEMQwOu4l6xvptFa3Sdzrvyw+ip8xZyF17Bx6QZ+hLSYUX3PE BpSFY1jOthnchUMo2hN849F7M4A3vrb7wFHr8uMiCEjRL3k25Ei8S4eVTYWvMdXfEL6S rQ+pfk8tMrRiB6KIagnAgxC0vCaoB/exOCLoBoa5Vs3nrAeQIGQmMCXnw1ZndNBPyeKa Pqy9GtNzjqtmyzvlexywrpEsbbJO/n0jInIum8dJNqYU6Twl9Z+JdjfwctnMiGN+vtw8 SHykqjcpeAkmGv1tVrRykba7n5eV2yPXjGyaEDtrG8rwk7j97KbqPMjqzD9dTG2CJM8t AO1w== 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=Okd3bI5c+x/edz0uCgHD9SmPtV+HD1rSAoiiXktDWk4=; b=n9bHVAhmJS+FdiQPyhRmsqqq7at4AUvutPZEBGGAtLss/bUtuNE8GjbNP+f84NEAxv DJOTRMu6strG/e5IftaX1NW+6V9M96e5K+hTdxOuDN/95xUzKZSLIQD26ghE3GNSGKug 1Ji+7Vq4UJatFS0jGWek1HcDmDWC/9j/aqWzjFpJ4xBdWv9mS1coimUkXxLOXy+KpNnN ohih2yWoodfFopAKuJe+Zqr34c/fcgdI965AbVO1U3BWJTMOlnL8zqpjVx7RczdTT3wO j/suHzugRY8ZO4G/GwZOyHcmckTRr+ibeGZ2xKZZ9Q7bnppnhiuZvB2JCAttFg2wWGj4 gG7w== X-Gm-Message-State: AOAM533e86Igx3sDbs6zYN6mq/nhzIesTi4iTQiY+YEQpfA4k4d0PmI3 5CZ4t2VXcwkWHop4oqXIVMDSfQ== X-Google-Smtp-Source: ABdhPJzp4aVeUkdcqrav6Nbo9XWTDVJHGjSJLUMbVc2uXacq6jNenR2lZnEBVbUI19SncSt2GwS2rA== X-Received: by 2002:a62:a204:0:b029:1c3:fb27:16f3 with SMTP id m4-20020a62a2040000b02901c3fb2716f3mr2604671pff.61.1611896336229; Thu, 28 Jan 2021 20:58:56 -0800 (PST) Received: from hermes.local (76-14-222-244.or.wavecable.com. [76.14.222.244]) by smtp.gmail.com with ESMTPSA id p2sm6880774pjj.0.2021.01.28.20.58.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Jan 2021 20:58:55 -0800 (PST) Date: Thu, 28 Jan 2021 20:58:47 -0800 From: Stephen Hemminger To: Honnappa Nagarahalli Cc: "Ananyev, Konstantin" , Feifei Wang , "dev@dpdk.org" , nd , Ruifeng Wang Message-ID: <20210128205847.509412db@hermes.local> In-Reply-To: References: <20201222063054.44429-1-feifei.wang2@arm.com> <20201222063054.44429-2-feifei.wang2@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v1 1/2] app/test: remove unnecessary barriers for ring stress test 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" On Fri, 29 Jan 2021 03:17:50 +0000 Honnappa Nagarahalli wrote: > >=20 > > =20 > > > > > > > > Hi Feifei, > > > > =20 > > > > > > > > > > The variable "wrk_cmd" is a signal to control threads from running > > > > > and stopping. When worker lcores load "wrk_cmd =3D=3D =20 > > WRK_CMD_RUN", =20 > > > > > they =20 > > > > start =20 > > > > > running and when worker lcores load "wrk_cmd =3D=3D WRK_CMD_STOP"= , =20 > > > > they =20 > > > > > stop. > > > > > > > > > > For the wmb in test_mt1, no storing operations must keep the order > > > > > after storing "wrk_cmd". Thus the wmb is unnecessary. =20 > > > > > > > > I think there is a bug in my original code, we should do smp_wmb() > > > > *before* setting wrk_cmd, not after: > > > > > > > > /* launch on all workers */ > > > > RTE_LCORE_FOREACH_WORKER(lc) { > > > > arg[lc].rng =3D r; > > > > arg[lc].stats =3D init_stat; > > > > rte_eal_remote_launch(test, &arg[lc], lc); > > > > } > > > > > > > > /* signal worker to start test */ > > > > + rte_smp_wmb(); > > > > wrk_cmd =3D WRK_CMD_RUN; > > > > - rte_smp_wmb(); > > > > > > > > usleep(run_time * US_PER_S); > > > > > > > > > > > > I still think we'd better have some synchronisation here. > > > > Otherwise what would prevent compiler and/or cpu to update wrk_cmd > > > > out of order (before _init_ phase is completed)? > > > > We probably can safely assume no reordering from the compiler here, > > > > as we have function calls straight before and after 'wrk_cmd =3D =20 > > WRK_CMD_RUN;' =20 > > > > But for consistency and easier maintenance, I still think it is > > > > better to have something here, after all it is not performance crit= ical pass. =20 > > > Agree that this is not performance critical. > > > > > > This is more about correctness (as usually people refer to code to > > > understand the concepts). You can refer to video [1]. Essentially, the > > > pthread_create has 'happens-before' behavior. i.e. all the memory > > > operations before the pthread_create are visible to the new thread. > > > The > > > rte_smp_rmb() barrier in the thread function is not required as it re= ads the =20 > > data that was set before the thread was launched. > >=20 > > rte_eal_remote_launch() doesn't call pthread_create(). > > All it does - updates global variable (lcore_config) and writes/reads = to/from > > the pipe. > > =20 > Thanks for the reminder =E2=98=B9 > I think rte_eal_remote_launch and rte_eal_wait_lcore need to provide beha= vior similar to pthread_launch and pthread_join respectively. >=20 > There is use of rte_smp_*mb in those functions as well. Those need to be = fixed first and then look at these. Looks like you want __atomic_thread_fence() here.