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 D9C5643FB2; Tue, 7 May 2024 16:52:55 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C6054433B9; Tue, 7 May 2024 16:52:55 +0200 (CEST) Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) by mails.dpdk.org (Postfix) with ESMTP id 1B747433B1 for ; Tue, 7 May 2024 16:52:54 +0200 (CEST) Received: by mail-pf1-f172.google.com with SMTP id d2e1a72fcca58-6f45f1179c3so2315966b3a.3 for ; Tue, 07 May 2024 07:52:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1715093573; x=1715698373; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=uTO/ZWQ00y6yVwh0OC2ipjmKCSGilvFurj/W8KFE1FY=; b=EpOi0J/48SjSselfGhnE8P9lwleHh+gkeYoWT4f7QAh1srZZwGq2tCRSxTMNq7aHj1 imJcsZqMmLUN8ieahkS+3AawlvmWrmv8aSYPNVvULDsu8D05PO8/F9idz0lfOU+hSmxK vq6AV/ArFrvmzN1ZfLRDN+29U35xhx1lQnpOX6gat1JRTJcpdjUcanbnQv/X+QfoKFiq kGYxYOF69jzcVPzjQcgVX2GUC1aS/NUaAYs39iZmbH5970Z9eV4KDGw8u1rCoAplX1sz sZ5fMzY8/vXZQLwXiXmX4dSJ4CWVgsxZKQQ9ZEMZP3nVDCEQ6H1NjTlpEnyqCUmAOeh1 abZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715093573; x=1715698373; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=uTO/ZWQ00y6yVwh0OC2ipjmKCSGilvFurj/W8KFE1FY=; b=pFZ1YJp/3dtenLzP1ORn6bb0Xkuzb9E1yW29WlDooi8NRuIMOAFHHDSEU2C5yEXDFp JUZupKYD4VsEe6y90MlJbXivqs73o7iPa3uPmx0GmhAevX+5/drIdOOgLp/QagIQjySm cBvYuHmOhFZsCIA3i0JCcAxms3vtACJnT2MeLKUJnWxhpinDDSQRPydEHPS71TPQOV5W yukJjOZdnw5amKDnjOd7r+Or1HN4HwR503UhgjSB3FwKzoXcaGHe6JRYIAENnRweAXUI izic1/QVbZYJzVfHForS098IxzQqWLH4TP1UWJ3kQP71YV2YY71xJMLOSKFugV1vs/fP YOBA== X-Forwarded-Encrypted: i=1; AJvYcCXbzvrgziVFjlcTaulXVtXEbuwJu9z87CPvZA4Plv49AnEVpMPQK9arj4zdIFc3NVLgwCywq3AbjGt6q/U= X-Gm-Message-State: AOJu0Yy1nAt0lsB1Ux2Xv+dWsu6e/Adajw+xJ1OBKejCIbNgQxrPNdsx 5plP7firNl7eruY19SfVMjmHPlVw5U3Gg6L168V3X9ns1tSxHNTb40pyt/heI0Y= X-Google-Smtp-Source: AGHT+IGSq3ZC9Hs8Yz8YAwZvRh5DZu8H4Rr1c91AFRSgYogs+ypVQS4iMixi8Bmc6fO7FcsSCsKHOQ== X-Received: by 2002:a05:6a20:a128:b0:1af:baf9:feee with SMTP id q40-20020a056a20a12800b001afbaf9feeemr4975893pzk.26.1715093573133; Tue, 07 May 2024 07:52:53 -0700 (PDT) Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id u10-20020a170902e80a00b001e826e4d087sm10111255plg.142.2024.05.07.07.52.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 May 2024 07:52:52 -0700 (PDT) Date: Tue, 7 May 2024 07:52:51 -0700 From: Stephen Hemminger To: Ferruh Yigit Cc: "John W. Linville" , Thomas Monjalon , dev@dpdk.org, Mattias =?UTF-8?B?UsO2bm5ibG9t?= , Morten =?UTF-8?B?QnLDuHJ1cA==?= , Honnappa Nagarahalli Subject: Re: [RFC v3] net/af_packet: make stats reset reliable Message-ID: <20240507075251.144e91b7@hermes.local> In-Reply-To: <44f2d6b8-9b38-4e1f-9d9a-46d719532808@amd.com> References: <20240425174617.2126159-1-ferruh.yigit@amd.com> <20240503154547.392069-1-ferruh.yigit@amd.com> <20240503150011.55681b97@hermes.local> <44f2d6b8-9b38-4e1f-9d9a-46d719532808@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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, 7 May 2024 14:48:51 +0100 Ferruh Yigit wrote: > On 5/3/2024 11:00 PM, Stephen Hemminger wrote: > > On Fri, 3 May 2024 16:45:47 +0100 > > Ferruh Yigit wrote: > > =20 > >> For stats reset, use an offset instead of zeroing out actual stats val= ues, > >> get_stats() displays diff between stats and offset. > >> This way stats only updated in datapath and offset only updated in sta= ts > >> reset function. This makes stats reset function more reliable. > >> > >> As stats only written by single thread, we can remove 'volatile' quali= fier > >> which should improve the performance in datapath. > >> > >> While updating around, 'igb_stats' parameter renamed as 'stats'. > >> > >> Signed-off-by: Ferruh Yigit > >> --- > >> Cc: Mattias R=C3=B6nnblom > >> Cc: Stephen Hemminger > >> Cc: Morten Br=C3=B8rup > >> > >> This update triggered by mail list discussion [1]. > >> > >> [1] > >> https://inbox.dpdk.org/dev/3b2cf48e-2293-4226-b6cd-5f4dd3969f99@lysato= r.liu.se/ =20 > >=20 > >=20 > > NAK > >=20 > > I did not hear a good argument why atomic or volatile was necessary in = the first place. > > Why? > > =20 >=20 > Sure, the patch is done as RFC intentionally to discuss the approach. >=20 > Agree that volatile and atomics (fetch + add + store) is not required > for thread synchronization, as only one CPU updates stats. > Even this understanding is important because there are PMDs using full > atomics for stats update, like null PMD [1], this will help up clear them. >=20 >=20 > And there is a case, stats reset and stats updated in different threads > simultaneously, for this 'volatile' is not sufficient anyway and full > atomics is required. As this will cause performance impact we are > already saying stats update and reset can't happen at the same time [2]. > With this update volatile and atomics are not required for this case too. > (Also using offset to increase stats reset reliability.) >=20 >=20 > In this patch volatile replaced with atomic load and atomic store (not > atomic fetch and add), to ensure that stats stored to memory and not > kept in device registers only. > With volatile, it is guaranteed that updated stats stored back to > memory, but without volatile and atomics I am not sure if this is > guaranteed. Practically I can see this working, but theoretically not > sure. This is similar concern with change in your patch that is casting > to volatile to ensure value read from memory [3]. >=20 > Expectation is, only atomics load and store will have smaller > performance impact than volatile, ensuring memory load and store when > needed. The device register worry, can just be handled with a compiler barrier. Does not need the stronger guarantee of atomic or volatile.