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 20CBE431DD; Tue, 24 Oct 2023 17:58:43 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C210140685; Tue, 24 Oct 2023 17:58:42 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 1159D402C5 for ; Tue, 24 Oct 2023 17:58:41 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 3061820B74C0; Tue, 24 Oct 2023 08:58:40 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 3061820B74C0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1698163120; bh=d+xLQd09r7IZ2+zugSE3h/LtG+8OBls7I7oMSuv6O14=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rao5vxirgrjCk7C4LKPKaoE8HrEkA1R/sU65OdZ+wMglFFK9TufPDcn3h3RGzqhTz cNW6wRlDeY9uTZqxEF6sSknc7I4jzx2veBEWPVRBN2N4W88TNjohCvraO0VB99gv+L iZqbKnqeFiTL3LbKTaHmtb9/+Tpv9UzXPliWbGJ0= Date: Tue, 24 Oct 2023 08:58:40 -0700 From: Tyler Retzlaff To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: Konstantin Ananyev , dev@dpdk.org, Akhil Goyal , Anatoly Burakov , Andrew Rybchenko , Bruce Richardson , Chenbo Xia , Ciara Power , David Christensen , David Hunt , Dmitry Kozlyuk , Dmitry Malloy , Elena Agostini , Erik Gabriel Carrillo , Fan Zhang , Ferruh Yigit , Harman Kalra , Harry van Haaren , Honnappa Nagarahalli , Jerin Jacob , Matan Azrad , Maxime Coquelin , Narcisa Ana Maria Vasile , Nicolas Chautru , Olivier Matz , Ori Kam , Pallavi Kadam , Pavan Nikhilesh , Reshma Pattan , Sameh Gobriel , Shijith Thotton , Sivaprasad Tummala , Stephen Hemminger , Suanming Mou , Sunil Kumar Kori , Thomas Monjalon , Viacheslav Ovsiienko , Vladimir Medvedkin , Yipeng Wang Subject: Re: [PATCH v2 19/19] ring: use rte optional stdatomic API Message-ID: <20231024155840.GA32052@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1697497745-20664-1-git-send-email-roretzla@linux.microsoft.com> <1697574677-16578-1-git-send-email-roretzla@linux.microsoft.com> <1697574677-16578-20-git-send-email-roretzla@linux.microsoft.com> <516905e7-20eb-495b-bd66-9598fd9f27a2@yandex.ru> <98CBD80474FA8B44BF855DF32C47DC35E9EF72@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9EF72@smartserver.smartshare.dk> 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 On Tue, Oct 24, 2023 at 11:56:11AM +0200, Morten Brørup wrote: > > From: Konstantin Ananyev [mailto:konstantin.v.ananyev@yandex.ru] > > Sent: Tuesday, 24 October 2023 10.43 > > > > 17.10.2023 21:31, Tyler Retzlaff пишет: > > > Replace the use of gcc builtin __atomic_xxx intrinsics with > > > corresponding rte_atomic_xxx optional stdatomic API > > > > > > Signed-off-by: Tyler Retzlaff > > > --- > > [...] > > > > if (!single) > > > - rte_wait_until_equal_32(&ht->tail, old_val, __ATOMIC_RELAXED); > > > + rte_wait_until_equal_32((volatile uint32_t *)(uintptr_t)&ht- > > >tail, old_val, > > > > I suppose we do need that double type conversion only for atomic types > > right? > > > > > + rte_memory_order_relaxed); > > > > > > ht->tail = new_val; > > > } > > This got me thinking... > > Do we want to cast away the value's atomic attribute like this, or should we introduce new rte_atomic_wait_XX() functions with the parameters being pointers to atomic values, instead of pointers to simple values? just some notes here. so first let me start with it's okay to do this cast but only because we have knowledge of the internal implementation detail and this series has to do this in a few places. basically internally the actual atomic operation is fed back into an intrinsic/builtin that is either re-qualified as __rte_atomic or doesn't require qualification. i agree it isn't optimal since we have to take care should we ever alter the implementation to avoid compatibility problems but unlikely for it to be changed. we could provide new api but i'm not sure we can do that this late in the release cycle. notably i think it would be nicer if it *could* be made to be 'generic' as used literally in the atomics documentation which means it may operate on non-integer and non-pointer types. > > Just a thought. > > The initial rte_atomic_wait_XX() implementations could simply cast a away the atomic attribute like here. >